From 4bd106b089aeb17c075d5305c00384c11c129005 Mon Sep 17 00:00:00 2001 From: alan-baker <33432579+alan-baker@users.noreply.github.com> Date: Wed, 3 Apr 2019 10:30:12 -0400 Subject: [PATCH] Handle dead infinite loops in DCE (#2471) Fixes #2456 * When eliminating a structured construct that has an unreachable merge, replace that unreachable terminator with an appropriate return * New tests --- source/opt/aggressive_dead_code_elim_pass.cpp | 20 +++ test/opt/aggressive_dead_code_elim_test.cpp | 128 ++++++++++++++++++ 2 files changed, 148 insertions(+) diff --git a/source/opt/aggressive_dead_code_elim_pass.cpp b/source/opt/aggressive_dead_code_elim_pass.cpp index 82d749905..b5ce4cc7d 100644 --- a/source/opt/aggressive_dead_code_elim_pass.cpp +++ b/source/opt/aggressive_dead_code_elim_pass.cpp @@ -513,6 +513,26 @@ bool AggressiveDCEPass::AggressiveDCE(Function* func) { AddBranch(mergeBlockId, *bi); for (++bi; (*bi)->id() != mergeBlockId; ++bi) { } + + auto merge_terminator = (*bi)->terminator(); + if (merge_terminator->opcode() == SpvOpUnreachable) { + // The merge was unreachable. This is undefined behaviour so just + // return (or return an undef). Then mark the new return as live. + auto func_ret_type_inst = get_def_use_mgr()->GetDef(func->type_id()); + if (func_ret_type_inst->opcode() == SpvOpTypeVoid) { + merge_terminator->SetOpcode(SpvOpReturn); + } else { + // Find an undef for the return value and make sure it gets kept by + // the pass. + auto undef_id = Type2Undef(func->type_id()); + auto undef = get_def_use_mgr()->GetDef(undef_id); + live_insts_.Set(undef->unique_id()); + merge_terminator->SetOpcode(SpvOpReturnValue); + merge_terminator->SetInOperands({{SPV_OPERAND_TYPE_ID, {undef_id}}}); + get_def_use_mgr()->AnalyzeInstUse(merge_terminator); + } + live_insts_.Set(merge_terminator->unique_id()); + } } else { ++bi; } diff --git a/test/opt/aggressive_dead_code_elim_test.cpp b/test/opt/aggressive_dead_code_elim_test.cpp index 8b9b40a04..a883fb2b3 100644 --- a/test/opt/aggressive_dead_code_elim_test.cpp +++ b/test/opt/aggressive_dead_code_elim_test.cpp @@ -6210,6 +6210,134 @@ TEST_F(AggressiveDCETest, Dead) { SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS); SinglePassRunAndMatch(test, true); } + +TEST_F(AggressiveDCETest, DeadInfiniteLoop) { + const std::string test = R"( +; CHECK: OpSwitch {{%\w+}} {{%\w+}} {{\w+}} {{%\w+}} {{\w+}} [[block:%\w+]] +; CHECK: [[block]] = OpLabel +; CHECK-NEXT: OpBranch [[block:%\w+]] +; CHECK: [[block]] = OpLabel +; CHECK-NEXT: OpBranch [[block:%\w+]] +; CHECK: [[block]] = OpLabel +; CHECK-NEXT: OpReturn + OpCapability Shader + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %2 "main" + OpExecutionMode %2 OriginUpperLeft + %6 = OpTypeVoid + %7 = OpTypeFunction %6 + %8 = OpTypeFloat 32 + %9 = OpTypeVector %8 3 + %10 = OpTypeFunction %9 + %11 = OpConstant %8 1 + %12 = OpConstantComposite %9 %11 %11 %11 + %13 = OpTypeInt 32 1 + %32 = OpUndef %13 + %2 = OpFunction %6 None %7 + %33 = OpLabel + OpBranch %34 + %34 = OpLabel + OpLoopMerge %35 %36 None + OpBranch %37 + %37 = OpLabel + %38 = OpFunctionCall %9 %39 + OpSelectionMerge %40 None + OpSwitch %32 %40 14 %41 58 %42 + %42 = OpLabel + OpBranch %43 + %43 = OpLabel + OpLoopMerge %44 %45 None + OpBranch %45 + %45 = OpLabel + OpBranch %43 + %44 = OpLabel + OpUnreachable + %41 = OpLabel + OpBranch %36 + %40 = OpLabel + OpBranch %36 + %36 = OpLabel + OpBranch %34 + %35 = OpLabel + OpReturn + OpFunctionEnd + %39 = OpFunction %9 None %10 + %46 = OpLabel + OpReturnValue %12 + OpFunctionEnd +)"; + + SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS); + SinglePassRunAndMatch(test, true); +} + +TEST_F(AggressiveDCETest, DeadInfiniteLoopReturnValue) { + const std::string test = R"( +; CHECK: [[vec3:%\w+]] = OpTypeVector +; CHECK: [[undef:%\w+]] = OpUndef [[vec3]] +; CHECK: OpSwitch {{%\w+}} {{%\w+}} {{\w+}} {{%\w+}} {{\w+}} [[block:%\w+]] +; CHECK: [[block]] = OpLabel +; CHECK-NEXT: OpBranch [[block:%\w+]] +; CHECK: [[block]] = OpLabel +; CHECK-NEXT: OpBranch [[block:%\w+]] +; CHECK: [[block]] = OpLabel +; CHECK-NEXT: OpReturnValue [[undef]] + OpCapability Shader + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %2 "main" + OpExecutionMode %2 OriginUpperLeft + %6 = OpTypeVoid + %7 = OpTypeFunction %6 + %8 = OpTypeFloat 32 + %9 = OpTypeVector %8 3 + %10 = OpTypeFunction %9 + %11 = OpConstant %8 1 + %12 = OpConstantComposite %9 %11 %11 %11 + %13 = OpTypeInt 32 1 + %32 = OpUndef %13 + %2 = OpFunction %6 None %7 + %entry = OpLabel + %call = OpFunctionCall %9 %func + OpReturn + OpFunctionEnd + %func = OpFunction %9 None %10 + %33 = OpLabel + OpBranch %34 + %34 = OpLabel + OpLoopMerge %35 %36 None + OpBranch %37 + %37 = OpLabel + %38 = OpFunctionCall %9 %39 + OpSelectionMerge %40 None + OpSwitch %32 %40 14 %41 58 %42 + %42 = OpLabel + OpBranch %43 + %43 = OpLabel + OpLoopMerge %44 %45 None + OpBranch %45 + %45 = OpLabel + OpBranch %43 + %44 = OpLabel + OpUnreachable + %41 = OpLabel + OpBranch %36 + %40 = OpLabel + OpBranch %36 + %36 = OpLabel + OpBranch %34 + %35 = OpLabel + OpReturnValue %12 + OpFunctionEnd + %39 = OpFunction %9 None %10 + %46 = OpLabel + OpReturnValue %12 + OpFunctionEnd +)"; + + SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS); + SinglePassRunAndMatch(test, true); +} + // TODO(greg-lunarg): Add tests to verify handling of these cases: // // Check that logical addressing required