Skip to content

Commit 5b2201d

Browse files
authored
Fixed lifetime marker causing unroll to fail (#4830)
Fixed a bug where enabling lifetime markers causes unroll to fail. Running instcombine before loop unroll may cause unroll to fail, because instcombine may rewrite expressions in a way that makes ScalarEvolutions unable to evaluate. This change replaces the instcombine with simplify-inst, which serves the same purpose as the instcombine, but leaves the expression easy for ScalarEvolutions to evaluate. Also moved the original test for instcombine to a new folder with the new test, since instcombine pass is no longer being tested for that test. The problematic expression rewrite was as follows: ``` limit = (non_const_int_value % 2) ? 1 : 2; ``` Instcombine rewrites it to ``` limit = 2 - (non_const_int_value & 1); ``` Which stumped ScalarEvolutions.
1 parent 7d9567c commit 5b2201d

3 files changed

Lines changed: 36 additions & 1 deletion

File tree

lib/Transforms/IPO/PassManagerBuilder.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ static void addHLSLPasses(bool HLSLHighLevel, unsigned OptLevel, bool OnlyWarnOn
270270
// without interfering with HLSL-specific lowering.
271271
if (EnableLifetimeMarkers) {
272272
MPM.add(createSROAPass());
273-
MPM.add(createInstructionCombiningPass());
273+
MPM.add(createSimplifyInstPass());
274274
MPM.add(createJumpThreadingPass());
275275
}
276276
}

tools/clang/test/HLSLFileCheck/passes/llvm/instcombine/del_phi_self_reference.hlsl renamed to tools/clang/test/HLSLFileCheck/passes/llvm/simplifyinst/del_phi_self_reference.hlsl

File renamed without changes.
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// RUN: %dxc -T ps_6_6 %s -enable-lifetime-markers /HV 2021 | FileCheck %s
2+
// RUN: %dxc -T ps_6_6 %s /Od -enable-lifetime-markers /HV 2021 | FileCheck %s
3+
4+
// CHECK: @main
5+
6+
//
7+
// Regression check for loop unrolling failure. Running instcombine when
8+
// lifetime markers are enabled rewrote the IR in a way that ScalarEvolution
9+
// could no longer figure out an upper bound. The expression:
10+
//
11+
// limit = (l % 2) ? 1 : 2;
12+
//
13+
// Got rewritten to
14+
//
15+
// limit = 2 - (l & 1);
16+
//
17+
// This fix for this was switching from running instcombine to running
18+
// simplifyinst, which is less aggressive in its rewriting.
19+
//
20+
21+
cbuffer cb : register(b0) {
22+
float foo[2];
23+
};
24+
25+
[RootSignature("CBV(b0)")]
26+
float main(uint l : L) : SV_Target {
27+
uint limit = (l % 2) ? 1 : 2;
28+
float ret = 0;
29+
[unroll]
30+
for (uint i = 0; i < limit; i++) {
31+
ret += foo[i];
32+
}
33+
return ret;
34+
}
35+

0 commit comments

Comments
 (0)