[llvm-branch-commits] [llvm] a4e6c2e - [NFC][SimplifyCFG] Extract PerformValueComparisonIntoPredecessorFolding() out of FoldValueComparisonIntoPredecessors()
Roman Lebedev via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sat Jan 23 13:59:37 PST 2021
Author: Roman Lebedev
Date: 2021-01-24T00:54:54+03:00
New Revision: a4e6c2e647b09dd8c2c5cf55bb05e3c7fd89646c
URL: https://github.com/llvm/llvm-project/commit/a4e6c2e647b09dd8c2c5cf55bb05e3c7fd89646c
DIFF: https://github.com/llvm/llvm-project/commit/a4e6c2e647b09dd8c2c5cf55bb05e3c7fd89646c.diff
LOG: [NFC][SimplifyCFG] Extract PerformValueComparisonIntoPredecessorFolding() out of FoldValueComparisonIntoPredecessors()
Less nested code is much easier to follow and modify.
Added:
Modified:
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index deaa0bbcf772..318e6d5cf810 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -229,6 +229,9 @@ class SimplifyCFGOpt {
bool SimplifyEqualityComparisonWithOnlyPredecessor(Instruction *TI,
BasicBlock *Pred,
IRBuilder<> &Builder);
+ bool PerformValueComparisonIntoPredecessorFolding(Instruction *TI, Value *&CV,
+ Instruction *PTI,
+ IRBuilder<> &Builder);
bool FoldValueComparisonIntoPredecessors(Instruction *TI,
IRBuilder<> &Builder);
@@ -1046,6 +1049,215 @@ static void FitWeights(MutableArrayRef<uint64_t> Weights) {
}
}
+bool SimplifyCFGOpt::PerformValueComparisonIntoPredecessorFolding(
+ Instruction *TI, Value *&CV, Instruction *PTI, IRBuilder<> &Builder) {
+ BasicBlock *BB = TI->getParent();
+ BasicBlock *Pred = PTI->getParent();
+
+ std::vector<DominatorTree::UpdateType> Updates;
+
+ // Figure out which 'cases' to copy from SI to PSI.
+ std::vector<ValueEqualityComparisonCase> BBCases;
+ BasicBlock *BBDefault = GetValueEqualityComparisonCases(TI, BBCases);
+
+ std::vector<ValueEqualityComparisonCase> PredCases;
+ BasicBlock *PredDefault = GetValueEqualityComparisonCases(PTI, PredCases);
+
+ // Based on whether the default edge from PTI goes to BB or not, fill in
+ // PredCases and PredDefault with the new switch cases we would like to
+ // build.
+ SmallMapVector<BasicBlock *, int, 8> NewSuccessors;
+
+ // Update the branch weight metadata along the way
+ SmallVector<uint64_t, 8> Weights;
+ bool PredHasWeights = HasBranchWeights(PTI);
+ bool SuccHasWeights = HasBranchWeights(TI);
+
+ if (PredHasWeights) {
+ GetBranchWeights(PTI, Weights);
+ // branch-weight metadata is inconsistent here.
+ if (Weights.size() != 1 + PredCases.size())
+ PredHasWeights = SuccHasWeights = false;
+ } else if (SuccHasWeights)
+ // If there are no predecessor weights but there are successor weights,
+ // populate Weights with 1, which will later be scaled to the sum of
+ // successor's weights
+ Weights.assign(1 + PredCases.size(), 1);
+
+ SmallVector<uint64_t, 8> SuccWeights;
+ if (SuccHasWeights) {
+ GetBranchWeights(TI, SuccWeights);
+ // branch-weight metadata is inconsistent here.
+ if (SuccWeights.size() != 1 + BBCases.size())
+ PredHasWeights = SuccHasWeights = false;
+ } else if (PredHasWeights)
+ SuccWeights.assign(1 + BBCases.size(), 1);
+
+ 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;
+ for (unsigned i = 0, e = PredCases.size(); i != e; ++i)
+ if (PredCases[i].Dest != BB)
+ PTIHandled.insert(PredCases[i].Value);
+ else {
+ // The default destination is BB, we don't need explicit targets.
+ std::swap(PredCases[i], PredCases.back());
+
+ if (PredHasWeights || SuccHasWeights) {
+ // Increase weight for the default case.
+ Weights[0] += Weights[i + 1];
+ std::swap(Weights[i + 1], Weights.back());
+ Weights.pop_back();
+ }
+
+ PredCases.pop_back();
+ --i;
+ --e;
+ }
+
+ // Reconstruct the new switch statement we will be building.
+ if (PredDefault != BBDefault) {
+ PredDefault->removePredecessor(Pred);
+ if (PredDefault != BB)
+ Updates.push_back({DominatorTree::Delete, Pred, PredDefault});
+ PredDefault = BBDefault;
+ ++NewSuccessors[BBDefault];
+ }
+
+ unsigned CasesFromPred = Weights.size();
+ uint64_t ValidTotalSuccWeight = 0;
+ for (unsigned i = 0, e = BBCases.size(); i != e; ++i)
+ if (!PTIHandled.count(BBCases[i].Value) && BBCases[i].Dest != BBDefault) {
+ PredCases.push_back(BBCases[i]);
+ ++NewSuccessors[BBCases[i].Dest];
+ if (SuccHasWeights || PredHasWeights) {
+ // The default weight is at index 0, so weight for the ith case
+ // should be at index i+1. Scale the cases from successor by
+ // PredDefaultWeight (Weights[0]).
+ Weights.push_back(Weights[0] * SuccWeights[i + 1]);
+ ValidTotalSuccWeight += SuccWeights[i + 1];
+ }
+ }
+
+ if (SuccHasWeights || PredHasWeights) {
+ ValidTotalSuccWeight += SuccWeights[0];
+ // Scale the cases from predecessor by ValidTotalSuccWeight.
+ for (unsigned i = 1; i < CasesFromPred; ++i)
+ Weights[i] *= ValidTotalSuccWeight;
+ // Scale the default weight by SuccDefaultWeight (SuccWeights[0]).
+ Weights[0] *= SuccWeights[0];
+ }
+ } else {
+ // If this is not the default destination from PSI, only the edges
+ // in SI that occur in PSI with a destination of BB will be
+ // activated.
+ std::set<ConstantInt *, ConstantIntOrdering> PTIHandled;
+ std::map<ConstantInt *, uint64_t> WeightsForHandled;
+ for (unsigned i = 0, e = PredCases.size(); i != e; ++i)
+ if (PredCases[i].Dest == BB) {
+ PTIHandled.insert(PredCases[i].Value);
+
+ if (PredHasWeights || SuccHasWeights) {
+ WeightsForHandled[PredCases[i].Value] = Weights[i + 1];
+ std::swap(Weights[i + 1], Weights.back());
+ Weights.pop_back();
+ }
+
+ std::swap(PredCases[i], PredCases.back());
+ PredCases.pop_back();
+ --i;
+ --e;
+ }
+
+ // Okay, now we know which constants were sent to BB from the
+ // predecessor. Figure out where they will all go now.
+ for (unsigned i = 0, e = BBCases.size(); i != e; ++i)
+ if (PTIHandled.count(BBCases[i].Value)) {
+ // If this is one we are capable of getting...
+ if (PredHasWeights || SuccHasWeights)
+ Weights.push_back(WeightsForHandled[BBCases[i].Value]);
+ PredCases.push_back(BBCases[i]);
+ ++NewSuccessors[BBCases[i].Dest];
+ PTIHandled.erase(BBCases[i].Value); // This constant is taken care of
+ }
+
+ // If there are any constants vectored to BB that TI doesn't handle,
+ // they must go to the default destination of TI.
+ for (ConstantInt *I : PTIHandled) {
+ if (PredHasWeights || SuccHasWeights)
+ Weights.push_back(WeightsForHandled[I]);
+ PredCases.push_back(ValueEqualityComparisonCase(I, BBDefault));
+ ++NewSuccessors[BBDefault];
+ }
+ }
+
+ // Okay, at this point, we know which new successor Pred will get. Make
+ // sure we update the number of entries in the PHI nodes for these
+ // successors.
+ for (const std::pair<BasicBlock *, int /*Num*/> &NewSuccessor :
+ NewSuccessors) {
+ for (auto I : seq(0, NewSuccessor.second)) {
+ (void)I;
+ AddPredecessorToBlock(NewSuccessor.first, Pred, BB);
+ }
+ if (!is_contained(successors(Pred), NewSuccessor.first))
+ Updates.push_back({DominatorTree::Insert, Pred, NewSuccessor.first});
+ }
+
+ Builder.SetInsertPoint(PTI);
+ // Convert pointer to int before we switch.
+ if (CV->getType()->isPointerTy()) {
+ CV =
+ Builder.CreatePtrToInt(CV, DL.getIntPtrType(CV->getType()), "magicptr");
+ }
+
+ // Now that the successors are updated, create the new Switch instruction.
+ SwitchInst *NewSI = Builder.CreateSwitch(CV, PredDefault, PredCases.size());
+ NewSI->setDebugLoc(PTI->getDebugLoc());
+ for (ValueEqualityComparisonCase &V : PredCases)
+ NewSI->addCase(V.Value, V.Dest);
+
+ if (PredHasWeights || SuccHasWeights) {
+ // Halve the weights if any of them cannot fit in an uint32_t
+ FitWeights(Weights);
+
+ SmallVector<uint32_t, 8> MDWeights(Weights.begin(), Weights.end());
+
+ setBranchWeights(NewSI, MDWeights);
+ }
+
+ 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.
+ 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);
+ Updates.push_back({DominatorTree::Insert, InfLoopBlock, InfLoopBlock});
+ }
+ NewSI->setSuccessor(i, InfLoopBlock);
+ }
+
+ if (InfLoopBlock)
+ Updates.push_back({DominatorTree::Insert, Pred, InfLoopBlock});
+
+ Updates.push_back({DominatorTree::Delete, Pred, BB});
+
+ if (DTU)
+ DTU->applyUpdates(Updates);
+
+ ++NumFoldValueComparisonIntoPredecessors;
+ 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
@@ -1058,11 +1270,6 @@ bool SimplifyCFGOpt::FoldValueComparisonIntoPredecessors(Instruction *TI,
bool Changed = false;
- auto _ = make_scope_exit([&]() {
- if (Changed)
- ++NumFoldValueComparisonIntoPredecessors;
- });
-
SmallSetVector<BasicBlock *, 16> Preds(pred_begin(BB), pred_end(BB));
while (!Preds.empty()) {
BasicBlock *Pred = Preds.pop_back_val();
@@ -1081,210 +1288,7 @@ bool SimplifyCFGOpt::FoldValueComparisonIntoPredecessors(Instruction *TI,
}
}
- std::vector<DominatorTree::UpdateType> Updates;
-
- // Figure out which 'cases' to copy from SI to PSI.
- std::vector<ValueEqualityComparisonCase> BBCases;
- BasicBlock *BBDefault = GetValueEqualityComparisonCases(TI, BBCases);
-
- std::vector<ValueEqualityComparisonCase> PredCases;
- BasicBlock *PredDefault = GetValueEqualityComparisonCases(PTI, PredCases);
-
- // Based on whether the default edge from PTI goes to BB or not, fill in
- // PredCases and PredDefault with the new switch cases we would like to
- // build.
- SmallMapVector<BasicBlock *, int, 8> NewSuccessors;
-
- // Update the branch weight metadata along the way
- SmallVector<uint64_t, 8> Weights;
- bool PredHasWeights = HasBranchWeights(PTI);
- bool SuccHasWeights = HasBranchWeights(TI);
-
- if (PredHasWeights) {
- GetBranchWeights(PTI, Weights);
- // branch-weight metadata is inconsistent here.
- if (Weights.size() != 1 + PredCases.size())
- PredHasWeights = SuccHasWeights = false;
- } else if (SuccHasWeights)
- // If there are no predecessor weights but there are successor weights,
- // populate Weights with 1, which will later be scaled to the sum of
- // successor's weights
- Weights.assign(1 + PredCases.size(), 1);
-
- SmallVector<uint64_t, 8> SuccWeights;
- if (SuccHasWeights) {
- GetBranchWeights(TI, SuccWeights);
- // branch-weight metadata is inconsistent here.
- if (SuccWeights.size() != 1 + BBCases.size())
- PredHasWeights = SuccHasWeights = false;
- } else if (PredHasWeights)
- SuccWeights.assign(1 + BBCases.size(), 1);
-
- 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;
- for (unsigned i = 0, e = PredCases.size(); i != e; ++i)
- if (PredCases[i].Dest != BB)
- PTIHandled.insert(PredCases[i].Value);
- else {
- // The default destination is BB, we don't need explicit targets.
- std::swap(PredCases[i], PredCases.back());
-
- if (PredHasWeights || SuccHasWeights) {
- // Increase weight for the default case.
- Weights[0] += Weights[i + 1];
- std::swap(Weights[i + 1], Weights.back());
- Weights.pop_back();
- }
-
- PredCases.pop_back();
- --i;
- --e;
- }
-
- // Reconstruct the new switch statement we will be building.
- if (PredDefault != BBDefault) {
- PredDefault->removePredecessor(Pred);
- if (PredDefault != BB)
- Updates.push_back({DominatorTree::Delete, Pred, PredDefault});
- PredDefault = BBDefault;
- ++NewSuccessors[BBDefault];
- }
-
- unsigned CasesFromPred = Weights.size();
- uint64_t ValidTotalSuccWeight = 0;
- for (unsigned i = 0, e = BBCases.size(); i != e; ++i)
- if (!PTIHandled.count(BBCases[i].Value) &&
- BBCases[i].Dest != BBDefault) {
- PredCases.push_back(BBCases[i]);
- ++NewSuccessors[BBCases[i].Dest];
- if (SuccHasWeights || PredHasWeights) {
- // The default weight is at index 0, so weight for the ith case
- // should be at index i+1. Scale the cases from successor by
- // PredDefaultWeight (Weights[0]).
- Weights.push_back(Weights[0] * SuccWeights[i + 1]);
- ValidTotalSuccWeight += SuccWeights[i + 1];
- }
- }
-
- if (SuccHasWeights || PredHasWeights) {
- ValidTotalSuccWeight += SuccWeights[0];
- // Scale the cases from predecessor by ValidTotalSuccWeight.
- for (unsigned i = 1; i < CasesFromPred; ++i)
- Weights[i] *= ValidTotalSuccWeight;
- // Scale the default weight by SuccDefaultWeight (SuccWeights[0]).
- Weights[0] *= SuccWeights[0];
- }
- } else {
- // If this is not the default destination from PSI, only the edges
- // in SI that occur in PSI with a destination of BB will be
- // activated.
- std::set<ConstantInt *, ConstantIntOrdering> PTIHandled;
- std::map<ConstantInt *, uint64_t> WeightsForHandled;
- for (unsigned i = 0, e = PredCases.size(); i != e; ++i)
- if (PredCases[i].Dest == BB) {
- PTIHandled.insert(PredCases[i].Value);
-
- if (PredHasWeights || SuccHasWeights) {
- WeightsForHandled[PredCases[i].Value] = Weights[i + 1];
- std::swap(Weights[i + 1], Weights.back());
- Weights.pop_back();
- }
-
- std::swap(PredCases[i], PredCases.back());
- PredCases.pop_back();
- --i;
- --e;
- }
-
- // Okay, now we know which constants were sent to BB from the
- // predecessor. Figure out where they will all go now.
- for (unsigned i = 0, e = BBCases.size(); i != e; ++i)
- if (PTIHandled.count(BBCases[i].Value)) {
- // If this is one we are capable of getting...
- if (PredHasWeights || SuccHasWeights)
- Weights.push_back(WeightsForHandled[BBCases[i].Value]);
- PredCases.push_back(BBCases[i]);
- ++NewSuccessors[BBCases[i].Dest];
- PTIHandled.erase(
- BBCases[i].Value); // This constant is taken care of
- }
-
- // If there are any constants vectored to BB that TI doesn't handle,
- // they must go to the default destination of TI.
- for (ConstantInt *I : PTIHandled) {
- if (PredHasWeights || SuccHasWeights)
- Weights.push_back(WeightsForHandled[I]);
- PredCases.push_back(ValueEqualityComparisonCase(I, BBDefault));
- ++NewSuccessors[BBDefault];
- }
- }
-
- // Okay, at this point, we know which new successor Pred will get. Make
- // sure we update the number of entries in the PHI nodes for these
- // successors.
- for (const std::pair<BasicBlock *, int /*Num*/> &NewSuccessor :
- NewSuccessors) {
- for (auto I : seq(0, NewSuccessor.second)) {
- (void)I;
- AddPredecessorToBlock(NewSuccessor.first, Pred, BB);
- }
- if (!is_contained(successors(Pred), NewSuccessor.first))
- Updates.push_back({DominatorTree::Insert, Pred, NewSuccessor.first});
- }
-
- Builder.SetInsertPoint(PTI);
- // Convert pointer to int before we switch.
- if (CV->getType()->isPointerTy()) {
- CV = Builder.CreatePtrToInt(CV, DL.getIntPtrType(CV->getType()),
- "magicptr");
- }
-
- // Now that the successors are updated, create the new Switch instruction.
- SwitchInst *NewSI =
- Builder.CreateSwitch(CV, PredDefault, PredCases.size());
- NewSI->setDebugLoc(PTI->getDebugLoc());
- for (ValueEqualityComparisonCase &V : PredCases)
- NewSI->addCase(V.Value, V.Dest);
-
- if (PredHasWeights || SuccHasWeights) {
- // Halve the weights if any of them cannot fit in an uint32_t
- FitWeights(Weights);
-
- SmallVector<uint32_t, 8> MDWeights(Weights.begin(), Weights.end());
-
- setBranchWeights(NewSI, MDWeights);
- }
-
- 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.
- 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);
- Updates.push_back(
- {DominatorTree::Insert, InfLoopBlock, InfLoopBlock});
- }
- NewSI->setSuccessor(i, InfLoopBlock);
- }
-
- if (InfLoopBlock)
- Updates.push_back({DominatorTree::Insert, Pred, InfLoopBlock});
-
- Updates.push_back({DominatorTree::Delete, Pred, BB});
-
- if (DTU)
- DTU->applyUpdates(Updates);
-
+ PerformValueComparisonIntoPredecessorFolding(TI, CV, PTI, Builder);
Changed = true;
}
}
More information about the llvm-branch-commits
mailing list