-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[SimplifyCFG][Local] Redirect other phi values from BB to Succ except commom preds #166390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
@llvm/pr-subscribers-llvm-transforms Author: Kunqiu Chen (Camsyn) ChangesPreviously, #67275 redirected phi values from BB to Succ, but bailed out when there were >1 common predecessors between BB and Succ. This PR redirects other incoming blocks in phi from BB to Succ, except the common predecessors of BB and Succ, relaxing the intermediate phis (some optimizations, like JumpThreading, can benefit from this). E.g., define i8 @<!-- -->phis_of_switch(i8 noundef %arg, i1 %cond) {
start:
switch i8 %arg, label %unreachable [
i8 0, label %case0
i8 1, label %case1
i8 2, label %case2
i8 3, label %end
]
unreachable: ; preds = %start
unreachable
case1: ; preds = %start
br label %case0
case2: ; preds = %start
br i1 %cond, label %case0, label %end
case0: ; preds = %case2, %case1, %start
; %case2 and %start are common predecessors, but we can redirect %case1 to %end
%phi1 = phi i8 [ 1, %start ], [ 2, %case1 ], [ 3, %case2 ]
br label %end
end: ; preds = %case0, %case2, %start
%phi2 = phi i8 [ %phi1, %case0 ], [ 3, %start ], [ 4, %case2 ]
ret i8 %phi2
}after: define i8 @<!-- -->phis_of_switch(i8 noundef %arg, i1 %cond) {
start:
switch i8 %arg, label %unreachable [
i8 0, label %case0
i8 1, label %case1
i8 2, label %case2
i8 3, label %end
]
unreachable: ; preds = %start
unreachable
case1: ; preds = %start
br label %end
case2: ; preds = %start
br i1 %cond, label %case0, label %end
case0: ; preds = %case2, %start
%phi1 = phi i8 [ 1, %start ], [ 3, %case2 ]
br label %end
end: ; preds = %case1, %case0, %case2, %start
%phi2 = phi i8 [ 3, %start ], [ 4, %case2 ], [ 2, %case1 ], [ %phi1, %case0 ]
ret i8 %phi2
}Alive2 Proof: & Impact to JumpThreading: https://alive2.llvm.org/ce/z/qvQvY_ Full diff: https://github.com/llvm/llvm-project/pull/166390.diff 4 Files Affected:
diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
index c7d71eb5633ec..683fc40154a1c 100644
--- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp
+++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
@@ -2397,8 +2397,8 @@ void JumpThreadingPass::threadEdge(BasicBlock *BB,
// And finally, do it!
LLVM_DEBUG(dbgs() << " Threading edge from '" << PredBB->getName()
- << "' to '" << SuccBB->getName()
- << ", across block:\n " << *BB << "\n");
+ << "' to '" << SuccBB->getName() << "', across block:\n "
+ << *BB << "\n");
LVI->threadEdge(PredBB, BB, SuccBB);
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 46f29030ddb05..daf1005cd0a82 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1001,13 +1001,14 @@ static void replaceUndefValuesInPhi(PHINode *PN,
}
}
-// Only when they shares a single common predecessor, return true.
+// Only when there exists other incoming blocks besides the common predecessors
+// of BB and Succ, return true.
// Only handles cases when BB can't be merged while its predecessors can be
// redirected.
static bool
CanRedirectPredsOfEmptyBBToSucc(BasicBlock *BB, BasicBlock *Succ,
const SmallPtrSetImpl<BasicBlock *> &BBPreds,
- BasicBlock *&CommonPred) {
+ SmallPtrSetImpl<BasicBlock *> &CommonPreds) {
// There must be phis in BB, otherwise BB will be merged into Succ directly
if (BB->phis().empty() || Succ->phis().empty())
@@ -1022,17 +1023,14 @@ CanRedirectPredsOfEmptyBBToSucc(BasicBlock *BB, BasicBlock *Succ,
}))
return false;
- // Get the single common predecessor of both BB and Succ. Return false
- // when there are more than one common predecessors.
- for (BasicBlock *SuccPred : predecessors(Succ)) {
- if (BBPreds.count(SuccPred)) {
- if (CommonPred)
- return false;
- CommonPred = SuccPred;
- }
- }
-
- return true;
+ // Get the common predecessors of BB and Succ.
+ CommonPreds.insert_range(
+ make_filter_range(predecessors(Succ), [&BBPreds](BasicBlock *SuccPred) {
+ return BBPreds.count(SuccPred);
+ }));
+ // If all the preds of BB are also common preds of Succ, we can't redirect
+ // them to Succ.
+ return CommonPreds.size() < BBPreds.size();
}
/// Check whether removing \p BB will make the phis in its \p Succ have too
@@ -1069,11 +1067,10 @@ static bool introduceTooManyPhiEntries(BasicBlock *BB, BasicBlock *Succ) {
/// \param BB The block with the value flowing into the phi.
/// \param BBPreds The predecessors of BB.
/// \param PN The phi that we are updating.
-/// \param CommonPred The common predecessor of BB and PN's BasicBlock
-static void redirectValuesFromPredecessorsToPhi(BasicBlock *BB,
- const PredBlockVector &BBPreds,
- PHINode *PN,
- BasicBlock *CommonPred) {
+/// \param CommonPreds The common predecessors of BB and PN's BasicBlock
+static void redirectValuesFromPredecessorsToPhi(
+ BasicBlock *BB, const PredBlockVector &BBPreds, PHINode *PN,
+ const SmallPtrSetImpl<BasicBlock *> &CommonPreds) {
Value *OldVal = PN->removeIncomingValue(BB, false);
assert(OldVal && "No entry in PHI for Pred BB!");
@@ -1102,7 +1099,7 @@ static void redirectValuesFromPredecessorsToPhi(BasicBlock *BB,
// simplifying the corresponding conditional branch).
BasicBlock *PredBB = OldValPN->getIncomingBlock(i);
- if (PredBB == CommonPred)
+ if (CommonPreds.contains(PredBB))
continue;
Value *PredVal = OldValPN->getIncomingValue(i);
@@ -1113,14 +1110,20 @@ static void redirectValuesFromPredecessorsToPhi(BasicBlock *BB,
// newly retargeted branch.
PN->addIncoming(Selected, PredBB);
}
- if (CommonPred)
- PN->addIncoming(OldValPN->getIncomingValueForBlock(CommonPred), BB);
+ if (CommonPreds.size() == 1) {
+ // Single common predecessor, fold the phi node into Succ.
+ PN->addIncoming(OldValPN->getIncomingValueForBlock(*CommonPreds.begin()),
+ BB);
+ } else if (CommonPreds.size() >= 2) {
+ // >1 common predecessors, reserve the phi in BB.
+ PN->addIncoming(OldVal, BB);
+ }
} else {
for (BasicBlock *PredBB : BBPreds) {
// Update existing incoming values in PN for this
// predecessor of BB.
- if (PredBB == CommonPred)
+ if (CommonPreds.contains(PredBB))
continue;
Value *Selected =
@@ -1130,7 +1133,7 @@ static void redirectValuesFromPredecessorsToPhi(BasicBlock *BB,
// newly retargeted branch.
PN->addIncoming(Selected, PredBB);
}
- if (CommonPred)
+ if (!CommonPreds.empty())
PN->addIncoming(OldVal, BB);
}
@@ -1149,15 +1152,15 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
SmallPtrSet<BasicBlock *, 16> BBPreds(llvm::from_range, predecessors(BB));
- // The single common predecessor of BB and Succ when BB cannot be killed
- BasicBlock *CommonPred = nullptr;
+ // The common predecessors of BB and Succ when BB cannot be killed
+ SmallPtrSet<BasicBlock *, 4> CommonPreds;
bool BBKillable = CanPropagatePredecessorsForPHIs(BB, Succ, BBPreds);
// Even if we can not fold BB into Succ, we may be able to redirect the
// predecessors of BB to Succ.
bool BBPhisMergeable = BBKillable || CanRedirectPredsOfEmptyBBToSucc(
- BB, Succ, BBPreds, CommonPred);
+ BB, Succ, BBPreds, CommonPreds);
if ((!BBKillable && !BBPhisMergeable) || introduceTooManyPhiEntries(BB, Succ))
return false;
@@ -1192,10 +1195,13 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
}
}
- if (BBPhisMergeable && CommonPred)
- LLVM_DEBUG(dbgs() << "Found Common Predecessor between: " << BB->getName()
- << " and " << Succ->getName() << " : "
- << CommonPred->getName() << "\n");
+ if (BBPhisMergeable && !CommonPreds.empty()) {
+ LLVM_DEBUG(dbgs() << "Found Common Predecessors between " << BB->getName()
+ << " and " << Succ->getName() << " :");
+ for (BasicBlock *Pred : CommonPreds)
+ LLVM_DEBUG(dbgs() << " " << Pred->getName());
+ LLVM_DEBUG(dbgs() << "\n");
+ }
// 'BB' and 'BB->Pred' are loop latches, bail out to presrve inner loop
// metadata.
@@ -1296,7 +1302,7 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
for (auto *PredOfBB : predecessors(BB))
// When BB cannot be killed, do not remove the edge between BB and
// CommonPred.
- if (SeenPreds.insert(PredOfBB).second && PredOfBB != CommonPred)
+ if (SeenPreds.insert(PredOfBB).second && !CommonPreds.contains(PredOfBB))
Updates.push_back({DominatorTree::Delete, PredOfBB, BB});
if (BBKillable)
@@ -1312,7 +1318,7 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
// Loop over all of the PHI nodes in the successor of BB.
for (BasicBlock::iterator I = Succ->begin(); isa<PHINode>(I); ++I) {
PHINode *PN = cast<PHINode>(I);
- redirectValuesFromPredecessorsToPhi(BB, BBPreds, PN, CommonPred);
+ redirectValuesFromPredecessorsToPhi(BB, BBPreds, PN, CommonPreds);
}
}
@@ -1323,11 +1329,22 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
BB->getTerminator()->eraseFromParent();
Succ->splice(Succ->getFirstNonPHIIt(), BB);
} else {
- while (PHINode *PN = dyn_cast<PHINode>(&BB->front())) {
- // We explicitly check for such uses for merging phis.
- assert(PN->use_empty() && "There shouldn't be any uses here!");
- PN->eraseFromParent();
- }
+ // If we have >1 common preds, we should retain the phis in BB; Otherwise,
+ // remove any PHI nodes in BB.
+ bool RetainPhi = CommonPreds.size() >= 2;
+ if (RetainPhi)
+ for (PHINode &PN : BB->phis())
+ PN.removeIncomingValueIf([&CommonPreds, &PN](unsigned idx) {
+ // If the incoming block is not a common predecessor, remove it from
+ // the phi.
+ return !CommonPreds.contains(PN.getIncomingBlock(idx));
+ });
+ else
+ while (PHINode *PN = dyn_cast<PHINode>(&BB->front())) {
+ // We explicitly check for such uses for merging phis.
+ assert(PN->use_empty() && "There shouldn't be any uses here!");
+ PN->eraseFromParent();
+ }
}
// If the unconditional branch we replaced contains non-debug llvm.loop
@@ -1356,9 +1373,9 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
"applying corresponding DTU updates.");
} else if (BBPhisMergeable) {
// Everything except CommonPred that jumped to BB now goes to Succ.
- BB->replaceUsesWithIf(Succ, [BBPreds, CommonPred](Use &U) -> bool {
+ BB->replaceUsesWithIf(Succ, [BBPreds, CommonPreds](Use &U) -> bool {
if (Instruction *UseInst = dyn_cast<Instruction>(U.getUser()))
- return UseInst->getParent() != CommonPred &&
+ return !CommonPreds.contains(UseInst->getParent()) &&
BBPreds.contains(UseInst->getParent());
return false;
});
diff --git a/llvm/test/Transforms/JumpThreading/common-preds.ll b/llvm/test/Transforms/JumpThreading/common-preds.ll
new file mode 100644
index 0000000000000..28a6c079993e6
--- /dev/null
+++ b/llvm/test/Transforms/JumpThreading/common-preds.ll
@@ -0,0 +1,65 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -passes=jump-threading < %s | FileCheck %s
+
+; Jump threading would generate an intermediate BB `foo.thread` uncond to `succ`,
+; with preds case0, case1, and case2.
+; Theoretically, path case1/case0 -> foo.thread -> succ -> exit can be folded into case1/case0 -> exit.
+
+define i64 @bar(i64 %0, i1 %1, i64 %num) {
+; CHECK-LABEL: @bar(
+; CHECK-NEXT: switch i64 [[TMP0:%.*]], label [[EXIT2:%.*]] [
+; CHECK-NEXT: i64 0, label [[CASE0:%.*]]
+; CHECK-NEXT: i64 1, label [[CASE1:%.*]]
+; CHECK-NEXT: i64 2, label [[CASE2:%.*]]
+; CHECK-NEXT: ]
+; CHECK: case0:
+; CHECK-NEXT: br i1 [[TMP1:%.*]], label [[SUCC:%.*]], label [[FOO_THREAD:%.*]]
+; CHECK: case1:
+; CHECK-NEXT: br i1 [[TMP1]], label [[SUCC]], label [[FOO_THREAD]]
+; CHECK: case2:
+; CHECK-NEXT: br i1 [[TMP1]], label [[EXIT2]], label [[EXIT2]]
+; CHECK: succ:
+; CHECK-NEXT: [[PHI2:%.*]] = phi i64 [ [[NUM:%.*]], [[CASE1]] ], [ [[NUM]], [[CASE0]] ]
+; CHECK-NEXT: [[COND2:%.*]] = icmp eq i64 [[PHI2]], 0
+; CHECK-NEXT: br i1 [[COND2]], label [[FOO_THREAD]], label [[EXIT2]]
+; CHECK: exit:
+; CHECK-NEXT: [[PHI25:%.*]] = phi i64 [ [[PHI2]], [[SUCC]] ], [ 0, [[CASE1]] ], [ 0, [[CASE0]] ]
+; CHECK-NEXT: call void @foo()
+; CHECK-NEXT: ret i64 [[PHI25]]
+; CHECK: exit2:
+; CHECK-NEXT: ret i64 0
+;
+ switch i64 %0, label %foo [
+ i64 0, label %case0
+ i64 1, label %case1
+ i64 2, label %case2
+ ]
+
+case0: ; preds = %2
+ br i1 %1, label %succ, label %foo
+
+case1: ; preds = %2
+ br i1 %1, label %succ, label %foo
+
+case2:
+ br i1 %1, label %exit2, label %foo
+
+foo: ; preds = %case1, %case0, %2
+ %phi1 = phi i64 [ 0, %case0 ], [ 0, %case1 ], [ 1, %case2 ], [ 10, %2 ]
+ %cond1 = icmp ult i64 %phi1, 2
+ br i1 %cond1, label %succ, label %exit2
+
+succ: ; preds = %foo, %case1, %case0
+ %phi2 = phi i64 [ %phi1, %foo ], [ %num, %case1 ], [ %num, %case0 ]
+ %cond2 = icmp eq i64 %phi2, 0
+ br i1 %cond2, label %exit, label %exit2
+
+exit:
+ call void @foo()
+ ret i64 %phi2
+
+exit2:
+ ret i64 0
+}
+
+declare void @foo()
diff --git a/llvm/test/Transforms/SimplifyCFG/merge-phi-values.ll b/llvm/test/Transforms/SimplifyCFG/merge-phi-values.ll
new file mode 100644
index 0000000000000..1ccef71cb173b
--- /dev/null
+++ b/llvm/test/Transforms/SimplifyCFG/merge-phi-values.ll
@@ -0,0 +1,170 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt < %s -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck %s
+
+; Test a bunch of cases where the other phi values (i.e., comes from non-common predecessors)
+; should be merged into phi of the successor if there are >1 common predecessors.
+
+declare void @use(i8)
+
+define i8 @phis_of_switch(i8 noundef %arg, i1 %cond) {
+; CHECK-LABEL: define i8 @phis_of_switch(
+; CHECK-SAME: i8 noundef [[ARG:%.*]], i1 [[COND:%.*]]) {
+; CHECK-NEXT: [[START:.*]]:
+; CHECK-NEXT: switch i8 [[ARG]], label %[[UNREACHABLE:.*]] [
+; CHECK-NEXT: i8 0, label %[[CASE0:.*]]
+; CHECK-NEXT: i8 1, label %[[CASE1:.*]]
+; CHECK-NEXT: i8 2, label %[[CASE2:.*]]
+; CHECK-NEXT: i8 3, label %[[END:.*]]
+; CHECK-NEXT: ]
+; CHECK: [[UNREACHABLE]]:
+; CHECK-NEXT: unreachable
+; CHECK: [[CASE1]]:
+; CHECK-NEXT: br label %[[END]]
+; CHECK: [[CASE2]]:
+; CHECK-NEXT: br i1 [[COND]], label %[[CASE0]], label %[[END]]
+; CHECK: [[CASE0]]:
+; CHECK-NEXT: [[PHI1:%.*]] = phi i8 [ 1, %[[START]] ], [ 3, %[[CASE2]] ]
+; CHECK-NEXT: br label %[[END]]
+; CHECK: [[END]]:
+; CHECK-NEXT: [[PHI2:%.*]] = phi i8 [ 3, %[[START]] ], [ 4, %[[CASE2]] ], [ 2, %[[CASE1]] ], [ [[PHI1]], %[[CASE0]] ]
+; CHECK-NEXT: ret i8 [[PHI2]]
+;
+start:
+ switch i8 %arg, label %unreachable [
+ i8 0, label %case0
+ i8 1, label %case1
+ i8 2, label %case2
+ i8 3, label %end
+ ]
+
+unreachable: ; preds = %start
+ unreachable
+
+case1: ; preds = %start
+ br label %case0
+
+case2: ; preds = %start
+ br i1 %cond, label %case0, label %end
+
+case0: ; preds = %case2, %case1, %start
+ ; %case2 and %start are common predecessors, but we can redirect %case1 to %end
+ %phi1 = phi i8 [ 1, %start ], [ 2, %case1 ], [ 3, %case2 ]
+ br label %end
+
+end: ; preds = %case0, %case2, %start
+ %phi2 = phi i8 [ %phi1, %case0 ], [ 3, %start ], [ 4, %case2 ]
+ ret i8 %phi2
+}
+
+define i8 @phis_of_if(i8 noundef %arg, i1 %cond) {
+; CHECK-LABEL: define i8 @phis_of_if(
+; CHECK-SAME: i8 noundef [[ARG:%.*]], i1 [[COND:%.*]]) {
+; CHECK-NEXT: [[START:.*]]:
+; CHECK-NEXT: br i1 [[COND]], label %[[BRANCH:.*]], label %[[END:.*]]
+; CHECK: [[BRANCH]]:
+; CHECK-NEXT: [[COND0:%.*]] = icmp sgt i8 [[ARG]], 0
+; CHECK-NEXT: call void @use(i8 1)
+; CHECK-NEXT: br i1 [[COND0]], label %[[CASE0:.*]], label %[[CASE1:.*]]
+; CHECK: [[CASE0]]:
+; CHECK-NEXT: call void @use(i8 1)
+; CHECK-NEXT: [[COND1:%.*]] = icmp eq i8 [[ARG]], 1
+; CHECK-NEXT: br i1 [[COND1]], label %[[SINK:.*]], label %[[END]]
+; CHECK: [[CASE1]]:
+; CHECK-NEXT: call void @use(i8 1)
+; CHECK-NEXT: [[COND2:%.*]] = icmp eq i8 [[ARG]], -1
+; CHECK-NEXT: br i1 [[COND2]], label %[[SINK]], label %[[END]]
+; CHECK: [[SINK]]:
+; CHECK-NEXT: [[PHI1:%.*]] = phi i8 [ 1, %[[CASE0]] ], [ 2, %[[CASE1]] ]
+; CHECK-NEXT: br label %[[END]]
+; CHECK: [[END]]:
+; CHECK-NEXT: [[PHI2:%.*]] = phi i8 [ 3, %[[CASE0]] ], [ 4, %[[CASE1]] ], [ 0, %[[START]] ], [ [[PHI1]], %[[SINK]] ]
+; CHECK-NEXT: ret i8 [[PHI2]]
+;
+start:
+ br i1 %cond, label %branch, label %sink
+
+branch: ; preds = %start
+ %cond0 = icmp sgt i8 %arg, 0
+ call void @use(i8 1)
+ br i1 %cond0, label %case0, label %case1
+
+case0: ; preds = %branch
+ call void @use(i8 1)
+ %cond1 = icmp eq i8 %arg, 1
+ br i1 %cond1, label %sink, label %end
+
+case1: ; preds = %branch
+ call void @use(i8 1)
+ %cond2 = icmp eq i8 %arg, -1
+ br i1 %cond2, label %sink, label %end
+
+sink: ; preds = %case1, %case0, %start
+ ; %case0 and %case1 are common predecessors, but we can redirect %start to %end
+ %phi1 = phi i8 [ 0, %start ], [ 1, %case0 ], [ 2, %case1 ]
+ br label %end
+
+end: ; preds = %sink, %case1, %case0
+ %phi2 = phi i8 [ 3, %case0 ], [ 4, %case1 ], [ %phi1, %sink ]
+ ret i8 %phi2
+}
+
+define i64 @from_jump_threading(i64 %0, i1 %1, i64 %num) {
+; CHECK-LABEL: define i64 @from_jump_threading(
+; CHECK-SAME: i64 [[TMP0:%.*]], i1 [[TMP1:%.*]], i64 [[NUM:%.*]]) {
+; CHECK-NEXT: switch i64 [[TMP0]], label %[[COMMON_RET:.*]] [
+; CHECK-NEXT: i64 0, label %[[CASE0:.*]]
+; CHECK-NEXT: i64 1, label %[[CASE1:.*]]
+; CHECK-NEXT: i64 2, label %[[CASE2:.*]]
+; CHECK-NEXT: ]
+; CHECK: [[CASE0]]:
+; CHECK-NEXT: br i1 [[TMP1]], label %[[SUCC:.*]], label %[[FOO_THREAD:.*]]
+; CHECK: [[CASE1]]:
+; CHECK-NEXT: br i1 [[TMP1]], label %[[SUCC]], label %[[FOO_THREAD]]
+; CHECK: [[CASE2]]:
+; CHECK-NEXT: br i1 [[TMP1]], label %[[COMMON_RET]], label %[[SUCC]]
+; CHECK: [[FOO_THREAD]]:
+; CHECK-NEXT: [[PHI1_PH:%.*]] = phi i64 [ 0, %[[CASE1]] ], [ 0, %[[CASE0]] ]
+; CHECK-NEXT: br label %[[SUCC]]
+; CHECK: [[SUCC]]:
+; CHECK-NEXT: [[PHI2:%.*]] = phi i64 [ [[NUM]], %[[CASE1]] ], [ [[NUM]], %[[CASE0]] ], [ 1, %[[CASE2]] ], [ [[PHI1_PH]], %[[FOO_THREAD]] ]
+; CHECK-NEXT: [[COND2:%.*]] = icmp eq i64 [[PHI2]], 0
+; CHECK-NEXT: br i1 [[COND2]], label %[[EXIT:.*]], label %[[COMMON_RET]]
+; CHECK: [[COMMON_RET]]:
+; CHECK-NEXT: [[COMMON_RET_OP:%.*]] = phi i64 [ [[PHI2]], %[[EXIT]] ], [ 0, %[[SUCC]] ], [ 0, %[[CASE2]] ], [ 0, [[TMP2:%.*]] ]
+; CHECK-NEXT: ret i64 [[COMMON_RET_OP]]
+; CHECK: [[EXIT]]:
+; CHECK-NEXT: call void @use(i8 0)
+; CHECK-NEXT: br label %[[COMMON_RET]]
+;
+ switch i64 %0, label %exit2 [
+ i64 0, label %case0
+ i64 1, label %case1
+ i64 2, label %case2
+ ]
+
+case0: ; preds = %2
+ br i1 %1, label %succ, label %foo.thread
+
+case1: ; preds = %2
+ br i1 %1, label %succ, label %foo.thread
+
+case2: ; preds = %2
+ br i1 %1, label %exit2, label %foo.thread
+
+foo.thread: ; preds = %case2, %case1, %case0
+ ; %case0 and %case1 are common predecessors, but we can redirect %case2 to %succ
+ %phi1.ph = phi i64 [ 1, %case2 ], [ 0, %case1 ], [ 0, %case0 ]
+ br label %succ
+
+succ: ; preds = %foo.thread, %case1, %case0
+ %phi2 = phi i64 [ %num, %case1 ], [ %num, %case0 ], [ %phi1.ph, %foo.thread ]
+ %cond2 = icmp eq i64 %phi2, 0
+ br i1 %cond2, label %exit, label %exit2
+
+exit: ; preds = %succ
+ call void @use(i8 0)
+ ret i64 %phi2
+
+exit2: ; preds = %succ, %case2, %2
+ ret i64 0
+}
|
|
Sounds reasonable to me. But I have not touched this code for a long time. I would have a deeper review after several days. |
| @@ -0,0 +1,65 @@ | |||
| ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add some tests with multiple common predecessors, where some of them are identical? Such as:
switch i64 %0, label %foo [
i64 0, label %bb
i64 1, label %bb
i64 2, label %succ
i64 3, label %succ
]There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, this PR can eliminate the whole phis in bb if the common predecessors between bb and succ are the same 😀.
Please refer to 39ec0c6
llvm/lib/Transforms/Utils/Local.cpp
Outdated
| } else if (BBPhisMergeable) { | ||
| // Everything except CommonPred that jumped to BB now goes to Succ. | ||
| BB->replaceUsesWithIf(Succ, [BBPreds, CommonPred](Use &U) -> bool { | ||
| BB->replaceUsesWithIf(Succ, [BBPreds, CommonPreds](Use &U) -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| BB->replaceUsesWithIf(Succ, [BBPreds, CommonPreds](Use &U) -> bool { | |
| BB->replaceUsesWithIf(Succ, [&BBPreds, &CommonPreds](Use &U) -> bool { |
|
@zyw-bot csmithfuzz |
|
Invalid command. Available commands: |
|
The redirection of phi-values might affect any other uncond br folding after it ... See dtcxzyw/llvm-opt-benchmark#3033 (comment). @XChy , do you have any idea? Or we can delay the timing to perform phi-values redirecting compared uncond br folding? |
|
@zyw-bot csmith-fuzz |
Previously, #67275 redirected phi values from BB to Succ, but bailed out when there were >1 common predecessors between BB and Succ.
This PR redirects other incoming blocks in phi from BB to Succ, except the common predecessors of BB and Succ, relaxing the intermediate phis (some optimizations, like JumpThreading, can benefit from this).
Moreover, this PR supports the elimination of
phis in BB if all the common predecessors are the same (e.g., multi-case dest in switch).E.g.,
before:
after:
Alive2 Proof: & Impact to JumpThreading: https://alive2.llvm.org/ce/z/qvQvY_
Fix the regression found in dtcxzyw/llvm-opt-benchmark#3007 (comment)
Negligible CompTime Impact: dtcxzyw/llvm-opt-benchmark#3022
IR diff: dtcxzyw/llvm-opt-benchmark#3033