Skip to content

Commit a4adb9a

Browse files
authored
[OPT] Delete decoration for OpPhi when unrolling (KhronosGroup#6064)
When unrolling, the decorations on an OpPhi in the header of the loop are treated like any other user outside of the loop. The id of the OpPhi is replaced by the value in the final iteration. This causes the decorations to be applied to this other value, which is not neccessarially correct. Contributes to microsoft/DirectXShaderCompiler#6914
1 parent 4b5584e commit a4adb9a

File tree

2 files changed

+46
-0
lines changed

2 files changed

+46
-0
lines changed

source/opt/loop_unroller.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,6 +560,10 @@ void LoopUnrollerUtilsImpl::ReplaceInductionUseWithFinalValue(Loop* loop) {
560560
loop->GetInductionVariables(inductions);
561561

562562
for (size_t index = 0; index < inductions.size(); ++index) {
563+
// We don't want the decorations that applied to the induction variable
564+
// to be applied to the value that replace it.
565+
context_->KillNamesAndDecorates(state_.previous_phis_[index]);
566+
563567
uint32_t trip_step_id = GetPhiDefID(state_.previous_phis_[index],
564568
state_.previous_latch_block_->id());
565569
context_->ReplaceAllUsesWith(inductions[index]->result_id(), trip_step_id);

test/opt/loop_optimizations/unroll_simple.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3788,6 +3788,48 @@ TEST_F(PassClassTest, PartialUnrollWithPhiReferencesPhi) {
37883788
SinglePassRunAndMatch<PartialUnrollerTestPass<2>>(text, true);
37893789
}
37903790

3791+
TEST_F(PassClassTest, UnrollWithDecorationOnPhi) {
3792+
// With LocalMultiStoreElimPass
3793+
const std::string text = R"(
3794+
OpCapability Shader
3795+
%1 = OpExtInstImport "GLSL.std.450"
3796+
OpMemoryModel Logical GLSL450
3797+
OpEntryPoint GLCompute %2 "main"
3798+
OpExecutionMode %2 LocalSize 16 16 1
3799+
; CHECK-NOT: OpDecorate {{%\w+}} RelaxedPrecision
3800+
OpDecorate %4 RelaxedPrecision
3801+
%float = OpTypeFloat 32
3802+
%float_0_000122070312 = OpConstant %float 0.000122070312
3803+
%int = OpTypeInt 32 1
3804+
%int_0 = OpConstant %int 0
3805+
%int_1 = OpConstant %int 1
3806+
%bool = OpTypeBool
3807+
%void = OpTypeVoid
3808+
%12 = OpTypeFunction %void
3809+
%2 = OpFunction %void None %12
3810+
%13 = OpLabel
3811+
OpBranch %14
3812+
%14 = OpLabel
3813+
%4 = OpPhi %float %float_0_000122070312 %13 %3 %15
3814+
%16 = OpPhi %int %int_0 %13 %17 %15
3815+
%18 = OpSLessThan %bool %16 %int_1
3816+
OpLoopMerge %19 %15 Unroll
3817+
OpBranchConditional %18 %15 %19
3818+
%15 = OpLabel
3819+
; CHECK: [[v:%\w+]] = OpExtInst %float
3820+
%3 = OpExtInst %float %1 NMax %float_0_000122070312 %float_0_000122070312
3821+
%17 = OpIAdd %int %16 %int_1
3822+
OpBranch %14
3823+
%19 = OpLabel
3824+
; CHECK: OpCopyObject %float [[v]]
3825+
%20 = OpCopyObject %float %4
3826+
OpReturn
3827+
OpFunctionEnd
3828+
)";
3829+
3830+
SinglePassRunAndMatch<LoopUnroller>(text, true);
3831+
}
3832+
37913833
TEST_F(PassClassTest, DontUnrollInfiteLoop) {
37923834
// This is an infinite loop that because the step is 0. We want to make sure
37933835
// the unroller does not try to unroll it.

0 commit comments

Comments
 (0)