[llvm] ad7f020 - [InstCombine] Process blocks in RPO
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Sun Jul 30 09:39:00 PDT 2023
Author: Nikita Popov
Date: 2023-07-30T18:38:45+02:00
New Revision: ad7f02010f32bff28fec139e103ad0240e160aa9
URL: https://github.com/llvm/llvm-project/commit/ad7f02010f32bff28fec139e103ad0240e160aa9
DIFF: https://github.com/llvm/llvm-project/commit/ad7f02010f32bff28fec139e103ad0240e160aa9.diff
LOG: [InstCombine] Process blocks in RPO
InstComine currently processes blocks in an arbitrary
depth-first order. This can break the usual invariant that the
operands of an instruction should be simplified before the
instruction itself, if uses across basic blocks (particularly
inside phi nodes) are involved.
This patch switches the initial worklist population to use RPO
instead, which will ensure that predecessors are visited before
successors (back-edges notwithstanding).
This allows us to fold more cases within a single InstCombine
iteration, in preparation for D154579. This change by itself
is a minor compile-time regression of about 0.1%, which will
be more than recovered by switching to single-iteration InstCombine.
Differential Revision: https://reviews.llvm.org/D75362
Added:
Modified:
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
llvm/test/Transforms/InstCombine/oss_fuzz_32759.ll
llvm/test/Transforms/InstCombine/pr44245.ll
llvm/test/Transforms/InstCombine/select.ll
llvm/test/Transforms/InstCombine/store.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 4e208703bac652..13ac11f3c12499 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -36,6 +36,7 @@
#include "llvm/ADT/APInt.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/PostOrderIterator.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/Statistic.h"
@@ -4108,31 +4109,29 @@ class AliasScopeTracker {
}
};
-/// Populate the IC worklist from a function, by walking it in depth-first
-/// order and adding all reachable code to the worklist.
+/// Populate the IC worklist from a function, by walking it in reverse
+/// post-order and adding all reachable code to the worklist.
///
/// This has a couple of tricks to make the code faster and more powerful. In
/// particular, we constant fold and DCE instructions as we go, to avoid adding
/// them to the worklist (this significantly speeds up instcombine on code where
/// many instructions are dead or constant). Additionally, if we find a branch
/// whose condition is a known constant, we only visit the reachable successors.
-static bool prepareICWorklistFromFunction(Function &F, const DataLayout &DL,
- const TargetLibraryInfo *TLI,
- InstructionWorklist &ICWorklist) {
+static bool
+prepareICWorklistFromFunction(Function &F, const DataLayout &DL,
+ const TargetLibraryInfo *TLI,
+ InstructionWorklist &ICWorklist,
+ ReversePostOrderTraversal<BasicBlock *> &RPOT) {
bool MadeIRChange = false;
- SmallPtrSet<BasicBlock *, 32> Visited;
- SmallVector<BasicBlock*, 256> Worklist;
- Worklist.push_back(&F.front());
+ SmallPtrSet<BasicBlock *, 32> LiveBlocks;
+ LiveBlocks.insert(&F.front());
SmallVector<Instruction *, 128> InstrsForInstructionWorklist;
DenseMap<Constant *, Constant *> FoldedConstants;
AliasScopeTracker SeenAliasScopes;
- do {
- BasicBlock *BB = Worklist.pop_back_val();
-
- // We have now visited this block! If we've already been here, ignore it.
- if (!Visited.insert(BB).second)
+ for (BasicBlock *BB : RPOT) {
+ if (!LiveBlocks.count(BB))
continue;
for (Instruction &Inst : llvm::make_early_inc_range(*BB)) {
@@ -4178,8 +4177,8 @@ static bool prepareICWorklistFromFunction(Function &F, const DataLayout &DL,
}
}
- // Recursively visit successors. If this is a branch or switch on a
- // constant, only visit the reachable successor.
+ // If this is a branch or switch on a constant, mark only the single
+ // 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()))
@@ -4188,7 +4187,7 @@ static bool prepareICWorklistFromFunction(Function &F, const DataLayout &DL,
if (auto *Cond = dyn_cast<ConstantInt>(BI->getCondition())) {
bool CondVal = Cond->getZExtValue();
BasicBlock *ReachableBB = BI->getSuccessor(!CondVal);
- Worklist.push_back(ReachableBB);
+ LiveBlocks.insert(ReachableBB);
continue;
}
} else if (SwitchInst *SI = dyn_cast<SwitchInst>(TI)) {
@@ -4196,19 +4195,20 @@ static bool prepareICWorklistFromFunction(Function &F, const DataLayout &DL,
// Switch on undef is UB.
continue;
if (auto *Cond = dyn_cast<ConstantInt>(SI->getCondition())) {
- Worklist.push_back(SI->findCaseValue(Cond)->getCaseSuccessor());
+ LiveBlocks.insert(SI->findCaseValue(Cond)->getCaseSuccessor());
continue;
}
}
- append_range(Worklist, successors(TI));
- } while (!Worklist.empty());
+ for (BasicBlock *SuccBB : successors(TI))
+ LiveBlocks.insert(SuccBB);
+ }
// Remove instructions inside unreachable blocks. This prevents the
// instcombine code from having to deal with some bad special cases, and
// reduces use counts of instructions.
for (BasicBlock &BB : F) {
- if (Visited.count(&BB))
+ if (LiveBlocks.count(&BB))
continue;
unsigned NumDeadInstInBB;
@@ -4262,6 +4262,8 @@ static bool combineInstructionsOverFunction(
AC.registerAssumption(Assume);
}));
+ ReversePostOrderTraversal<BasicBlock *> RPOT(&F.front());
+
// Lower dbg.declare intrinsics otherwise their value may be clobbered
// by instcombiner.
bool MadeIRChange = false;
@@ -4290,7 +4292,7 @@ static bool combineInstructionsOverFunction(
LLVM_DEBUG(dbgs() << "\n\nINSTCOMBINE ITERATION #" << Iteration << " on "
<< F.getName() << "\n");
- MadeIRChange |= prepareICWorklistFromFunction(F, DL, &TLI, Worklist);
+ MadeIRChange |= prepareICWorklistFromFunction(F, DL, &TLI, Worklist, RPOT);
InstCombinerImpl IC(Worklist, Builder, F.hasMinSize(), AA, AC, TLI, TTI, DT,
ORE, BFI, PSI, DL, LI);
diff --git a/llvm/test/Transforms/InstCombine/oss_fuzz_32759.ll b/llvm/test/Transforms/InstCombine/oss_fuzz_32759.ll
index 96a16751740c81..9f6db50d0c4bbb 100644
--- a/llvm/test/Transforms/InstCombine/oss_fuzz_32759.ll
+++ b/llvm/test/Transforms/InstCombine/oss_fuzz_32759.ll
@@ -9,7 +9,7 @@ define i1 @oss_fuzz_32759(i1 %y, i1 %c1) {
; CHECK: cond.true:
; CHECK-NEXT: br label [[END]]
; CHECK: end:
-; CHECK-NEXT: ret i1 [[C1]]
+; CHECK-NEXT: ret i1 false
;
entry:
br i1 %c1, label %cond.true, label %end
diff --git a/llvm/test/Transforms/InstCombine/pr44245.ll b/llvm/test/Transforms/InstCombine/pr44245.ll
index 895a38a4fe865a..c0daa2ec7197f0 100644
--- a/llvm/test/Transforms/InstCombine/pr44245.ll
+++ b/llvm/test/Transforms/InstCombine/pr44245.ll
@@ -157,9 +157,9 @@ define void @test_2(i1 %c) local_unnamed_addr {
; CHECK: cond.true133:
; CHECK-NEXT: br label [[COND_END144:%.*]]
; CHECK: cond.false138:
-; CHECK-NEXT: store i1 true, ptr poison, align 1
; CHECK-NEXT: br label [[COND_END144]]
; CHECK: cond.end144:
+; CHECK-NEXT: store i1 true, ptr poison, align 1
; CHECK-NEXT: br label [[WHILE_COND]]
;
entry:
diff --git a/llvm/test/Transforms/InstCombine/select.ll b/llvm/test/Transforms/InstCombine/select.ll
index fb65d4f3acae4f..a2cdb951d31bd0 100644
--- a/llvm/test/Transforms/InstCombine/select.ll
+++ b/llvm/test/Transforms/InstCombine/select.ll
@@ -960,7 +960,7 @@ define void @test64(i32 %p, i16 %b, i1 %c1) noreturn {
; CHECK: lor.rhs:
; CHECK-NEXT: br label [[LOR_END]]
; CHECK: lor.end:
-; CHECK-NEXT: br i1 true, label [[COND_END17:%.*]], label [[COND_FALSE16:%.*]]
+; CHECK-NEXT: br i1 poison, label [[COND_END17:%.*]], label [[COND_FALSE16:%.*]]
; CHECK: cond.false16:
; CHECK-NEXT: br label [[COND_END17]]
; CHECK: cond.end17:
diff --git a/llvm/test/Transforms/InstCombine/store.ll b/llvm/test/Transforms/InstCombine/store.ll
index 70f5d9a1209ae8..fab280b366f44c 100644
--- a/llvm/test/Transforms/InstCombine/store.ll
+++ b/llvm/test/Transforms/InstCombine/store.ll
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+; RUN: opt < %s -passes=instcombine -instcombine-infinite-loop-threshold=2 -S | FileCheck %s
; FIXME: This is technically incorrect because it might overwrite a poison
; value. Stop folding it once #52930 is resolved.
More information about the llvm-commits
mailing list