Skip to content

Commit 5fb5713

Browse files
UsmanNadeemUsmanNadeem
authored
[DFAJumpThreading] Don't bail early after encountering unpredictable values (#119774)
After #96127 landed, mshockwave reported that the pass was no longer threading SPEC2006/perlbench. After 96127 we started bailing out in `getStateDefMap` and rejecting the transformation because one of the unpredictable values was coming from inside the loop. There was no fundamental change in that function except that we started calling `Loop->contains(IncomingBB)` instead of `LoopBBs.count(IncomingBB)`. After some analysis I came to the conclusion that even before 96127 we would reject the transformation if we provided large enough limits on the path traversal (large enough so that LoopBBs contained blocks corresponding to that unpredictable value). In this patch I changed `getStateDefMap` to not terminate early on finding an unpredictable value, this is because `getPathsFromStateDefMap`, later, actually has checks to ensure that the final list of paths only have predictable values. As a result we can now partially thread functions like `negative6` in the tests that have some predictable paths. This patch does not really have any compile-time impact on the test suite without `-dfa-early-exit-heuristic=false` (early exit is enabled by default). Change-Id: Ie1633b370ed4a0eda8dea52650b40f6f66ef49a3
1 parent ae435ad commit 5fb5713

File tree

3 files changed

+87
-24
lines changed

3 files changed

+87
-24
lines changed

‎llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ static cl::opt<unsigned> MaxNumVisitiedPaths(
109109
"dfa-max-num-visited-paths",
110110
cl::desc(
111111
"Max number of blocks visited while enumerating paths around a switch"),
112-
cl::Hidden, cl::init(2000));
112+
cl::Hidden, cl::init(2500));
113113

114114
static cl::opt<unsigned>
115115
MaxNumPaths("dfa-max-num-paths",
@@ -754,17 +754,15 @@ struct AllSwitchPaths {
754754
return Res;
755755
}
756756

757-
/// Walk the use-def chain and collect all the state-defining instructions.
758-
///
759-
/// Return an empty map if unpredictable values encountered inside the basic
760-
/// blocks of \p LoopPaths.
757+
/// Walk the use-def chain and collect all the state-defining blocks and the
758+
/// PHI nodes in those blocks that define the state.
761759
StateDefMap getStateDefMap() const {
762760
StateDefMap Res;
763-
Value *FirstDef = Switch->getOperand(0);
764-
assert(isa<PHINode>(FirstDef) && "The first definition must be a phi.");
761+
PHINode *FirstDef = dyn_cast<PHINode>(Switch->getOperand(0));
762+
assert(FirstDef && "The first definition must be a phi.");
765763

766764
SmallVector<PHINode *, 8> Stack;
767-
Stack.push_back(dyn_cast<PHINode>(FirstDef));
765+
Stack.push_back(FirstDef);
768766
SmallSet<Value *, 16> SeenValues;
769767

770768
while (!Stack.empty()) {
@@ -774,18 +772,15 @@ struct AllSwitchPaths {
774772
SeenValues.insert(CurPhi);
775773

776774
for (BasicBlock *IncomingBB : CurPhi->blocks()) {
777-
Value *Incoming = CurPhi->getIncomingValueForBlock(IncomingBB);
775+
PHINode *IncomingPhi =
776+
dyn_cast<PHINode>(CurPhi->getIncomingValueForBlock(IncomingBB));
777+
if (!IncomingPhi)
778+
continue;
778779
bool IsOutsideLoops = !SwitchOuterLoop->contains(IncomingBB);
779-
if (Incoming == FirstDef || isa<ConstantInt>(Incoming) ||
780-
SeenValues.contains(Incoming) || IsOutsideLoops) {
780+
if (SeenValues.contains(IncomingPhi) || IsOutsideLoops)
781781
continue;
782-
}
783-
784-
// Any unpredictable value inside the loops means we must bail out.
785-
if (!isa<PHINode>(Incoming))
786-
return StateDefMap();
787782

788-
Stack.push_back(cast<PHINode>(Incoming));
783+
Stack.push_back(IncomingPhi);
789784
}
790785
}
791786

‎llvm/test/Transforms/DFAJumpThreading/dfa-unfold-select.ll

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -381,26 +381,58 @@ define void @pr65222(i32 %flags, i1 %cmp, i1 %tobool.not) {
381381
; CHECK: then:
382382
; CHECK-NEXT: br i1 [[TOBOOL_NOT:%.*]], label [[COND1_SI_UNFOLD_TRUE:%.*]], label [[COND_SI_UNFOLD_TRUE:%.*]]
383383
; CHECK: cond.si.unfold.true:
384+
; CHECK-NEXT: br i1 [[CMP]], label [[TOUNFOLD_SI_UNFOLD_FALSE1:%.*]], label [[COND_SI_UNFOLD_FALSE_JT0:%.*]]
385+
; CHECK: cond.si.unfold.true.jt2:
384386
; CHECK-NEXT: [[DOTSI_UNFOLD_PHI:%.*]] = phi i32 [ 2, [[THEN]] ]
385387
; CHECK-NEXT: br i1 [[CMP]], label [[TOUNFOLD_SI_UNFOLD_FALSE:%.*]], label [[COND_SI_UNFOLD_FALSE:%.*]]
386388
; CHECK: cond.si.unfold.false:
387389
; CHECK-NEXT: [[DOTSI_UNFOLD_PHI1:%.*]] = phi i32 [ 0, [[COND_SI_UNFOLD_TRUE]] ]
388-
; CHECK-NEXT: br label [[TOUNFOLD_SI_UNFOLD_FALSE]]
390+
; CHECK-NEXT: br label [[TOUNFOLD_SI_UNFOLD_FALSE1]]
391+
; CHECK: cond.si.unfold.false.jt0:
392+
; CHECK-NEXT: [[DOTSI_UNFOLD_PHI1_JT0:%.*]] = phi i32 [ 0, [[COND_SI_UNFOLD_TRUE1:%.*]] ]
393+
; CHECK-NEXT: br label [[TOUNFOLD_SI_UNFOLD_FALSE_JT0:%.*]]
389394
; CHECK: tounfold.si.unfold.false:
390-
; CHECK-NEXT: [[COND_SI_UNFOLD_PHI:%.*]] = phi i32 [ [[DOTSI_UNFOLD_PHI]], [[COND_SI_UNFOLD_TRUE]] ], [ [[DOTSI_UNFOLD_PHI1]], [[COND_SI_UNFOLD_FALSE]] ]
395+
; CHECK-NEXT: [[COND_SI_UNFOLD_PHI:%.*]] = phi i32 [ poison, [[COND_SI_UNFOLD_TRUE1]] ], [ [[DOTSI_UNFOLD_PHI1]], [[COND_SI_UNFOLD_FALSE]] ]
391396
; CHECK-NEXT: br label [[IF_END]]
397+
; CHECK: tounfold.si.unfold.false.jt0:
398+
; CHECK-NEXT: [[COND_SI_UNFOLD_PHI_JT0:%.*]] = phi i32 [ [[DOTSI_UNFOLD_PHI1_JT0]], [[COND_SI_UNFOLD_FALSE_JT0]] ]
399+
; CHECK-NEXT: br label [[IF_END_JT0:%.*]]
400+
; CHECK: tounfold.si.unfold.false.jt2:
401+
; CHECK-NEXT: [[COND_SI_UNFOLD_PHI_JT2:%.*]] = phi i32 [ [[DOTSI_UNFOLD_PHI]], [[COND_SI_UNFOLD_TRUE]] ]
402+
; CHECK-NEXT: br label [[IF_END_JT2:%.*]]
392403
; CHECK: cond1.si.unfold.true:
404+
; CHECK-NEXT: br i1 [[CMP]], label [[IF_END]], label [[COND1_SI_UNFOLD_FALSE_JT1:%.*]]
405+
; CHECK: cond1.si.unfold.true.jt3:
393406
; CHECK-NEXT: [[DOTSI_UNFOLD_PHI2:%.*]] = phi i32 [ 3, [[THEN]] ]
394-
; CHECK-NEXT: br i1 [[CMP]], label [[IF_END]], label [[COND1_SI_UNFOLD_FALSE:%.*]]
407+
; CHECK-NEXT: br i1 [[CMP]], label [[IF_END_JT3:%.*]], label [[COND1_SI_UNFOLD_FALSE:%.*]]
395408
; CHECK: cond1.si.unfold.false:
396409
; CHECK-NEXT: [[DOTSI_UNFOLD_PHI3:%.*]] = phi i32 [ 1, [[COND1_SI_UNFOLD_TRUE]] ]
397410
; CHECK-NEXT: br label [[IF_END]]
411+
; CHECK: cond1.si.unfold.false.jt1:
412+
; CHECK-NEXT: [[DOTSI_UNFOLD_PHI3_JT1:%.*]] = phi i32 [ 1, [[COND1_SI_UNFOLD_TRUE1:%.*]] ]
413+
; CHECK-NEXT: br label [[IF_END_JT1:%.*]]
398414
; CHECK: if.end:
399-
; CHECK-NEXT: [[UNFOLDED:%.*]] = phi i32 [ [[FLAGS:%.*]], [[WHILE_COND]] ], [ [[COND_SI_UNFOLD_PHI]], [[TOUNFOLD_SI_UNFOLD_FALSE]] ], [ [[DOTSI_UNFOLD_PHI2]], [[COND1_SI_UNFOLD_TRUE]] ], [ [[DOTSI_UNFOLD_PHI3]], [[COND1_SI_UNFOLD_FALSE]] ]
400-
; CHECK-NEXT: [[OTHER:%.*]] = phi i32 [ [[FLAGS]], [[WHILE_COND]] ], [ 0, [[TOUNFOLD_SI_UNFOLD_FALSE]] ], [ 0, [[COND1_SI_UNFOLD_TRUE]] ], [ 0, [[COND1_SI_UNFOLD_FALSE]] ]
415+
; CHECK-NEXT: [[UNFOLDED:%.*]] = phi i32 [ [[FLAGS:%.*]], [[WHILE_COND]] ], [ [[COND_SI_UNFOLD_PHI]], [[TOUNFOLD_SI_UNFOLD_FALSE1]] ], [ poison, [[COND1_SI_UNFOLD_TRUE1]] ], [ [[DOTSI_UNFOLD_PHI3]], [[COND1_SI_UNFOLD_FALSE]] ]
416+
; CHECK-NEXT: [[OTHER:%.*]] = phi i32 [ [[FLAGS]], [[WHILE_COND]] ], [ 0, [[TOUNFOLD_SI_UNFOLD_FALSE1]] ], [ 0, [[COND1_SI_UNFOLD_TRUE1]] ], [ 0, [[COND1_SI_UNFOLD_FALSE]] ]
401417
; CHECK-NEXT: switch i32 [[UNFOLDED]], label [[UNREACHABLE:%.*]] [
402418
; CHECK-NEXT: i32 0, label [[SW_BB:%.*]]
403419
; CHECK-NEXT: ]
420+
; CHECK: if.end.jt1:
421+
; CHECK-NEXT: [[UNFOLDED_JT1:%.*]] = phi i32 [ [[DOTSI_UNFOLD_PHI3_JT1]], [[COND1_SI_UNFOLD_FALSE_JT1]] ]
422+
; CHECK-NEXT: [[OTHER_JT1:%.*]] = phi i32 [ 0, [[COND1_SI_UNFOLD_FALSE_JT1]] ]
423+
; CHECK-NEXT: br label [[UNREACHABLE]]
424+
; CHECK: if.end.jt3:
425+
; CHECK-NEXT: [[UNFOLDED_JT3:%.*]] = phi i32 [ [[DOTSI_UNFOLD_PHI2]], [[COND1_SI_UNFOLD_TRUE]] ]
426+
; CHECK-NEXT: [[OTHER_JT3:%.*]] = phi i32 [ 0, [[COND1_SI_UNFOLD_TRUE]] ]
427+
; CHECK-NEXT: br label [[UNREACHABLE]]
428+
; CHECK: if.end.jt0:
429+
; CHECK-NEXT: [[UNFOLDED_JT0:%.*]] = phi i32 [ [[COND_SI_UNFOLD_PHI_JT0]], [[TOUNFOLD_SI_UNFOLD_FALSE_JT0]] ]
430+
; CHECK-NEXT: [[OTHER_JT0:%.*]] = phi i32 [ 0, [[TOUNFOLD_SI_UNFOLD_FALSE_JT0]] ]
431+
; CHECK-NEXT: br label [[SW_BB]]
432+
; CHECK: if.end.jt2:
433+
; CHECK-NEXT: [[UNFOLDED_JT2:%.*]] = phi i32 [ [[COND_SI_UNFOLD_PHI_JT2]], [[TOUNFOLD_SI_UNFOLD_FALSE]] ]
434+
; CHECK-NEXT: [[OTHER_JT2:%.*]] = phi i32 [ 0, [[TOUNFOLD_SI_UNFOLD_FALSE]] ]
435+
; CHECK-NEXT: br label [[UNREACHABLE]]
404436
; CHECK: unreachable:
405437
; CHECK-NEXT: unreachable
406438
; CHECK: sw.bb:

‎llvm/test/Transforms/DFAJumpThreading/negative.ll

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,9 +218,45 @@ for.end:
218218
declare i32 @arbitrary_function()
219219

220220
; Don't confuse %state.2 for the initial switch value.
221+
; [ 3, %case2 ] can still be threaded.
221222
define i32 @negative6(i32 %init) {
222-
; REMARK: SwitchNotPredictable
223-
; REMARK-NEXT: negative6
223+
; CHECK-LABEL: define i32 @negative6(
224+
; CHECK-SAME: i32 [[INIT:%.*]]) {
225+
; CHECK-NEXT: [[ENTRY:.*:]]
226+
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[INIT]], 0
227+
; CHECK-NEXT: br label %[[LOOP_2:.*]]
228+
; CHECK: [[LOOP_2]]:
229+
; CHECK-NEXT: [[STATE_2:%.*]] = call i32 @arbitrary_function()
230+
; CHECK-NEXT: br label %[[LOOP_3:.*]]
231+
; CHECK: [[LOOP_3]]:
232+
; CHECK-NEXT: [[STATE:%.*]] = phi i32 [ [[STATE_2]], %[[LOOP_2]] ]
233+
; CHECK-NEXT: switch i32 [[STATE]], label %[[INFLOOP_I:.*]] [
234+
; CHECK-NEXT: i32 2, label %[[CASE2:.*]]
235+
; CHECK-NEXT: i32 3, label %[[CASE3:.*]]
236+
; CHECK-NEXT: i32 4, label %[[CASE4:.*]]
237+
; CHECK-NEXT: i32 0, label %[[CASE0:.*]]
238+
; CHECK-NEXT: i32 1, label %[[CASE1:.*]]
239+
; CHECK-NEXT: ]
240+
; CHECK: [[LOOP_3_JT3:.*]]:
241+
; CHECK-NEXT: [[STATE_JT3:%.*]] = phi i32 [ 3, %[[CASE2]] ]
242+
; CHECK-NEXT: br label %[[CASE3]]
243+
; CHECK: [[CASE2]]:
244+
; CHECK-NEXT: br label %[[LOOP_3_JT3]]
245+
; CHECK: [[CASE3]]:
246+
; CHECK-NEXT: br i1 [[CMP]], label %[[LOOP_2_BACKEDGE:.*]], label %[[CASE4]]
247+
; CHECK: [[CASE4]]:
248+
; CHECK-NEXT: br label %[[LOOP_2_BACKEDGE]]
249+
; CHECK: [[LOOP_2_BACKEDGE]]:
250+
; CHECK-NEXT: br label %[[LOOP_2]]
251+
; CHECK: [[CASE0]]:
252+
; CHECK-NEXT: br label %[[EXIT:.*]]
253+
; CHECK: [[CASE1]]:
254+
; CHECK-NEXT: br label %[[EXIT]]
255+
; CHECK: [[INFLOOP_I]]:
256+
; CHECK-NEXT: br label %[[INFLOOP_I]]
257+
; CHECK: [[EXIT]]:
258+
; CHECK-NEXT: ret i32 0
259+
;
224260
entry:
225261
%cmp = icmp eq i32 %init, 0
226262
br label %loop.2

0 commit comments

Comments
 (0)