Skip to content

Commit 72ec2c0

Browse files
committed
[InstCombine] Fix handling of irreducible loops (PR64259)
Fixes a regression introduced by D75362 for irreducible control flow. In that case, we may visit the predecessor that renders the current block live only later, and incorrectly determine that a block is dead. Instead, switch to using the same DeadEdges based implementation we also use during the main InstCombine iteration. This temporarily regresses some cases that need replacement of dead phi operands with poison, which is currently only done during the main run, but not worklist population. This will be addressed in a followup, to keep it separate from the correctness fix here. Fixes #64259.
1 parent 2cb6d0c commit 72ec2c0

File tree

3 files changed

+101
-13
lines changed

3 files changed

+101
-13
lines changed

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4122,15 +4122,24 @@ bool InstCombinerImpl::prepareWorklist(
41224122
Function &F, ReversePostOrderTraversal<BasicBlock *> &RPOT) {
41234123
bool MadeIRChange = false;
41244124
SmallPtrSet<BasicBlock *, 32> LiveBlocks;
4125-
LiveBlocks.insert(&F.front());
4126-
41274125
SmallVector<Instruction *, 128> InstrsForInstructionWorklist;
41284126
DenseMap<Constant *, Constant *> FoldedConstants;
41294127
AliasScopeTracker SeenAliasScopes;
41304128

4129+
auto HandleOnlyLiveSuccessor = [&](BasicBlock *BB, BasicBlock *LiveSucc) {
4130+
for (BasicBlock *Succ : successors(BB))
4131+
if (Succ != LiveSucc)
4132+
DeadEdges.insert({BB, Succ});
4133+
};
4134+
41314135
for (BasicBlock *BB : RPOT) {
4132-
if (!LiveBlocks.count(BB))
4136+
if (!BB->isEntryBlock() && all_of(predecessors(BB), [&](BasicBlock *Pred) {
4137+
return DeadEdges.contains({Pred, BB}) || DT.dominates(BB, Pred);
4138+
})) {
4139+
HandleOnlyLiveSuccessor(BB, nullptr);
41334140
continue;
4141+
}
4142+
LiveBlocks.insert(BB);
41344143

41354144
for (Instruction &Inst : llvm::make_early_inc_range(*BB)) {
41364145
// ConstantProp instruction if trivially constant.
@@ -4179,27 +4188,28 @@ bool InstCombinerImpl::prepareWorklist(
41794188
// live successor. Otherwise assume all successors are live.
41804189
Instruction *TI = BB->getTerminator();
41814190
if (BranchInst *BI = dyn_cast<BranchInst>(TI); BI && BI->isConditional()) {
4182-
if (isa<UndefValue>(BI->getCondition()))
4191+
if (isa<UndefValue>(BI->getCondition())) {
41834192
// Branch on undef is UB.
4193+
HandleOnlyLiveSuccessor(BB, nullptr);
41844194
continue;
4195+
}
41854196
if (auto *Cond = dyn_cast<ConstantInt>(BI->getCondition())) {
41864197
bool CondVal = Cond->getZExtValue();
4187-
BasicBlock *ReachableBB = BI->getSuccessor(!CondVal);
4188-
LiveBlocks.insert(ReachableBB);
4198+
HandleOnlyLiveSuccessor(BB, BI->getSuccessor(!CondVal));
41894199
continue;
41904200
}
41914201
} else if (SwitchInst *SI = dyn_cast<SwitchInst>(TI)) {
4192-
if (isa<UndefValue>(SI->getCondition()))
4202+
if (isa<UndefValue>(SI->getCondition())) {
41934203
// Switch on undef is UB.
4204+
HandleOnlyLiveSuccessor(BB, nullptr);
41944205
continue;
4206+
}
41954207
if (auto *Cond = dyn_cast<ConstantInt>(SI->getCondition())) {
4196-
LiveBlocks.insert(SI->findCaseValue(Cond)->getCaseSuccessor());
4208+
HandleOnlyLiveSuccessor(BB,
4209+
SI->findCaseValue(Cond)->getCaseSuccessor());
41974210
continue;
41984211
}
41994212
}
4200-
4201-
for (BasicBlock *SuccBB : successors(TI))
4202-
LiveBlocks.insert(SuccBB);
42034213
}
42044214

42054215
// Remove instructions inside unreachable blocks. This prevents the

llvm/test/Transforms/InstCombine/pr38677.ll

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
2-
;RUN: opt -passes=instcombine -S %s | FileCheck %s
2+
; RUN: opt -passes='instcombine<no-verify-fixpoint>' -S %s | FileCheck %s
3+
4+
; FIXME: Does not reach fix point because dead phi operands are not replaced
5+
; with poison during worklist population.
36

47
@A = extern_weak global i32, align 4
58
@B = extern_weak global i32, align 4
@@ -9,10 +12,12 @@ define i32 @foo(i1 %which, ptr %dst) {
912
; CHECK-NEXT: entry:
1013
; CHECK-NEXT: br i1 true, label [[FINAL:%.*]], label [[DELAY:%.*]]
1114
; CHECK: delay:
15+
; CHECK-NEXT: [[TMP0:%.*]] = select i1 icmp eq (ptr @A, ptr @B), i32 2, i32 1
1216
; CHECK-NEXT: br label [[FINAL]]
1317
; CHECK: final:
18+
; CHECK-NEXT: [[USE2:%.*]] = phi i32 [ 1, [[ENTRY:%.*]] ], [ [[TMP0]], [[DELAY]] ]
1419
; CHECK-NEXT: store i1 false, ptr [[DST:%.*]], align 1
15-
; CHECK-NEXT: ret i32 1
20+
; CHECK-NEXT: ret i32 [[USE2]]
1621
;
1722
entry:
1823
br i1 true, label %final, label %delay

llvm/test/Transforms/InstCombine/unreachable-code.ll

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,79 @@ exit:
392392
ret void
393393
}
394394

395+
define void @irreducible() {
396+
; CHECK-LABEL: define void @irreducible() {
397+
; CHECK-NEXT: entry:
398+
; CHECK-NEXT: br i1 false, label [[LOOP2:%.*]], label [[LOOP1:%.*]]
399+
; CHECK: loop1:
400+
; CHECK-NEXT: call void @dummy()
401+
; CHECK-NEXT: br label [[LOOP2]]
402+
; CHECK: loop2:
403+
; CHECK-NEXT: call void @dummy()
404+
; CHECK-NEXT: br i1 true, label [[EXIT:%.*]], label [[LOOP1]]
405+
; CHECK: exit:
406+
; CHECK-NEXT: call void @dummy()
407+
; CHECK-NEXT: ret void
408+
;
409+
entry:
410+
br i1 false, label %loop2, label %loop1
411+
412+
loop1:
413+
call void @dummy()
414+
br label %loop2
415+
416+
loop2:
417+
call void @dummy()
418+
br i1 true, label %exit, label %loop1
419+
420+
exit:
421+
call void @dummy()
422+
ret void
423+
}
424+
425+
define void @really_unreachable() {
426+
; CHECK-LABEL: define void @really_unreachable() {
427+
; CHECK-NEXT: entry:
428+
; CHECK-NEXT: ret void
429+
; CHECK: unreachable:
430+
; CHECK-NEXT: ret void
431+
;
432+
entry:
433+
ret void
434+
435+
unreachable:
436+
call void @dummy()
437+
ret void
438+
}
439+
440+
define void @really_unreachable_predecessor() {
441+
; CHECK-LABEL: define void @really_unreachable_predecessor() {
442+
; CHECK-NEXT: entry:
443+
; CHECK-NEXT: br i1 false, label [[BB:%.*]], label [[EXIT:%.*]]
444+
; CHECK: unreachable:
445+
; CHECK-NEXT: br label [[BB]]
446+
; CHECK: bb:
447+
; CHECK-NEXT: ret void
448+
; CHECK: exit:
449+
; CHECK-NEXT: call void @dummy()
450+
; CHECK-NEXT: ret void
451+
;
452+
entry:
453+
br i1 false, label %bb, label %exit
454+
455+
unreachable:
456+
call void @dummy()
457+
br label %bb
458+
459+
bb:
460+
call void @dummy()
461+
ret void
462+
463+
exit:
464+
call void @dummy()
465+
ret void
466+
}
467+
395468
;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
396469
; DEFAULT_ITER: {{.*}}
397470
; MAX1: {{.*}}

0 commit comments

Comments
 (0)