From f98473ceeb1d33700d01e20910433583e5256030 Mon Sep 17 00:00:00 2001
From: Steven Perron <stevenperron@google.com>
Date: Wed, 21 Sep 2022 16:10:58 -0400
Subject: Remove `spvOpcodeTerminatesExecution` (#4931)

* Remove `spvOpcodeTerminatesExecution`

This function is the same as `spvOpcodeIsAbort` except for
OpUnreachable.  The names are so close in meaning that it is hard to
distinguish them.  I've removed `spvOpcodeTerminatesExecution` since it
is used in only a single place.  I've special cased OpUnreachable in
that location.

At the same time, I fixed up some comments related to the use of the
TerminatesExecution and IsAbort functions.

Following up on #4930.

* Fix comments
---
 source/opcode.cpp          | 13 -------------
 source/opcode.h            | 10 ++++------
 source/opt/inline_pass.cpp | 15 +++++++++------
 source/opt/inline_pass.h   |  6 +++---
 4 files changed, 16 insertions(+), 28 deletions(-)

diff --git a/source/opcode.cpp b/source/opcode.cpp
index 88c5c51b9..3f927290e 100644
--- a/source/opcode.cpp
+++ b/source/opcode.cpp
@@ -468,19 +468,6 @@ bool spvOpcodeIsBlockTerminator(SpvOp opcode) {
   return spvOpcodeIsBranch(opcode) || spvOpcodeIsReturnOrAbort(opcode);
 }
 
-bool spvOpcodeTerminatesExecution(SpvOp opcode) {
-  switch (opcode) {
-    case SpvOpKill:
-    case SpvOpTerminateInvocation:
-    case SpvOpTerminateRayKHR:
-    case SpvOpIgnoreIntersectionKHR:
-    case SpvOpEmitMeshTasksEXT:
-      return true;
-    default:
-      return false;
-  }
-}
-
 bool spvOpcodeIsBaseOpaqueType(SpvOp opcode) {
   switch (opcode) {
     case SpvOpTypeImage:
diff --git a/source/opcode.h b/source/opcode.h
index c8525a253..77a0bed25 100644
--- a/source/opcode.h
+++ b/source/opcode.h
@@ -110,18 +110,16 @@ bool spvOpcodeIsBranch(SpvOp opcode);
 // Returns true if the given opcode is a return instruction.
 bool spvOpcodeIsReturn(SpvOp opcode);
 
-// Returns true if the given opcode aborts execution.
+// Returns true if the given opcode aborts execution.  To abort means that after
+// executing that instruction, no other instructions will be executed regardless
+// of the context in which the instruction appears.  Note that `OpUnreachable`
+// is considered an abort even if its behaviour is undefined.
 bool spvOpcodeIsAbort(SpvOp opcode);
 
 // Returns true if the given opcode is a return instruction or it aborts
 // execution.
 bool spvOpcodeIsReturnOrAbort(SpvOp opcode);
 
-// Returns true if the given opcode is a kill instruction or it terminates
-// execution. Note that branches, returns, and unreachables do not terminate
-// execution.
-bool spvOpcodeTerminatesExecution(SpvOp opcode);
-
 // Returns true if the given opcode is a basic block terminator.
 bool spvOpcodeIsBlockTerminator(SpvOp opcode);
 
diff --git a/source/opt/inline_pass.cpp b/source/opt/inline_pass.cpp
index 6e73f1cb9..e14516f75 100644
--- a/source/opt/inline_pass.cpp
+++ b/source/opt/inline_pass.cpp
@@ -794,22 +794,25 @@ bool InlinePass::IsInlinableFunction(Function* func) {
     return false;
   }
 
-  // Do not inline functions with an OpKill if they are called from a continue
-  // construct. If it is inlined into a continue construct it will generate
-  // invalid code.
+  // Do not inline functions with an abort instruction if they are called from a
+  // continue construct. If it is inlined into a continue construct the backedge
+  // will no longer post-dominate the continue target, which is invalid.  An
+  // `OpUnreachable` is acceptable because it will not change post-dominance if
+  // it is statically unreachable.
   bool func_is_called_from_continue =
       funcs_called_from_continue_.count(func->result_id()) != 0;
 
-  if (func_is_called_from_continue && ContainsKillOrTerminateInvocation(func)) {
+  if (func_is_called_from_continue && ContainsAbortOtherThanUnreachable(func)) {
     return false;
   }
 
   return true;
 }
 
-bool InlinePass::ContainsKillOrTerminateInvocation(Function* func) const {
+bool InlinePass::ContainsAbortOtherThanUnreachable(Function* func) const {
   return !func->WhileEachInst([](Instruction* inst) {
-    return !spvOpcodeTerminatesExecution(inst->opcode());
+    return inst->opcode() == SpvOpUnreachable ||
+           !spvOpcodeIsAbort(inst->opcode());
   });
 }
 
diff --git a/source/opt/inline_pass.h b/source/opt/inline_pass.h
index f20439542..d29c1e074 100644
--- a/source/opt/inline_pass.h
+++ b/source/opt/inline_pass.h
@@ -139,9 +139,9 @@ class InlinePass : public Pass {
   // Return true if |func| is a function that can be inlined.
   bool IsInlinableFunction(Function* func);
 
-  // Returns true if |func| contains an OpKill or OpTerminateInvocation
-  // instruction.
-  bool ContainsKillOrTerminateInvocation(Function* func) const;
+  // Returns true if |func| contains an abort instruction that is not an
+  // `OpUnreachable` instruction.
+  bool ContainsAbortOtherThanUnreachable(Function* func) const;
 
   // Update phis in succeeding blocks to point to new last block
   void UpdateSucceedingPhis(
-- 
cgit v1.2.3