diff --git a/source/opt/ssa_rewrite_pass.cpp b/source/opt/ssa_rewrite_pass.cpp index f2cb2da28..7144ca0ca 100644 --- a/source/opt/ssa_rewrite_pass.cpp +++ b/source/opt/ssa_rewrite_pass.cpp @@ -102,6 +102,7 @@ void SSARewriter::ReplacePhiUsersWith(const PhiCandidate& phi_to_remove, uint32_t repl_id) { for (uint32_t user_id : phi_to_remove.users()) { PhiCandidate* user_phi = GetPhiCandidate(user_id); + BasicBlock* bb = pass_->context()->get_instr_block(user_id); if (user_phi) { // If the user is a Phi candidate, replace all arguments that refer to // |phi_to_remove.result_id()| with |repl_id|. @@ -110,6 +111,10 @@ void SSARewriter::ReplacePhiUsersWith(const PhiCandidate& phi_to_remove, arg = repl_id; } } + } else if (bb->id() == user_id) { + // The phi candidate is the definition of the variable at basic block + // |bb|. We must change this to the replacement. + WriteVariable(phi_to_remove.var_id(), bb, repl_id); } else { // For regular loads, traverse the |load_replacement_| table looking for // instances of |phi_to_remove|. @@ -259,6 +264,8 @@ uint32_t SSARewriter::GetReachingDef(uint32_t var_id, BasicBlock* bb) { // require a Phi instruction. This will act as |var_id|'s current // definition to break potential cycles. PhiCandidate& phi_candidate = CreatePhiCandidate(var_id, bb); + + // Set the value for |bb| to avoid an infinite recursion. WriteVariable(var_id, bb, phi_candidate.result_id()); val_id = AddPhiOperands(&phi_candidate); } diff --git a/source/opt/ssa_rewrite_pass.h b/source/opt/ssa_rewrite_pass.h index c0373dc06..fddbdaf54 100644 --- a/source/opt/ssa_rewrite_pass.h +++ b/source/opt/ssa_rewrite_pass.h @@ -188,6 +188,9 @@ class SSARewriter { // value |val_id|. void WriteVariable(uint32_t var_id, BasicBlock* bb, uint32_t val_id) { defs_at_block_[bb][var_id] = val_id; + if (auto* pc = GetPhiCandidate(val_id)) { + pc->AddUser(bb->id()); + } } // Processes the store operation |inst| in basic block |bb|. This extracts diff --git a/test/opt/local_ssa_elim_test.cpp b/test/opt/local_ssa_elim_test.cpp index d2da1f9c1..a490d5290 100644 --- a/test/opt/local_ssa_elim_test.cpp +++ b/test/opt/local_ssa_elim_test.cpp @@ -1930,6 +1930,63 @@ TEST_F(LocalSSAElimTest, VariablePointerTest2) { SinglePassRunAndMatch(text, false); } +TEST_F(LocalSSAElimTest, ChainedTrivialPhis) { + // Check that the copy object get the undef value implicitly assigned in the + // entry block. + const std::string text = R"( +; CHECK: [[undef:%\w+]] = OpUndef %v4float +; CHECK: OpCopyObject %v4float [[undef]] + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint GLCompute %2 "main" + OpExecutionMode %2 LocalSize 1 18 6 + OpSource ESSL 310 + %void = OpTypeVoid + %4 = OpTypeFunction %void + %bool = OpTypeBool + %float = OpTypeFloat 32 + %v4float = OpTypeVector %float 4 +%_ptr_Function_v4float = OpTypePointer Function %v4float + %2 = OpFunction %void None %4 + %9 = OpLabel + %10 = OpVariable %_ptr_Function_v4float Function + OpBranch %11 + %11 = OpLabel + OpLoopMerge %12 %13 None + OpBranch %14 + %14 = OpLabel + %15 = OpUndef %bool + OpBranchConditional %15 %16 %12 + %16 = OpLabel + %17 = OpUndef %bool + OpSelectionMerge %18 None + OpBranchConditional %17 %19 %18 + %19 = OpLabel + %20 = OpUndef %bool + OpLoopMerge %21 %22 None + OpBranchConditional %20 %23 %21 + %23 = OpLabel + %24 = OpLoad %v4float %10 + %25 = OpCopyObject %v4float %24 + %26 = OpUndef %bool + OpBranch %22 + %22 = OpLabel + OpBranch %19 + %21 = OpLabel + OpBranch %12 + %18 = OpLabel + OpBranch %13 + %13 = OpLabel + OpBranch %11 + %12 = OpLabel + %27 = OpLoad %v4float %10 + OpReturn + OpFunctionEnd + )"; + SinglePassRunAndMatch(text, false); +} + // TODO(greg-lunarg): Add tests to verify handling of these cases: // // No optimization in the presence of