AuroraMiddleware/SPIRV-Tools
1
0
Fork 0
mirror of https://github.com/KhronosGroup/SPIRV-Tools synced 2024-10-18 19:20:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

Place load after OpPhi instructions in block. (#2246)

Browse Source 
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.
This commit is contained in:
Steven Perron 2018-12-19 15:18:22 +00:00 committed by GitHub
parent 71aa48f91d
commit 9e81c337f9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 59 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
source/opt/common_uniform_elim_pass.cpp
View File

@ -282,6 +282,7 @@ bool CommonUniformElimPass::UniformAccessChainConvert(Function* func) {
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 @@ bool CommonUniformElimPass::CommonUniformLoadElimination(Function* func) {
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;

54
test/opt/common_uniform_elim_test.cpp
View File

@ -1329,6 +1329,60 @@ TEST_F(CommonUniformElimTest, MixedConstantAndNonConstantIndexes) {
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,