[llvm] 72ec2c0 - [InstCombine] Fix handling of irreducible loops (PR64259)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 31 07:20:30 PDT 2023
Author: Nikita Popov
Date: 2023-07-31T16:20:22+02:00
New Revision: 72ec2c007e4c5f46995c8f633a946cfa43da6bfb
URL: https://github.com/llvm/llvm-project/commit/72ec2c007e4c5f46995c8f633a946cfa43da6bfb
DIFF: https://github.com/llvm/llvm-project/commit/72ec2c007e4c5f46995c8f633a946cfa43da6bfb.diff
LOG: [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 https://github.com/llvm/llvm-project/issues/64259.
Added:
Modified:
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
llvm/test/Transforms/InstCombine/pr38677.ll
llvm/test/Transforms/InstCombine/unreachable-code.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index d2915bcc8c37c4..beea3dd259a808 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -4122,15 +4122,24 @@ bool InstCombinerImpl::prepareWorklist(
Function &F, ReversePostOrderTraversal<BasicBlock *> &RPOT) {
bool MadeIRChange = false;
SmallPtrSet<BasicBlock *, 32> LiveBlocks;
- LiveBlocks.insert(&F.front());
-
SmallVector<Instruction *, 128> InstrsForInstructionWorklist;
DenseMap<Constant *, Constant *> FoldedConstants;
AliasScopeTracker SeenAliasScopes;
+ auto HandleOnlyLiveSuccessor = [&](BasicBlock *BB, BasicBlock *LiveSucc) {
+ for (BasicBlock *Succ : successors(BB))
+ if (Succ != LiveSucc)
+ DeadEdges.insert({BB, Succ});
+ };
+
for (BasicBlock *BB : RPOT) {
- if (!LiveBlocks.count(BB))
+ if (!BB->isEntryBlock() && all_of(predecessors(BB), [&](BasicBlock *Pred) {
+ return DeadEdges.contains({Pred, BB}) || DT.dominates(BB, Pred);
+ })) {
+ HandleOnlyLiveSuccessor(BB, nullptr);
continue;
+ }
+ LiveBlocks.insert(BB);
for (Instruction &Inst : llvm::make_early_inc_range(*BB)) {
// ConstantProp instruction if trivially constant.
@@ -4179,27 +4188,28 @@ bool InstCombinerImpl::prepareWorklist(
// live successor. Otherwise assume all successors are live.
Instruction *TI = BB->getTerminator();
if (BranchInst *BI = dyn_cast<BranchInst>(TI); BI && BI->isConditional()) {
- if (isa<UndefValue>(BI->getCondition()))
+ if (isa<UndefValue>(BI->getCondition())) {
// Branch on undef is UB.
+ HandleOnlyLiveSuccessor(BB, nullptr);
continue;
+ }
if (auto *Cond = dyn_cast<ConstantInt>(BI->getCondition())) {
bool CondVal = Cond->getZExtValue();
- BasicBlock *ReachableBB = BI->getSuccessor(!CondVal);
- LiveBlocks.insert(ReachableBB);
+ HandleOnlyLiveSuccessor(BB, BI->getSuccessor(!CondVal));
continue;
}
} else if (SwitchInst *SI = dyn_cast<SwitchInst>(TI)) {
- if (isa<UndefValue>(SI->getCondition()))
+ if (isa<UndefValue>(SI->getCondition())) {
// Switch on undef is UB.
+ HandleOnlyLiveSuccessor(BB, nullptr);
continue;
+ }
if (auto *Cond = dyn_cast<ConstantInt>(SI->getCondition())) {
- LiveBlocks.insert(SI->findCaseValue(Cond)->getCaseSuccessor());
+ HandleOnlyLiveSuccessor(BB,
+ SI->findCaseValue(Cond)->getCaseSuccessor());
continue;
}
}
-
- for (BasicBlock *SuccBB : successors(TI))
- LiveBlocks.insert(SuccBB);
}
// Remove instructions inside unreachable blocks. This prevents the
diff --git a/llvm/test/Transforms/InstCombine/pr38677.ll b/llvm/test/Transforms/InstCombine/pr38677.ll
index f60c7fb209f219..acd9236cd2ed63 100644
--- a/llvm/test/Transforms/InstCombine/pr38677.ll
+++ b/llvm/test/Transforms/InstCombine/pr38677.ll
@@ -1,5 +1,8 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-;RUN: opt -passes=instcombine -S %s | FileCheck %s
+; RUN: opt -passes='instcombine<no-verify-fixpoint>' -S %s | FileCheck %s
+
+; FIXME: Does not reach fix point because dead phi operands are not replaced
+; with poison during worklist population.
@A = extern_weak global i32, align 4
@B = extern_weak global i32, align 4
@@ -9,10 +12,12 @@ define i32 @foo(i1 %which, ptr %dst) {
; CHECK-NEXT: entry:
; CHECK-NEXT: br i1 true, label [[FINAL:%.*]], label [[DELAY:%.*]]
; CHECK: delay:
+; CHECK-NEXT: [[TMP0:%.*]] = select i1 icmp eq (ptr @A, ptr @B), i32 2, i32 1
; CHECK-NEXT: br label [[FINAL]]
; CHECK: final:
+; CHECK-NEXT: [[USE2:%.*]] = phi i32 [ 1, [[ENTRY:%.*]] ], [ [[TMP0]], [[DELAY]] ]
; CHECK-NEXT: store i1 false, ptr [[DST:%.*]], align 1
-; CHECK-NEXT: ret i32 1
+; CHECK-NEXT: ret i32 [[USE2]]
;
entry:
br i1 true, label %final, label %delay
diff --git a/llvm/test/Transforms/InstCombine/unreachable-code.ll b/llvm/test/Transforms/InstCombine/unreachable-code.ll
index 7c37690bc9808a..6a05a2804bf6df 100644
--- a/llvm/test/Transforms/InstCombine/unreachable-code.ll
+++ b/llvm/test/Transforms/InstCombine/unreachable-code.ll
@@ -392,6 +392,79 @@ exit:
ret void
}
+define void @irreducible() {
+; CHECK-LABEL: define void @irreducible() {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br i1 false, label [[LOOP2:%.*]], label [[LOOP1:%.*]]
+; CHECK: loop1:
+; CHECK-NEXT: call void @dummy()
+; CHECK-NEXT: br label [[LOOP2]]
+; CHECK: loop2:
+; CHECK-NEXT: call void @dummy()
+; CHECK-NEXT: br i1 true, label [[EXIT:%.*]], label [[LOOP1]]
+; CHECK: exit:
+; CHECK-NEXT: call void @dummy()
+; CHECK-NEXT: ret void
+;
+entry:
+ br i1 false, label %loop2, label %loop1
+
+loop1:
+ call void @dummy()
+ br label %loop2
+
+loop2:
+ call void @dummy()
+ br i1 true, label %exit, label %loop1
+
+exit:
+ call void @dummy()
+ ret void
+}
+
+define void @really_unreachable() {
+; CHECK-LABEL: define void @really_unreachable() {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: ret void
+; CHECK: unreachable:
+; CHECK-NEXT: ret void
+;
+entry:
+ ret void
+
+unreachable:
+ call void @dummy()
+ ret void
+}
+
+define void @really_unreachable_predecessor() {
+; CHECK-LABEL: define void @really_unreachable_predecessor() {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br i1 false, label [[BB:%.*]], label [[EXIT:%.*]]
+; CHECK: unreachable:
+; CHECK-NEXT: br label [[BB]]
+; CHECK: bb:
+; CHECK-NEXT: ret void
+; CHECK: exit:
+; CHECK-NEXT: call void @dummy()
+; CHECK-NEXT: ret void
+;
+entry:
+ br i1 false, label %bb, label %exit
+
+unreachable:
+ call void @dummy()
+ br label %bb
+
+bb:
+ call void @dummy()
+ ret void
+
+exit:
+ call void @dummy()
+ ret void
+}
+
;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
; DEFAULT_ITER: {{.*}}
; MAX1: {{.*}}
More information about the llvm-commits
mailing list