Fix dangling phi bug from loop-unroll (#4239)

Fix dangling phi bug from loop-unroll

When unrolling the following loop:
```
%const0 = OpConstant ...
%const1 = OpConstant ...
...
%LoopHeader = OpLabel
%phi0 = OpPhi %float %const0 %PreHeader %phi1 %Latch
%phi1 = OpPhi %float %const1 %PreHeader %x    %Latch
...
%LoopBody = OpLabel
%x = OpFSub %float %phi1 %phi0
...
```

the loop-unroll pass sets the value of `%phi0` as `%phi1` for the second
copy of the loop body. For example, the second copy of
`%x = OpFSub %float %phi1 %phi0` will be
`%y = OpFSub %float %x %phi1`.

Since all phi instructions for inductions will are removed after the
loop unrolling, `%phi1` will be a dead dangling phi.

It happens only for the phi values of the first loop iteration. Replacing those
dangling phis with their initial values fixes this issue.

For example, the second copy of `%x = OpFSub %float %phi1 %phi0` should be
`%y = OpFSub %float %x %const1` because the value of `%phi1` from the
first loop iteration is `%const1`.
This commit is contained in:
Jaebaek Seo 2021-04-27 16:27:09 -04:00 committed by GitHub
parent 07ec4f83c5
commit 089d716d25
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 212 additions and 0 deletions

View File

@ -797,6 +797,9 @@ void LoopUnrollerUtilsImpl::CloseUnrolledLoop(Loop* loop) {
for (BasicBlock* block : loop_blocks_inorder_) {
RemapOperands(block);
}
for (auto& block_itr : blocks_to_add_) {
RemapOperands(block_itr.get());
}
// Rewrite the last phis, since they may still reference the original phi.
for (Instruction* last_phi : state_.previous_phis_) {

View File

@ -3401,6 +3401,215 @@ OpFunctionEnd
SinglePassRunAndCheck<LoopUnroller>(shader, output, false);
}
TEST_F(PassClassTest, UnrollWithPhiReferencesPhi) {
const std::string text = R"(
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main" %color
OpExecutionMode %main OriginUpperLeft
OpSource HLSL 600
OpName %main "main"
OpName %color "color"
OpDecorate %color Location 0
%uint = OpTypeInt 32 0
%float = OpTypeFloat 32
%float_0 = OpConstant %float 0
%float_1 = OpConstant %float 1
%uint_1 = OpConstant %uint 1
%uint_3 = OpConstant %uint 3
%void = OpTypeVoid
%11 = OpTypeFunction %void
%bool = OpTypeBool
%v4float = OpTypeVector %float 4
%_ptr_Output_v4float = OpTypePointer Output %v4float
%color = OpVariable %_ptr_Output_v4float Output
%main = OpFunction %void None %11
%15 = OpLabel
OpBranch %16
%16 = OpLabel
%17 = OpPhi %float %float_0 %15 %18 %19
%18 = OpPhi %float %float_1 %15 %20 %19
%21 = OpPhi %uint %uint_1 %15 %22 %19
%23 = OpULessThanEqual %bool %21 %uint_3
OpLoopMerge %24 %19 Unroll
OpBranchConditional %23 %25 %24
%25 = OpLabel
; First loop iteration
; CHECK: [[next_phi1_0:%\w+]] = OpFSub %float %float_1 %float_0
; Second loop iteration
; CHECK: [[next_phi1_1:%\w+]] = OpFSub %float [[next_phi1_0]] %float_1
; Third loop iteration
; CHECK: OpFSub %float [[next_phi1_1]] [[next_phi1_0]]
%20 = OpFSub %float %18 %17
OpBranch %19
%19 = OpLabel
%22 = OpIAdd %uint %21 %uint_1
OpBranch %16
%24 = OpLabel
OpReturn
OpFunctionEnd
)";
std::unique_ptr<IRContext> context =
BuildModule(SPV_ENV_UNIVERSAL_1_1, nullptr, text,
SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
Module* module = context->module();
EXPECT_NE(nullptr, module) << "Assembling failed for ushader:\n"
<< text << std::endl;
LoopUnroller loop_unroller;
SetDisassembleOptions(SPV_BINARY_TO_TEXT_OPTION_NO_HEADER |
SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES);
SinglePassRunAndMatch<LoopUnroller>(text, true);
}
TEST_F(PassClassTest, UnrollWithDoublePhiReferencesPhi) {
const std::string text = R"(
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main" %color
OpExecutionMode %main OriginUpperLeft
OpSource HLSL 600
OpName %main "main"
OpName %color "color"
OpDecorate %color Location 0
%uint = OpTypeInt 32 0
%float = OpTypeFloat 32
%float_0 = OpConstant %float 0
%float_1 = OpConstant %float 1
%uint_1 = OpConstant %uint 1
%uint_3 = OpConstant %uint 3
%void = OpTypeVoid
%11 = OpTypeFunction %void
%bool = OpTypeBool
%v4float = OpTypeVector %float 4
%_ptr_Output_v4float = OpTypePointer Output %v4float
%color = OpVariable %_ptr_Output_v4float Output
%main = OpFunction %void None %11
%15 = OpLabel
OpBranch %16
%16 = OpLabel
%17 = OpPhi %float %float_1 %15 %18 %19
%18 = OpPhi %float %float_0 %15 %20 %19
%20 = OpPhi %float %float_1 %15 %21 %19
%22 = OpPhi %uint %uint_1 %15 %23 %19
%24 = OpULessThanEqual %bool %22 %uint_3
OpLoopMerge %25 %19 Unroll
OpBranchConditional %24 %26 %25
%26 = OpLabel
; First loop iteration
; CHECK: [[next_phi1_0:%\w+]] = OpFSub %float %float_1 %float_0
; CHECK: OpFMul %float %float_1
; Second loop iteration
; CHECK: [[next_phi1_1:%\w+]] = OpFSub %float [[next_phi1_0]] %float_1
; CHECK: OpFMul %float %float_0
; Third loop iteration
; CHECK: OpFSub %float [[next_phi1_1]] [[next_phi1_0]]
; CHECK: OpFMul %float %float_1
%21 = OpFSub %float %20 %18
%27 = OpFMul %float %17 %21
OpBranch %19
%19 = OpLabel
%23 = OpIAdd %uint %22 %uint_1
OpBranch %16
%25 = OpLabel
OpReturn
OpFunctionEnd
)";
std::unique_ptr<IRContext> context =
BuildModule(SPV_ENV_UNIVERSAL_1_1, nullptr, text,
SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
Module* module = context->module();
EXPECT_NE(nullptr, module) << "Assembling failed for ushader:\n"
<< text << std::endl;
LoopUnroller loop_unroller;
SetDisassembleOptions(SPV_BINARY_TO_TEXT_OPTION_NO_HEADER |
SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES);
SinglePassRunAndMatch<LoopUnroller>(text, true);
}
TEST_F(PassClassTest, PartialUnrollWithPhiReferencesPhi) {
// With LocalMultiStoreElimPass
const std::string text = R"(
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main" %color
OpExecutionMode %main OriginUpperLeft
OpSource HLSL 600
OpName %main "main"
OpName %color "color"
OpDecorate %color Location 0
%uint = OpTypeInt 32 0
%float = OpTypeFloat 32
%float_0 = OpConstant %float 0
%float_1 = OpConstant %float 1
%uint_1 = OpConstant %uint 1
%uint_3 = OpConstant %uint 3
%void = OpTypeVoid
%11 = OpTypeFunction %void
%bool = OpTypeBool
%v4float = OpTypeVector %float 4
%_ptr_Output_v4float = OpTypePointer Output %v4float
%color = OpVariable %_ptr_Output_v4float Output
%main = OpFunction %void None %11
%15 = OpLabel
OpBranch %16
%16 = OpLabel
%17 = OpPhi %float %float_0 %15 %18 %19
%18 = OpPhi %float %float_1 %15 %20 %19
%21 = OpPhi %uint %uint_1 %15 %22 %19
%23 = OpULessThanEqual %bool %21 %uint_3
OpLoopMerge %24 %19 Unroll
OpBranchConditional %23 %25 %24
%25 = OpLabel
; CHECK: [[phi0_0:%\w+]] = OpPhi {{%\w+}} {{%\w+}} {{%\w+}} [[phi1_0:%\w+]]
; CHECK: [[phi1_0]] = OpPhi {{%\w+}} {{%\w+}} {{%\w+}} [[sub:%\w+]]
; CHECK: [[sub]] = OpFSub {{%\w+}} [[phi1_0]] [[phi0_0]]
; CHECK: [[phi0_1:%\w+]] = OpPhi {{%\w+}} [[phi0_0]]
; CHECK: [[phi1_1:%\w+]] = OpPhi {{%\w+}} [[phi1_0]]
; CHECK: [[sub:%\w+]] = OpFSub {{%\w+}} [[phi1_1]] [[phi0_1]]
; CHECK: OpFSub {{%\w+}} [[sub]] [[phi1_1]]
%20 = OpFSub %float %18 %17
OpBranch %19
%19 = OpLabel
%22 = OpIAdd %uint %21 %uint_1
OpBranch %16
%24 = OpLabel
OpReturn
OpFunctionEnd
)";
std::unique_ptr<IRContext> context =
BuildModule(SPV_ENV_UNIVERSAL_1_1, nullptr, text,
SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
Module* module = context->module();
EXPECT_NE(nullptr, module) << "Assembling failed for ushader:\n"
<< text << std::endl;
LoopUnroller loop_unroller;
SetDisassembleOptions(SPV_BINARY_TO_TEXT_OPTION_NO_HEADER);
SinglePassRunAndMatch<PartialUnrollerTestPass<2>>(text, true);
}
} // namespace
} // namespace opt
} // namespace spvtools