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
diff --git a/source/opcode.cpp b/source/opcode.cpp
index 88c5c51..3f92729 100644
--- a/source/opcode.cpp
+++ b/source/opcode.cpp
@@ -468,19 +468,6 @@
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 c8525a2..77a0bed 100644
--- a/source/opcode.h
+++ b/source/opcode.h
@@ -110,18 +110,16 @@
// 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 6e73f1c..e14516f 100644
--- a/source/opt/inline_pass.cpp
+++ b/source/opt/inline_pass.cpp
@@ -794,22 +794,25 @@
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 f204395..d29c1e0 100644
--- a/source/opt/inline_pass.h
+++ b/source/opt/inline_pass.h
@@ -139,9 +139,9 @@
// 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(
