SSA-rewriter: make sure phi entries are unique. (#2206)
If there are multiple edges to a basic block, then the ssa rewriter will
create OpPhi instructions with duplicate entries. This is invalid, and
it is fixed in this commit.
Fixes #2202.
diff --git a/source/opt/ssa_rewrite_pass.cpp b/source/opt/ssa_rewrite_pass.cpp
index 0a5d390..f2cb2da 100644
--- a/source/opt/ssa_rewrite_pass.cpp
+++ b/source/opt/ssa_rewrite_pass.cpp
@@ -435,12 +435,22 @@
pass_->get_def_use_mgr()->GetDef(phi_candidate->var_id()));
std::vector<Operand> phi_operands;
uint32_t arg_ix = 0;
+ std::unordered_map<uint32_t, uint32_t> already_seen;
for (uint32_t pred_label : pass_->cfg()->preds(phi_candidate->bb()->id())) {
uint32_t op_val_id = GetPhiArgument(phi_candidate, arg_ix++);
- phi_operands.push_back(
- {spv_operand_type_t::SPV_OPERAND_TYPE_ID, {op_val_id}});
- phi_operands.push_back(
- {spv_operand_type_t::SPV_OPERAND_TYPE_ID, {pred_label}});
+ if (already_seen.count(pred_label) == 0) {
+ phi_operands.push_back(
+ {spv_operand_type_t::SPV_OPERAND_TYPE_ID, {op_val_id}});
+ phi_operands.push_back(
+ {spv_operand_type_t::SPV_OPERAND_TYPE_ID, {pred_label}});
+ already_seen[pred_label] = op_val_id;
+ } else {
+ // It is possible that there are two edges from the same parent block.
+ // Since the OpPhi can have only one entry for each parent, we have to
+ // make sure the two edges are consistent with each other.
+ assert(already_seen[pred_label] == op_val_id &&
+ "Inconsistent value for duplicate edges.");
+ }
}
// Generate a new OpPhi instruction and insert it in its basic
@@ -453,7 +463,6 @@
pass_->context()->set_instr_block(&*phi_inst, phi_candidate->bb());
auto insert_it = phi_candidate->bb()->begin();
insert_it.InsertBefore(std::move(phi_inst));
-
pass_->context()->get_decoration_mgr()->CloneDecorations(
phi_candidate->var_id(), phi_candidate->result_id(),
{SpvDecorationRelaxedPrecision});
diff --git a/test/opt/local_ssa_elim_test.cpp b/test/opt/local_ssa_elim_test.cpp
index 3bb1d53..b2961a2 100644
--- a/test/opt/local_ssa_elim_test.cpp
+++ b/test/opt/local_ssa_elim_test.cpp
@@ -1759,6 +1759,67 @@
SinglePassRunAndMatch<SSARewritePass>(spv_asm, true);
}
+// Test that the RelaxedPrecision decoration on the variable to added to the
+// result of the OpPhi instruction.
+TEST_F(LocalSSAElimTest, MultipleEdges) {
+ const std::string spv_asm = R"(
+ ; CHECK: OpSelectionMerge
+ ; CHECK: [[header_bb:%\w+]] = OpLabel
+ ; CHECK-NOT: OpLabel
+ ; CHECK: OpSwitch {{%\w+}} {{%\w+}} 76 [[bb1:%\w+]] 17 [[bb2:%\w+]]
+ ; CHECK-SAME: 4 [[bb2]]
+ ; CHECK: [[bb2]] = OpLabel
+ ; CHECK-NEXT: OpPhi [[type:%\w+]] [[val:%\w+]] [[header_bb]] %int_0 [[bb1]]
+ OpCapability Shader
+ %1 = OpExtInstImport "GLSL.std.450"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %4 "main"
+ OpExecutionMode %4 OriginUpperLeft
+ OpSource ESSL 310
+ %void = OpTypeVoid
+ %3 = OpTypeFunction %void
+ %int = OpTypeInt 32 1
+ %_ptr_Function_int = OpTypePointer Function %int
+ %int_0 = OpConstant %int 0
+ %bool = OpTypeBool
+ %true = OpConstantTrue %bool
+ %false = OpConstantFalse %bool
+ %int_1 = OpConstant %int 1
+ %4 = OpFunction %void None %3
+ %5 = OpLabel
+ %8 = OpVariable %_ptr_Function_int Function
+ OpBranch %10
+ %10 = OpLabel
+ OpLoopMerge %12 %13 None
+ OpBranch %14
+ %14 = OpLabel
+ OpBranchConditional %true %11 %12
+ %11 = OpLabel
+ OpSelectionMerge %19 None
+ OpBranchConditional %false %18 %19
+ %18 = OpLabel
+ OpSelectionMerge %22 None
+ OpSwitch %int_0 %22 76 %20 17 %21 4 %21
+ %20 = OpLabel
+ %23 = OpLoad %int %8
+ OpStore %8 %int_0
+ OpBranch %21
+ %21 = OpLabel
+ OpBranch %22
+ %22 = OpLabel
+ OpBranch %19
+ %19 = OpLabel
+ OpBranch %13
+ %13 = OpLabel
+ OpBranch %10
+ %12 = OpLabel
+ OpReturn
+ OpFunctionEnd
+ )";
+
+ SinglePassRunAndMatch<SSARewritePass>(spv_asm, true);
+}
+
// TODO(greg-lunarg): Add tests to verify handling of these cases:
//
// No optimization in the presence of
