Place load after OpPhi instructions in block. (#2246)
We currently place the load instructions at the start of the basic block
that dominates all of the loads. If that basic block contains OpPhi
instructions, then this will generate invalid code. We just need to
search for a location that comes after all of the OpPhi instructions.
Fixes #2204.
diff --git a/source/opt/common_uniform_elim_pass.cpp b/source/opt/common_uniform_elim_pass.cpp
index efa40aa..52d279f 100644
--- a/source/opt/common_uniform_elim_pass.cpp
+++ b/source/opt/common_uniform_elim_pass.cpp
@@ -282,6 +282,7 @@
uint32_t replId;
GenACLoadRepl(ptrInst, &newInsts, &replId);
inst = ReplaceAndDeleteLoad(inst, replId, ptrInst);
+ assert(inst->opcode() != SpvOpPhi);
inst = inst->InsertBefore(std::move(newInsts));
modified = true;
}
@@ -355,6 +356,10 @@
if (mergeBlockId == bp->id()) {
mergeBlockId = 0;
insertItr = bp->begin();
+ while (insertItr->opcode() == SpvOpPhi) {
+ ++insertItr;
+ }
+
// Update insertItr until it will not be removed. Without this code,
// ReplaceAndDeleteLoad() can set |insertItr| as a dangling pointer.
while (IsUniformLoadToBeRemoved(&*insertItr)) ++insertItr;
diff --git a/test/opt/common_uniform_elim_test.cpp b/test/opt/common_uniform_elim_test.cpp
index 76ab344..9e39439 100644
--- a/test/opt/common_uniform_elim_test.cpp
+++ b/test/opt/common_uniform_elim_test.cpp
@@ -1329,6 +1329,60 @@
SinglePassRunAndMatch<CommonUniformElimPass>(text, true);
}
+TEST_F(CommonUniformElimTest, LoadPlacedAfterPhi) {
+ const std::string text = R"(
+; CHECK: [[var:%\w+]] = OpVariable {{%\w+}} Uniform
+; CHECK: OpSelectionMerge [[merge:%\w+]]
+; CHECK: [[merge]] = OpLabel
+; CHECK-NEXT: OpPhi
+; CHECK-NEXT: OpLoad {{%\w+}} [[var]]
+ OpCapability Shader
+ %1 = OpExtInstImport "GLSL.std.450"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %2 "main"
+ OpExecutionMode %2 OriginUpperLeft
+ OpSource ESSL 310
+ OpMemberDecorate %_struct_3 0 Offset 0
+ OpDecorate %_struct_3 Block
+ OpDecorate %4 DescriptorSet 0
+ OpDecorate %4 Binding 0
+ %void = OpTypeVoid
+ %6 = OpTypeFunction %void
+ %bool = OpTypeBool
+ %false = OpConstantFalse %bool
+ %uint = OpTypeInt 32 0
+ %v2uint = OpTypeVector %uint 2
+ %_struct_3 = OpTypeStruct %v2uint
+%_ptr_Uniform__struct_3 = OpTypePointer Uniform %_struct_3
+ %4 = OpVariable %_ptr_Uniform__struct_3 Uniform
+ %uint_0 = OpConstant %uint 0
+%_ptr_Uniform_uint = OpTypePointer Uniform %uint
+ %uint_2 = OpConstant %uint 2
+ %2 = OpFunction %void None %6
+ %15 = OpLabel
+ OpSelectionMerge %16 None
+ OpBranchConditional %false %17 %16
+ %17 = OpLabel
+ OpBranch %16
+ %16 = OpLabel
+ %18 = OpPhi %bool %false %15 %false %17
+ OpSelectionMerge %19 None
+ OpBranchConditional %false %20 %21
+ %20 = OpLabel
+ %22 = OpAccessChain %_ptr_Uniform_uint %4 %uint_0 %uint_0
+ %23 = OpLoad %uint %22
+ OpBranch %19
+ %21 = OpLabel
+ OpBranch %19
+ %19 = OpLabel
+ OpReturn
+ OpFunctionEnd
+)";
+
+ SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+ SinglePassRunAndMatch<CommonUniformElimPass>(text, true);
+}
+
// TODO(greg-lunarg): Add tests to verify handling of these cases:
//
// Disqualifying cases: extensions, decorations, non-logical addressing,
