From 3f8501de9e17528a87c9abd6ab2bffd9948402d7 Mon Sep 17 00:00:00 2001 From: Steven Perron Date: Mon, 24 Aug 2020 15:08:55 -0400 Subject: [PATCH] Add undef for inlined void function (#3720) It is possible that the result of a void function call is used. In case it is used, we need something that still defines its id after inlining. We use an undef for that purpose. Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/3704 --- source/opt/inline_pass.cpp | 8 ++++ test/opt/inline_opaque_test.cpp | 6 ++- test/opt/inline_test.cpp | 68 +++++++++++++++++++++++++-------- 3 files changed, 65 insertions(+), 17 deletions(-) diff --git a/source/opt/inline_pass.cpp b/source/opt/inline_pass.cpp index ef94d0d6c..6021a7c53 100644 --- a/source/opt/inline_pass.cpp +++ b/source/opt/inline_pass.cpp @@ -619,6 +619,14 @@ bool InlinePass::GenInlineCode( assert(resId != 0); AddLoad(calleeTypeId, resId, returnVarId, &new_blk_ptr, call_inst_itr->dbg_line_inst(), call_inst_itr->GetDebugScope()); + } else { + // Even though it is very unlikely, it is possible that the result id of + // the void-function call is used, so we need to generate an instruction + // with that result id. + std::unique_ptr undef_inst( + new Instruction(context(), SpvOpUndef, call_inst_itr->type_id(), + call_inst_itr->result_id(), {})); + context()->AddGlobalValue(std::move(undef_inst)); } // Move instructions of original caller block after call instruction. diff --git a/test/opt/inline_opaque_test.cpp b/test/opt/inline_opaque_test.cpp index b8d2dfada..47e053339 100644 --- a/test/opt/inline_opaque_test.cpp +++ b/test/opt/inline_opaque_test.cpp @@ -90,7 +90,8 @@ OpFunctionEnd )"; const std::string after = - R"(%main = OpFunction %void None %12 + R"(%34 = OpUndef %void +%main = OpFunction %void None %12 %28 = OpLabel %s0 = OpVariable %_ptr_Function_S_t Function %param = OpVariable %_ptr_Function_S_t Function @@ -289,7 +290,8 @@ OpFunctionEnd )"; const std::string after = - R"(%main2 = OpFunction %void None %13 + R"(%35 = OpUndef %void +%main2 = OpFunction %void None %13 %29 = OpLabel %s0 = OpVariable %_ptr_Function_S_t Function %param = OpVariable %_ptr_Function_S_t Function diff --git a/test/opt/inline_test.cpp b/test/opt/inline_test.cpp index 572e7b0cc..951721bfa 100644 --- a/test/opt/inline_test.cpp +++ b/test/opt/inline_test.cpp @@ -381,6 +381,7 @@ TEST_F(InlineTest, InOutParameter) { const std::vector after = { // clang-format off + "%26 = OpUndef %void", "%main = OpFunction %void None %11", "%23 = OpLabel", "%b = OpVariable %_ptr_Function_v4float Function", @@ -1503,11 +1504,11 @@ OpSource OpenCL_C 120 %bool = OpTypeBool %true = OpConstantTrue %bool %void = OpTypeVoid +%5 = OpTypeFunction %void )"; const std::string nonEntryFuncs = - R"(%5 = OpTypeFunction %void -%6 = OpFunction %void None %5 + R"(%6 = OpFunction %void None %5 %7 = OpLabel OpBranch %8 %8 = OpLabel @@ -1542,9 +1543,11 @@ OpReturn OpFunctionEnd )"; - SinglePassRunAndCheck(predefs + nonEntryFuncs + before, - predefs + nonEntryFuncs + after, - false, true); + const std::string undef = "%11 = OpUndef %void\n"; + + SinglePassRunAndCheck( + predefs + nonEntryFuncs + before, predefs + undef + nonEntryFuncs + after, + false, true); } TEST_F(InlineTest, MultiBlockLoopHeaderCallsMultiBlockCallee) { @@ -1619,9 +1622,10 @@ OpReturn OpFunctionEnd )"; - SinglePassRunAndCheck(predefs + nonEntryFuncs + before, - predefs + nonEntryFuncs + after, - false, true); + const std::string undef = "%20 = OpUndef %void\n"; + SinglePassRunAndCheck( + predefs + nonEntryFuncs + before, predefs + undef + nonEntryFuncs + after, + false, true); } TEST_F(InlineTest, SingleBlockLoopCallsMultiBlockCalleeHavingSelectionMerge) { @@ -1707,10 +1711,10 @@ OpBranchConditional %true %13 %16 OpReturn OpFunctionEnd )"; - - SinglePassRunAndCheck(predefs + nonEntryFuncs + before, - predefs + nonEntryFuncs + after, - false, true); + const std::string undef = "%15 = OpUndef %void\n"; + SinglePassRunAndCheck( + predefs + nonEntryFuncs + before, predefs + undef + nonEntryFuncs + after, + false, true); } TEST_F(InlineTest, @@ -1789,9 +1793,10 @@ OpReturn OpFunctionEnd )"; - SinglePassRunAndCheck(predefs + nonEntryFuncs + before, - predefs + nonEntryFuncs + after, - false, true); + const std::string undef = "%20 = OpUndef %void\n"; + SinglePassRunAndCheck( + predefs + nonEntryFuncs + before, predefs + undef + nonEntryFuncs + after, + false, true); } TEST_F(InlineTest, NonInlinableCalleeWithSingleReturn) { @@ -2164,6 +2169,7 @@ OpName %foo "foo" OpName %foo_entry "foo_entry" %void = OpTypeVoid %void_fn = OpTypeFunction %void +%3 = OpUndef %void %foo = OpFunction %void None %void_fn %foo_entry = OpLabel OpReturn @@ -2437,6 +2443,7 @@ OpName %kill_ "kill(" %3 = OpTypeFunction %void %bool = OpTypeBool %true = OpConstantTrue %bool +%16 = OpUndef %void %main = OpFunction %void None %3 %5 = OpLabel OpKill @@ -2534,6 +2541,7 @@ OpName %kill_ "kill(" %3 = OpTypeFunction %void %bool = OpTypeBool %true = OpConstantTrue %bool +%16 = OpUndef %void %main = OpFunction %void None %3 %5 = OpLabel OpTerminateInvocation @@ -2761,6 +2769,7 @@ OpFunctionEnd %uint_0 = OpConstant %uint 0 %false = OpConstantFalse %bool %_ptr_Function_bool = OpTypePointer Function %bool +%11 = OpUndef %void %foo_ = OpFunction %void None %4 %7 = OpLabel %18 = OpVariable %_ptr_Function_bool Function %false @@ -3849,6 +3858,35 @@ OpFunctionEnd SinglePassRunAndMatch(text, true); } +TEST_F(InlineTest, UsingVoidFunctionResult) { + const std::string text = R"( +; CHECK: [[undef:%\w+]] = OpUndef %void +; CHECK: OpFunction +; CHECK: OpCopyObject %void [[undef]] +; CHECK: OpFunctionEnd + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %4 "main" + OpExecutionMode %4 OriginUpperLeft + OpSource ESSL 320 + %2 = OpTypeVoid + %3 = OpTypeFunction %2 + %4 = OpFunction %2 None %3 + %5 = OpLabel + %8 = OpFunctionCall %2 %6 + %9 = OpCopyObject %2 %8 + OpReturn + OpFunctionEnd + %6 = OpFunction %2 None %3 + %7 = OpLabel + OpReturn + OpFunctionEnd +)"; + + SinglePassRunAndMatch(text, true); +} + // TODO(greg-lunarg): Add tests to verify handling of these cases: // // Empty modules