[llvm] [SimplifyCFG] Fold `select` of equality comparison into switch predecessor (PR #79177)
Antonio Frighetto via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 24 10:15:53 PST 2024
https://github.com/antoniofrighetto updated https://github.com/llvm/llvm-project/pull/79177
>From 36e8967b785ce86e645334c1a5adc02f079f8f01 Mon Sep 17 00:00:00 2001
From: Antonio Frighetto <me at antoniofrighetto.com>
Date: Mon, 22 Jan 2024 11:21:54 +0100
Subject: [PATCH 1/2] [SimplifyCFG] Introduce test for PR79177 (NFC)
---
...itch-multiple-comparisons-consolidation.ll | 58 +++++++++++++++++++
1 file changed, 58 insertions(+)
create mode 100644 llvm/test/Transforms/SimplifyCFG/switch-multiple-comparisons-consolidation.ll
diff --git a/llvm/test/Transforms/SimplifyCFG/switch-multiple-comparisons-consolidation.ll b/llvm/test/Transforms/SimplifyCFG/switch-multiple-comparisons-consolidation.ll
new file mode 100644
index 000000000000000..0a873175939e111
--- /dev/null
+++ b/llvm/test/Transforms/SimplifyCFG/switch-multiple-comparisons-consolidation.ll
@@ -0,0 +1,58 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt < %s -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck %s
+
+define i1 @test(i32 %c) {
+; CHECK-LABEL: define i1 @test(
+; CHECK-SAME: i32 [[C:%.*]]) {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: switch i32 [[C]], label [[BB8:%.*]] [
+; CHECK-NEXT: i32 115, label [[BB9:%.*]]
+; CHECK-NEXT: i32 109, label [[BB9]]
+; CHECK-NEXT: i32 104, label [[BB9]]
+; CHECK-NEXT: ]
+; CHECK: bb8:
+; CHECK-NEXT: br label [[BB9]]
+; CHECK: bb9:
+; CHECK-NEXT: [[_3_0:%.*]] = phi i1 [ false, [[BB8]] ], [ true, [[ENTRY:%.*]] ], [ true, [[ENTRY]] ], [ true, [[ENTRY]] ]
+; CHECK-NEXT: [[_10:%.*]] = icmp eq i32 [[C]], 100
+; CHECK-NEXT: [[SPEC_SELECT1:%.*]] = select i1 [[_3_0]], i1 true, i1 [[_10]]
+; CHECK-NEXT: [[_12:%.*]] = icmp eq i32 [[C]], 119
+; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[SPEC_SELECT1]], i1 true, i1 [[_12]]
+; CHECK-NEXT: ret i1 [[SPEC_SELECT]]
+;
+entry:
+ %i1 = icmp eq i32 %c, 115
+ br i1 %i1, label %bb12, label %bb11
+
+bb11: ; preds = %entry
+ %_6 = icmp eq i32 %c, 109
+ br label %bb12
+
+bb12: ; preds = %entry, %bb11
+ %_4.0 = phi i1 [ %_6, %bb11 ], [ true, %entry ]
+ br i1 %_4.0, label %bb9, label %bb8
+
+bb8: ; preds = %bb12
+ %_8 = icmp eq i32 %c, 104
+ br label %bb9
+
+bb9: ; preds = %bb12, %bb8
+ %_3.0 = phi i1 [ %_8, %bb8 ], [ true, %bb12 ]
+ br i1 %_3.0, label %bb6, label %bb5
+
+bb5: ; preds = %bb9
+ %_10 = icmp eq i32 %c, 100
+ br label %bb6
+
+bb6: ; preds = %bb9, %bb5
+ %_2.0 = phi i1 [ %_10, %bb5 ], [ true, %bb9 ]
+ br i1 %_2.0, label %bb3, label %bb2
+
+bb2: ; preds = %bb6
+ %_12 = icmp eq i32 %c, 119
+ br label %bb3
+
+bb3: ; preds = %bb6, %bb2
+ %i.0 = phi i1 [ %_12, %bb2 ], [ true, %bb6 ]
+ ret i1 %i.0
+}
>From ef1828b89e0eda2c8634cd7b54713975a5e6efb8 Mon Sep 17 00:00:00 2001
From: Antonio Frighetto <me at antoniofrighetto.com>
Date: Mon, 22 Jan 2024 18:23:48 +0100
Subject: [PATCH 2/2] [SimplifyCFG] Fold `select` of equality cmp into switch
predecessors
When a conditional basic block is speculatively executed, the phi
operands of the fall-through block are rewritten with a `select`.
Revisit `FoldValueComparisonIntoPredecessors` to check whether
the condition of a branch may encompass a `select`, whose true
value is always constant true and false arm is equality comparison.
---
llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 130 ++++++++++++++----
...itch-multiple-comparisons-consolidation.ll | 7 +-
2 files changed, 104 insertions(+), 33 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 13eae549b2ce41b..6eb860fef0b4f48 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -254,7 +254,8 @@ class SimplifyCFGOpt {
Instruction *PTI,
IRBuilder<> &Builder);
bool FoldValueComparisonIntoPredecessors(Instruction *TI,
- IRBuilder<> &Builder);
+ IRBuilder<> &Builder,
+ Value *CV = nullptr);
bool simplifyResume(ResumeInst *RI, IRBuilder<> &Builder);
bool simplifySingleResume(ResumeInst *RI);
@@ -280,6 +281,8 @@ class SimplifyCFGOpt {
uint32_t TrueWeight, uint32_t FalseWeight);
bool SimplifyBranchOnICmpChain(BranchInst *BI, IRBuilder<> &Builder,
const DataLayout &DL);
+ bool SimplifyEqualityComparisonOnSelectIntoSwitchPredecessors(BranchInst *BI,
+ IRBuilder<> &);
bool SimplifySwitchOnSelect(SwitchInst *SI, SelectInst *Select);
bool SimplifyIndirectBrOnSelect(IndirectBrInst *IBI, SelectInst *SI);
bool TurnSwitchRangeIntoICmp(SwitchInst *SI, IRBuilder<> &Builder);
@@ -809,7 +812,13 @@ BasicBlock *SimplifyCFGOpt::GetValueEqualityComparisonCases(
}
BranchInst *BI = cast<BranchInst>(TI);
- ICmpInst *ICI = cast<ICmpInst>(BI->getCondition());
+ ICmpInst *ICI = nullptr;
+ if (auto *SI = dyn_cast<SelectInst>(BI->getCondition())) {
+ // We have already checked that the comparison is on the false arm.
+ ICI = cast<ICmpInst>(SI->getFalseValue());
+ } else {
+ ICI = cast<ICmpInst>(BI->getCondition());
+ }
BasicBlock *Succ = BI->getSuccessor(ICI->getPredicate() == ICmpInst::ICMP_NE);
Cases.push_back(ValueEqualityComparisonCase(
GetConstantInt(ICI->getOperand(1), DL), Succ));
@@ -1204,7 +1213,23 @@ bool SimplifyCFGOpt::PerformValueComparisonIntoPredecessorFolding(
} else if (PredHasWeights)
SuccWeights.assign(1 + BBCases.size(), 1);
- if (PredDefault == BB) {
+ auto I = BB->instructionsWithoutDebug(true).begin();
+ if (auto *BI = dyn_cast<BranchInst>(TI);
+ BI && isa<SelectInst>(BI->getCondition())) {
+ if (!isa<SwitchInst>(PTI))
+ return false;
+ // TODO: Preserve branch weight metadata
+ // We handle a phi node by the time we fold the select of a comparison.
+ PHINode &PHI = cast<PHINode>(*I);
+ if (PHI.getBasicBlockIndex(PredDefault) == -1)
+ return false;
+ auto *OldCond = BI->getCondition();
+ BI->setCondition(&PHI);
+ // We have harvested only one comparison.
+ PredCases.push_back(ValueEqualityComparisonCase(BBCases[0].Value, BB));
+ ++NewSuccessors[BB];
+ RecursivelyDeleteTriviallyDeadInstructions(OldCond);
+ } else if (PredDefault == BB) {
// If this is the default destination from PTI, only the edges in TI
// that don't occur in PTI, or that branch to BB will be activated.
std::set<ConstantInt *, ConstantIntOrdering> PTIHandled;
@@ -1315,7 +1340,10 @@ bool SimplifyCFGOpt::PerformValueComparisonIntoPredecessorFolding(
NewSuccessors) {
for (auto I : seq(NewSuccessor.second)) {
(void)I;
- AddPredecessorToBlock(NewSuccessor.first, Pred, BB);
+ // If the new successor happens to be `BB` itself, we are dealing with the
+ // case of the select of a comparison.
+ AddPredecessorToBlock(NewSuccessor.first, Pred,
+ NewSuccessor.first != BB ? BB : Pred);
}
if (DTU && !SuccsOfPred.contains(NewSuccessor.first))
Updates.push_back({DominatorTree::Insert, Pred, NewSuccessor.first});
@@ -1345,30 +1373,36 @@ bool SimplifyCFGOpt::PerformValueComparisonIntoPredecessorFolding(
EraseTerminatorAndDCECond(PTI);
- // Okay, last check. If BB is still a successor of PSI, then we must
- // have an infinite loop case. If so, add an infinitely looping block
- // to handle the case to preserve the behavior of the code.
+ // Okay, last check. If we are not handling a select of comparison, and BB is
+ // still a successor of PSI, then we must have an infinite loop case. If so,
+ // add an infinitely looping block to handle the case to preserve the behavior
+ // of the code.
BasicBlock *InfLoopBlock = nullptr;
- for (unsigned i = 0, e = NewSI->getNumSuccessors(); i != e; ++i)
- if (NewSI->getSuccessor(i) == BB) {
- if (!InfLoopBlock) {
- // Insert it at the end of the function, because it's either code,
- // or it won't matter if it's hot. :)
- InfLoopBlock =
- BasicBlock::Create(BB->getContext(), "infloop", BB->getParent());
- BranchInst::Create(InfLoopBlock, InfLoopBlock);
- if (DTU)
- Updates.push_back(
- {DominatorTree::Insert, InfLoopBlock, InfLoopBlock});
+ if (!isa<PHINode>(*I)) {
+ for (unsigned i = 0, e = NewSI->getNumSuccessors(); i != e; ++i)
+ if (NewSI->getSuccessor(i) == BB) {
+ if (!InfLoopBlock) {
+ // Insert it at the end of the function, because it's either code,
+ // or it won't matter if it's hot. :)
+ InfLoopBlock =
+ BasicBlock::Create(BB->getContext(), "infloop", BB->getParent());
+ BranchInst::Create(InfLoopBlock, InfLoopBlock);
+ if (DTU)
+ Updates.push_back(
+ {DominatorTree::Insert, InfLoopBlock, InfLoopBlock});
+ }
+ NewSI->setSuccessor(i, InfLoopBlock);
}
- NewSI->setSuccessor(i, InfLoopBlock);
- }
+ }
if (DTU) {
if (InfLoopBlock)
Updates.push_back({DominatorTree::Insert, Pred, InfLoopBlock});
- Updates.push_back({DominatorTree::Delete, Pred, BB});
+ if (!isa<PHINode>(*I))
+ Updates.push_back({DominatorTree::Delete, Pred, BB});
+ else
+ Updates.push_back({DominatorTree::Insert, Pred, BB});
DTU->applyUpdates(Updates);
}
@@ -1377,14 +1411,26 @@ bool SimplifyCFGOpt::PerformValueComparisonIntoPredecessorFolding(
return true;
}
-/// The specified terminator is a value equality comparison instruction
-/// (either a switch or a branch on "X == c").
-/// See if any of the predecessors of the terminator block are value comparisons
-/// on the same value. If so, and if safe to do so, fold them together.
+/// The specified terminator is a value equality comparison instruction, either
+/// a switch or a branch on "X == c", or a branch on select whose false arm is
+/// "X == c". If we happen to have a select, previously generated after
+/// speculatively executing its fall-through basic block, of the following kind:
+/// \code
+/// BB:
+/// %phi = phi i1 [ false, %edge ], [ true, %switch ], [ true, %switch ]
+/// %icmp = icmp eq i32 %c, X
+/// %spec.select = select i1 %phi, i1 true, i1 %icmp
+/// br i1 %spec.select1, label %EndBB, label %ThenBB
+/// \endcode
+/// We attempt folding them into its predecessor. To do so, see if any of the
+/// predecessors of the terminator block are value comparisons on the same
+/// value.
bool SimplifyCFGOpt::FoldValueComparisonIntoPredecessors(Instruction *TI,
- IRBuilder<> &Builder) {
+ IRBuilder<> &Builder,
+ Value *CV) {
BasicBlock *BB = TI->getParent();
- Value *CV = isValueEqualityComparison(TI); // CondVal
+ if (!CV)
+ CV = isValueEqualityComparison(TI); // CondVal
assert(CV && "Not a comparison?");
bool Changed = false;
@@ -1411,8 +1457,8 @@ bool SimplifyCFGOpt::FoldValueComparisonIntoPredecessors(Instruction *TI,
}
}
- PerformValueComparisonIntoPredecessorFolding(TI, CV, PTI, Builder);
- Changed = true;
+ Changed =
+ PerformValueComparisonIntoPredecessorFolding(TI, CV, PTI, Builder);
}
return Changed;
}
@@ -7255,6 +7301,29 @@ static BasicBlock *allPredecessorsComeFromSameSource(BasicBlock *BB) {
return PredPred;
}
+bool SimplifyCFGOpt::SimplifyEqualityComparisonOnSelectIntoSwitchPredecessors(
+ BranchInst *BI, IRBuilder<> &Builder) {
+ auto I = BI->getParent()->instructionsWithoutDebug(true).begin();
+ if (!isa<PHINode>(*I))
+ return false;
+
+ Value *CV = nullptr;
+ SelectInst *SI = nullptr;
+ if ((SI = dyn_cast<SelectInst>(BI->getCondition())) && SI->hasOneUse() &&
+ SI->getCondition() == &*I)
+ if (auto *ICI = dyn_cast<ICmpInst>(SI->getFalseValue()))
+ if (ICI->hasOneUse() && ICI->getPredicate() == ICmpInst::ICMP_EQ &&
+ GetConstantInt(ICI->getOperand(1), DL))
+ CV = ICI->getOperand(0);
+
+ if (CV)
+ if (auto *CI = dyn_cast<ConstantInt>(SI->getTrueValue()); CI && CI->isOne())
+ if (FoldValueComparisonIntoPredecessors(BI, Builder, CV))
+ return true;
+
+ return false;
+}
+
bool SimplifyCFGOpt::simplifyCondBranch(BranchInst *BI, IRBuilder<> &Builder) {
assert(
!isa<ConstantInt>(BI->getCondition()) &&
@@ -7288,6 +7357,9 @@ bool SimplifyCFGOpt::simplifyCondBranch(BranchInst *BI, IRBuilder<> &Builder) {
}
}
+ if (SimplifyEqualityComparisonOnSelectIntoSwitchPredecessors(BI, Builder))
+ return requestResimplify();
+
// Try to turn "br (X == 0 | X == 1), T, F" into a switch instruction.
if (SimplifyBranchOnICmpChain(BI, Builder, DL))
return true;
diff --git a/llvm/test/Transforms/SimplifyCFG/switch-multiple-comparisons-consolidation.ll b/llvm/test/Transforms/SimplifyCFG/switch-multiple-comparisons-consolidation.ll
index 0a873175939e111..fb39e50c15d0ab0 100644
--- a/llvm/test/Transforms/SimplifyCFG/switch-multiple-comparisons-consolidation.ll
+++ b/llvm/test/Transforms/SimplifyCFG/switch-multiple-comparisons-consolidation.ll
@@ -9,15 +9,14 @@ define i1 @test(i32 %c) {
; CHECK-NEXT: i32 115, label [[BB9:%.*]]
; CHECK-NEXT: i32 109, label [[BB9]]
; CHECK-NEXT: i32 104, label [[BB9]]
+; CHECK-NEXT: i32 100, label [[BB9]]
; CHECK-NEXT: ]
; CHECK: bb8:
; CHECK-NEXT: br label [[BB9]]
; CHECK: bb9:
-; CHECK-NEXT: [[_3_0:%.*]] = phi i1 [ false, [[BB8]] ], [ true, [[ENTRY:%.*]] ], [ true, [[ENTRY]] ], [ true, [[ENTRY]] ]
-; CHECK-NEXT: [[_10:%.*]] = icmp eq i32 [[C]], 100
-; CHECK-NEXT: [[SPEC_SELECT1:%.*]] = select i1 [[_3_0]], i1 true, i1 [[_10]]
+; CHECK-NEXT: [[_3_0:%.*]] = phi i1 [ false, [[BB8]] ], [ true, [[ENTRY:%.*]] ], [ true, [[ENTRY]] ], [ true, [[ENTRY]] ], [ true, [[ENTRY]] ]
; CHECK-NEXT: [[_12:%.*]] = icmp eq i32 [[C]], 119
-; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[SPEC_SELECT1]], i1 true, i1 [[_12]]
+; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[_3_0]], i1 true, i1 [[_12]]
; CHECK-NEXT: ret i1 [[SPEC_SELECT]]
;
entry:
More information about the llvm-commits
mailing list