Don't inline function containing OpKill (#2842)
If an OpKill instruction is inlined into a continue construct, then the
spir-v is no longer valid. To avoid this issue, we do inline into an
OpKill at all. This method was chosen because it is difficult to keep
track of whether or not you are in a continue construct while changing
the function that is being inlined into. This will work well with wrap
OpKill because every will still be inlined except for the OpKill
instruction itself.
Fixes #2554
Fixes #2433
This reverts commit aa9e8f538041db3055ea443080e0ccc315fa114f.
diff --git a/source/opt/inline_pass.cpp b/source/opt/inline_pass.cpp
index f348bbe..98daaf4 100644
--- a/source/opt/inline_pass.cpp
+++ b/source/opt/inline_pass.cpp
@@ -720,6 +720,15 @@
return false;
}
+ // Do not inline functions with an OpKill because they may be inlined into a
+ // continue construct.
+ bool has_opkill = !func->WhileEachInst(
+ [](Instruction* inst) { return inst->opcode() != SpvOpKill; });
+
+ if (has_opkill) {
+ return false;
+ }
+
return true;
}
diff --git a/test/opt/inline_test.cpp b/test/opt/inline_test.cpp
index 7170812..1fb0e72 100644
--- a/test/opt/inline_test.cpp
+++ b/test/opt/inline_test.cpp
@@ -2792,182 +2792,6 @@
}
}
-TEST_F(InlineTest, OpKill) {
- const std::string text = R"(
-; CHECK: OpFunction
-; CHECK-NEXT: OpLabel
-; CHECK-NEXT: OpKill
-; CHECK-NEXT: OpLabel
-; CHECK-NEXT: OpReturn
-; CHECK-NEXT: OpFunctionEnd
-OpCapability Shader
-OpMemoryModel Logical GLSL450
-OpEntryPoint Fragment %main "main"
-OpExecutionMode %main OriginUpperLeft
-%void = OpTypeVoid
-%voidfuncty = OpTypeFunction %void
-%main = OpFunction %void None %voidfuncty
-%1 = OpLabel
-%2 = OpFunctionCall %void %func
-OpReturn
-OpFunctionEnd
-%func = OpFunction %void None %voidfuncty
-%3 = OpLabel
-OpKill
-OpFunctionEnd
-)";
-
- SinglePassRunAndMatch<InlineExhaustivePass>(text, true);
-}
-
-TEST_F(InlineTest, OpKillWithTrailingInstructions) {
- const std::string text = R"(
-; CHECK: OpFunction
-; CHECK-NEXT: OpLabel
-; CHECK-NEXT: [[var:%\w+]] = OpVariable
-; CHECK-NEXT: OpKill
-; CHECK-NEXT: OpLabel
-; CHECK-NEXT: OpStore [[var]]
-; CHECK-NEXT: OpReturn
-; CHECK-NEXT: OpFunctionEnd
-OpCapability Shader
-OpMemoryModel Logical GLSL450
-OpEntryPoint Fragment %main "main"
-OpExecutionMode %main OriginUpperLeft
-%void = OpTypeVoid
-%bool = OpTypeBool
-%true = OpConstantTrue %bool
-%bool_func_ptr = OpTypePointer Function %bool
-%voidfuncty = OpTypeFunction %void
-%main = OpFunction %void None %voidfuncty
-%1 = OpLabel
-%2 = OpVariable %bool_func_ptr Function
-%3 = OpFunctionCall %void %func
-OpStore %2 %true
-OpReturn
-OpFunctionEnd
-%func = OpFunction %void None %voidfuncty
-%4 = OpLabel
-OpKill
-OpFunctionEnd
-)";
-
- SinglePassRunAndMatch<InlineExhaustivePass>(text, true);
-}
-
-TEST_F(InlineTest, OpKillInIf) {
- const std::string text = R"(
-; CHECK: OpFunction
-; CHECK: OpLabel
-; CHECK: [[var:%\w+]] = OpVariable
-; CHECK-NEXT: [[ld:%\w+]] = OpLoad {{%\w+}} [[var]]
-; CHECK-NEXT: OpBranch [[label:%\w+]]
-; CHECK-NEXT: [[label]] = OpLabel
-; CHECK-NEXT: OpLoopMerge [[loop_merge:%\w+]] [[continue:%\w+]] None
-; CHECK-NEXT: OpBranch [[label:%\w+]]
-; CHECK-NEXT: [[label]] = OpLabel
-; CHECK-NEXT: OpSelectionMerge [[sel_merge:%\w+]] None
-; CHECK-NEXT: OpBranchConditional {{%\w+}} [[kill_label:%\w+]] [[label:%\w+]]
-; CHECK-NEXT: [[kill_label]] = OpLabel
-; CHECK-NEXT: OpKill
-; CHECK-NEXT: [[label]] = OpLabel
-; CHECK-NEXT: OpBranch [[loop_merge]]
-; CHECK-NEXT: [[sel_merge]] = OpLabel
-; CHECK-NEXT: OpBranch [[loop_merge]]
-; CHECK-NEXT: [[continue]] = OpLabel
-; CHECK-NEXT: OpBranchConditional
-; CHECK-NEXT: [[loop_merge]] = OpLabel
-; CHECK-NEXT: OpStore [[var]] [[ld]]
-; CHECK-NEXT: OpReturn
-; CHECK-NEXT: OpFunctionEnd
-OpCapability Shader
-OpMemoryModel Logical GLSL450
-OpEntryPoint Fragment %main "main"
-OpExecutionMode %main OriginUpperLeft
-%void = OpTypeVoid
-%bool = OpTypeBool
-%true = OpConstantTrue %bool
-%bool_func_ptr = OpTypePointer Function %bool
-%voidfuncty = OpTypeFunction %void
-%main = OpFunction %void None %voidfuncty
-%1 = OpLabel
-%2 = OpVariable %bool_func_ptr Function
-%3 = OpLoad %bool %2
-%4 = OpFunctionCall %void %func
-OpStore %2 %3
-OpReturn
-OpFunctionEnd
-%func = OpFunction %void None %voidfuncty
-%5 = OpLabel
-OpSelectionMerge %6 None
-OpBranchConditional %true %7 %8
-%7 = OpLabel
-OpKill
-%8 = OpLabel
-OpReturn
-%6 = OpLabel
-OpReturn
-OpFunctionEnd
-)";
-
- SinglePassRunAndMatch<InlineExhaustivePass>(text, true);
-}
-
-TEST_F(InlineTest, OpKillInLoop) {
- const std::string text = R"(
-; CHECK: OpFunction
-; CHECK: OpLabel
-; CHECK: [[var:%\w+]] = OpVariable
-; CHECK-NEXT: [[ld:%\w+]] = OpLoad {{%\w+}} [[var]]
-; CHECK-NEXT: OpBranch [[loop:%\w+]]
-; CHECK-NEXT: [[loop]] = OpLabel
-; CHECK-NEXT: OpLoopMerge [[loop_merge:%\w+]] [[continue:%\w+]] None
-; CHECK-NEXT: OpBranch [[label:%\w+]]
-; CHECK-NEXT: [[label]] = OpLabel
-; CHECK-NEXT: OpKill
-; CHECK-NEXT: [[loop_merge]] = OpLabel
-; CHECK-NEXT: OpBranch [[label:%\w+]]
-; CHECK-NEXT: [[continue]] = OpLabel
-; CHECK-NEXT: OpBranch [[loop]]
-; CHECK-NEXT: [[label]] = OpLabel
-; CHECK-NEXT: OpStore [[var]] [[ld]]
-; CHECK-NEXT: OpReturn
-; CHECK-NEXT: OpFunctionEnd
-OpCapability Shader
-OpMemoryModel Logical GLSL450
-OpEntryPoint Fragment %main "main"
-OpExecutionMode %main OriginUpperLeft
-%void = OpTypeVoid
-%bool = OpTypeBool
-%true = OpConstantTrue %bool
-%voidfuncty = OpTypeFunction %void
-%bool_func_ptr = OpTypePointer Function %bool
-%main = OpFunction %void None %voidfuncty
-%1 = OpLabel
-%2 = OpVariable %bool_func_ptr Function
-%3 = OpLoad %bool %2
-%4 = OpFunctionCall %void %func
-OpStore %2 %3
-OpReturn
-OpFunctionEnd
-%func = OpFunction %void None %voidfuncty
-%5 = OpLabel
-OpBranch %10
-%10 = OpLabel
-OpLoopMerge %6 %7 None
-OpBranch %8
-%8 = OpLabel
-OpKill
-%6 = OpLabel
-OpReturn
-%7 = OpLabel
-OpBranch %10
-OpFunctionEnd
-)";
-
- SinglePassRunAndMatch<InlineExhaustivePass>(text, true);
-}
-
TEST_F(InlineTest, OpVariableWithInit) {
// Check that there is a store that corresponds to the initializer. This
// test makes sure that is a store to the variable in the loop and before any
@@ -3112,6 +2936,46 @@
SinglePassRunAndCheck<InlineExhaustivePass>(test, test, false, true);
}
+TEST_F(InlineTest, DontInlineFuncWithOpKill) {
+ const std::string test =
+ R"(OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+OpExecutionMode %main OriginUpperLeft
+OpSource GLSL 330
+OpName %main "main"
+OpName %kill_ "kill("
+%void = OpTypeVoid
+%3 = OpTypeFunction %void
+%bool = OpTypeBool
+%true = OpConstantTrue %bool
+%main = OpFunction %void None %3
+%5 = OpLabel
+OpBranch %9
+%9 = OpLabel
+OpLoopMerge %11 %12 None
+OpBranch %13
+%13 = OpLabel
+OpBranchConditional %true %10 %11
+%10 = OpLabel
+OpBranch %12
+%12 = OpLabel
+%16 = OpFunctionCall %void %kill_
+OpBranch %9
+%11 = OpLabel
+OpReturn
+OpFunctionEnd
+%kill_ = OpFunction %void None %3
+%7 = OpLabel
+OpKill
+OpFunctionEnd
+)";
+
+ SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+ SinglePassRunAndCheck<InlineExhaustivePass>(test, test, false, true);
+}
+
// TODO(greg-lunarg): Add tests to verify handling of these cases:
//
// Empty modules
