Skip to content

Commit 35e350d

Browse files
committed
Revert "[SimplifyCFG] Handle branch on same condition in pred more directly" and "[SimplifyCFG] Make FoldCondBranchOnPHI more amenable to extension"
This reverts commit 3df86e7. This reverts commit 8988254. `[SimplifyCFG] Handle branch on same condition in pred more directly` caused non-determinism when compiling opt with a bootstrapped clang. I have to revert the dependent commit as well.
1 parent a7691de commit 35e350d

File tree

1 file changed

+61
-60
lines changed

1 file changed

+61
-60
lines changed

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

Lines changed: 61 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -2975,66 +2975,40 @@ static bool BlockIsSimpleEnoughToThreadThrough(BasicBlock *BB) {
29752975
return true;
29762976
}
29772977

2978-
static ConstantInt *getKnownValueOnEdge(Value *V, BasicBlock *From,
2979-
BasicBlock *To) {
2980-
// Don't look past the block defining the value, we might get the value from
2981-
// a previous loop iteration.
2982-
auto *I = dyn_cast<Instruction>(V);
2983-
if (I && I->getParent() == To)
2984-
return nullptr;
2985-
2986-
// We know the value if the From block branches on it.
2987-
auto *BI = dyn_cast<BranchInst>(From->getTerminator());
2988-
if (BI && BI->isConditional() && BI->getCondition() == V &&
2989-
BI->getSuccessor(0) != BI->getSuccessor(1))
2990-
return BI->getSuccessor(0) == To ? ConstantInt::getTrue(BI->getContext())
2991-
: ConstantInt::getFalse(BI->getContext());
2992-
2993-
return nullptr;
2994-
}
2995-
2996-
/// If we have a conditional branch on something for which we know the constant
2997-
/// value in predecessors (e.g. a phi node in the current block), thread edges
2998-
/// from the predecessor to their ultimate destination.
2999-
static Optional<bool>
3000-
FoldCondBranchOnValueKnownInPredecessorImpl(BranchInst *BI, DomTreeUpdater *DTU,
3001-
const DataLayout &DL,
3002-
AssumptionCache *AC) {
3003-
SmallDenseMap<BasicBlock *, ConstantInt *> KnownValues;
2978+
/// If we have a conditional branch on a PHI node value that is defined in the
2979+
/// same block as the branch and if any PHI entries are constants, thread edges
2980+
/// corresponding to that entry to be branches to their ultimate destination.
2981+
static Optional<bool> FoldCondBranchOnPHIImpl(BranchInst *BI,
2982+
DomTreeUpdater *DTU,
2983+
const DataLayout &DL,
2984+
AssumptionCache *AC) {
30042985
BasicBlock *BB = BI->getParent();
3005-
Value *Cond = BI->getCondition();
3006-
PHINode *PN = dyn_cast<PHINode>(Cond);
3007-
if (PN && PN->getParent() == BB) {
3008-
// Degenerate case of a single entry PHI.
3009-
if (PN->getNumIncomingValues() == 1) {
3010-
FoldSingleEntryPHINodes(PN->getParent());
3011-
return true;
3012-
}
2986+
PHINode *PN = dyn_cast<PHINode>(BI->getCondition());
2987+
if (!PN || PN->getParent() != BB)
2988+
return false;
30132989

3014-
for (Use &U : PN->incoming_values())
3015-
if (auto *CB = dyn_cast<ConstantInt>(U))
3016-
KnownValues.insert({PN->getIncomingBlock(U), CB});
3017-
} else {
3018-
for (BasicBlock *Pred : predecessors(BB)) {
3019-
if (ConstantInt *CB = getKnownValueOnEdge(Cond, Pred, BB))
3020-
KnownValues.insert({Pred, CB});
3021-
}
2990+
// Degenerate case of a single entry PHI.
2991+
if (PN->getNumIncomingValues() == 1) {
2992+
FoldSingleEntryPHINodes(PN->getParent());
2993+
return true;
30222994
}
30232995

3024-
if (KnownValues.empty())
3025-
return false;
3026-
30272996
// Now we know that this block has multiple preds and two succs.
30282997
// Check that the block is small enough and values defined in the block are
30292998
// not used outside of it.
30302999
if (!BlockIsSimpleEnoughToThreadThrough(BB))
30313000
return false;
30323001

3033-
for (const auto &Pair : KnownValues) {
3002+
// Okay, this is a simple enough basic block. See if any phi values are
3003+
// constants.
3004+
for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
3005+
ConstantInt *CB = dyn_cast<ConstantInt>(PN->getIncomingValue(i));
3006+
if (!CB || !CB->getType()->isIntegerTy(1))
3007+
continue;
3008+
30343009
// Okay, we now know that all edges from PredBB should be revectored to
30353010
// branch to RealDest.
3036-
ConstantInt *CB = Pair.second;
3037-
BasicBlock *PredBB = Pair.first;
3011+
BasicBlock *PredBB = PN->getIncomingBlock(i);
30383012
BasicBlock *RealDest = BI->getSuccessor(!CB->getZExtValue());
30393013

30403014
if (RealDest == BB)
@@ -3128,15 +3102,13 @@ FoldCondBranchOnValueKnownInPredecessorImpl(BranchInst *BI, DomTreeUpdater *DTU,
31283102
return false;
31293103
}
31303104

3131-
static bool FoldCondBranchOnValueKnownInPredecessor(BranchInst *BI,
3132-
DomTreeUpdater *DTU,
3133-
const DataLayout &DL,
3134-
AssumptionCache *AC) {
3105+
static bool FoldCondBranchOnPHI(BranchInst *BI, DomTreeUpdater *DTU,
3106+
const DataLayout &DL, AssumptionCache *AC) {
31353107
Optional<bool> Result;
31363108
bool EverChanged = false;
31373109
do {
31383110
// Note that None means "we changed things, but recurse further."
3139-
Result = FoldCondBranchOnValueKnownInPredecessorImpl(BI, DTU, DL, AC);
3111+
Result = FoldCondBranchOnPHIImpl(BI, DTU, DL, AC);
31403112
EverChanged |= Result == None || *Result;
31413113
} while (Result == None);
31423114
return EverChanged;
@@ -4068,15 +4040,43 @@ static bool SimplifyCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
40684040
if (PBI->getCondition() == BI->getCondition() &&
40694041
PBI->getSuccessor(0) != PBI->getSuccessor(1)) {
40704042
// Okay, the outcome of this conditional branch is statically
4071-
// knowable. If this block had a single pred, handle specially, otherwise
4072-
// FoldCondBranchOnValueKnownInPredecessor() will handle it.
4043+
// knowable. If this block had a single pred, handle specially.
40734044
if (BB->getSinglePredecessor()) {
40744045
// Turn this into a branch on constant.
40754046
bool CondIsTrue = PBI->getSuccessor(0) == BB;
40764047
BI->setCondition(
40774048
ConstantInt::get(Type::getInt1Ty(BB->getContext()), CondIsTrue));
40784049
return true; // Nuke the branch on constant.
40794050
}
4051+
4052+
// Otherwise, if there are multiple predecessors, insert a PHI that merges
4053+
// in the constant and simplify the block result. Subsequent passes of
4054+
// simplifycfg will thread the block.
4055+
if (BlockIsSimpleEnoughToThreadThrough(BB)) {
4056+
pred_iterator PB = pred_begin(BB), PE = pred_end(BB);
4057+
PHINode *NewPN = PHINode::Create(
4058+
Type::getInt1Ty(BB->getContext()), std::distance(PB, PE),
4059+
BI->getCondition()->getName() + ".pr", &BB->front());
4060+
// Okay, we're going to insert the PHI node. Since PBI is not the only
4061+
// predecessor, compute the PHI'd conditional value for all of the preds.
4062+
// Any predecessor where the condition is not computable we keep symbolic.
4063+
for (pred_iterator PI = PB; PI != PE; ++PI) {
4064+
BasicBlock *P = *PI;
4065+
if ((PBI = dyn_cast<BranchInst>(P->getTerminator())) && PBI != BI &&
4066+
PBI->isConditional() && PBI->getCondition() == BI->getCondition() &&
4067+
PBI->getSuccessor(0) != PBI->getSuccessor(1)) {
4068+
bool CondIsTrue = PBI->getSuccessor(0) == BB;
4069+
NewPN->addIncoming(
4070+
ConstantInt::get(Type::getInt1Ty(BB->getContext()), CondIsTrue),
4071+
P);
4072+
} else {
4073+
NewPN->addIncoming(BI->getCondition(), P);
4074+
}
4075+
}
4076+
4077+
BI->setCondition(NewPN);
4078+
return true;
4079+
}
40804080
}
40814081

40824082
// If the previous block ended with a widenable branch, determine if reusing
@@ -6903,11 +6903,12 @@ bool SimplifyCFGOpt::simplifyCondBranch(BranchInst *BI, IRBuilder<> &Builder) {
69036903
return requestResimplify();
69046904
}
69056905

6906-
// If this is a branch on something for which we know the constant value in
6907-
// predecessors (e.g. a phi node in the current block), thread control
6908-
// through this block.
6909-
if (FoldCondBranchOnValueKnownInPredecessor(BI, DTU, DL, Options.AC))
6910-
return requestResimplify();
6906+
// If this is a branch on a phi node in the current block, thread control
6907+
// through this block if any PHI node entries are constants.
6908+
if (PHINode *PN = dyn_cast<PHINode>(BI->getCondition()))
6909+
if (PN->getParent() == BI->getParent())
6910+
if (FoldCondBranchOnPHI(BI, DTU, DL, Options.AC))
6911+
return requestResimplify();
69116912

69126913
// Scan predecessor blocks for conditional branches.
69136914
for (BasicBlock *Pred : predecessors(BB))

0 commit comments

Comments
 (0)