[llvm] r346769 - [CSP, Cloning] Update DuplicateInstructionsInSplitBetween to use DomTreeUpdater.
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 13 09:54:43 PST 2018
Author: fhahn
Date: Tue Nov 13 09:54:43 2018
New Revision: 346769
URL: http://llvm.org/viewvc/llvm-project?rev=346769&view=rev
Log:
[CSP, Cloning] Update DuplicateInstructionsInSplitBetween to use DomTreeUpdater.
This patch updates DuplicateInstructionsInSplitBetween to update a DTU
instead of applying updates to the DT directly.
Given that there only are 2 users, also updated them in this patch to
avoid churn.
I slightly moved the code in CallSiteSplitting around to reduce the
places where we have to pass in DTU. If necessary, I could split those
changes in a separate patch.
This fixes missing DT updates when dealing with musttail calls in
CallSiteSplitting, by using DTU->deleteBB.
Reviewers: junbuml, kuhar, NutshellySima, indutny, brzycki
Reviewed By: NutshellySima
Modified:
llvm/trunk/include/llvm/Transforms/Utils/Cloning.h
llvm/trunk/lib/Transforms/Scalar/CallSiteSplitting.cpp
llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp
llvm/trunk/test/Transforms/CallSiteSplitting/musttail.ll
llvm/trunk/unittests/Transforms/Utils/CloningTest.cpp
Modified: llvm/trunk/include/llvm/Transforms/Utils/Cloning.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/Cloning.h?rev=346769&r1=346768&r2=346769&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/Utils/Cloning.h (original)
+++ llvm/trunk/include/llvm/Transforms/Utils/Cloning.h Tue Nov 13 09:54:43 2018
@@ -47,6 +47,7 @@ class LoopInfo;
class Module;
class ProfileSummaryInfo;
class ReturnInst;
+class DomTreeUpdater;
/// Return an exact copy of the specified module
std::unique_ptr<Module> CloneModule(const Module &M);
@@ -262,11 +263,12 @@ void remapInstructionsInBlocks(const Sma
/// we replace them with the uses of corresponding Phi inputs. ValueMapping
/// is used to map the original instructions from BB to their newly-created
/// copies. Returns the split block.
-BasicBlock *
-DuplicateInstructionsInSplitBetween(BasicBlock *BB, BasicBlock *PredBB,
- Instruction *StopAt,
- ValueToValueMapTy &ValueMapping,
- DominatorTree *DT = nullptr);
+BasicBlock *DuplicateInstructionsInSplitBetween(BasicBlock *BB,
+ BasicBlock *PredBB,
+ Instruction *StopAt,
+ ValueToValueMapTy &ValueMapping,
+ DomTreeUpdater &DTU);
+
} // end namespace llvm
#endif // LLVM_TRANSFORMS_UTILS_CLONING_H
Modified: llvm/trunk/lib/Transforms/Scalar/CallSiteSplitting.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/CallSiteSplitting.cpp?rev=346769&r1=346768&r2=346769&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/CallSiteSplitting.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/CallSiteSplitting.cpp Tue Nov 13 09:54:43 2018
@@ -302,7 +302,7 @@ static void copyMustTailReturn(BasicBloc
static void splitCallSite(
CallSite CS,
const SmallVectorImpl<std::pair<BasicBlock *, ConditionsTy>> &Preds,
- DominatorTree *DT) {
+ DomTreeUpdater &DTU) {
Instruction *Instr = CS.getInstruction();
BasicBlock *TailBB = Instr->getParent();
bool IsMustTailCall = CS.isMustTailCall();
@@ -327,7 +327,7 @@ static void splitCallSite(
BasicBlock *PredBB = Preds[i].first;
BasicBlock *SplitBlock = DuplicateInstructionsInSplitBetween(
TailBB, PredBB, &*std::next(Instr->getIterator()), ValueToValueMaps[i],
- DT);
+ DTU);
assert(SplitBlock && "Unexpected new basic block split.");
Instruction *NewCI =
@@ -369,7 +369,7 @@ static void splitCallSite(
Splits[i]->getTerminator()->eraseFromParent();
// Erase the tail block once done with musttail patching
- TailBB->eraseFromParent();
+ DTU.deleteBB(TailBB);
return;
}
@@ -438,21 +438,25 @@ static bool isPredicatedOnPHI(CallSite C
return false;
}
-static bool tryToSplitOnPHIPredicatedArgument(CallSite CS, DominatorTree *DT) {
+using PredsWithCondsTy = SmallVector<std::pair<BasicBlock *, ConditionsTy>, 2>;
+
+// Check if any of the arguments in CS are predicated on a PHI node and return
+// the set of predecessors we should use for splitting.
+static PredsWithCondsTy shouldSplitOnPHIPredicatedArgument(CallSite CS) {
if (!isPredicatedOnPHI(CS))
- return false;
+ return {};
auto Preds = getTwoPredecessors(CS.getInstruction()->getParent());
- SmallVector<std::pair<BasicBlock *, ConditionsTy>, 2> PredsCS = {
- {Preds[0], {}}, {Preds[1], {}}};
- splitCallSite(CS, PredsCS, DT);
- return true;
+ return {{Preds[0], {}}, {Preds[1], {}}};
}
-static bool tryToSplitOnPredicatedArgument(CallSite CS, DominatorTree *DT) {
+// Checks if any of the arguments in CS are predicated in a predecessor and
+// returns a list of predecessors with the conditions that hold on their edges
+// to CS.
+static PredsWithCondsTy shouldSplitOnPredicatedArgument(CallSite CS) {
auto Preds = getTwoPredecessors(CS.getInstruction()->getParent());
if (Preds[0] == Preds[1])
- return false;
+ return {};
SmallVector<std::pair<BasicBlock *, ConditionsTy>, 2> PredsCS;
for (auto *Pred : make_range(Preds.rbegin(), Preds.rend())) {
@@ -464,22 +468,31 @@ static bool tryToSplitOnPredicatedArgume
if (all_of(PredsCS, [](const std::pair<BasicBlock *, ConditionsTy> &P) {
return P.second.empty();
}))
- return false;
+ return {};
- splitCallSite(CS, PredsCS, DT);
- return true;
+ return PredsCS;
}
static bool tryToSplitCallSite(CallSite CS, TargetTransformInfo &TTI,
- DominatorTree *DT) {
+ DomTreeUpdater &DTU) {
+ // Check if we can split the call site.
if (!CS.arg_size() || !canSplitCallSite(CS, TTI))
return false;
- return tryToSplitOnPredicatedArgument(CS, DT) ||
- tryToSplitOnPHIPredicatedArgument(CS, DT);
+
+ auto PredsWithConds = shouldSplitOnPredicatedArgument(CS);
+ if (PredsWithConds.empty())
+ PredsWithConds = shouldSplitOnPHIPredicatedArgument(CS);
+ if (PredsWithConds.empty())
+ return false;
+
+ splitCallSite(CS, PredsWithConds, DTU);
+ return true;
}
static bool doCallSiteSplitting(Function &F, TargetLibraryInfo &TLI,
TargetTransformInfo &TTI, DominatorTree *DT) {
+
+ DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Lazy);
bool Changed = false;
for (Function::iterator BI = F.begin(), BE = F.end(); BI != BE;) {
BasicBlock &BB = *BI++;
@@ -503,7 +516,7 @@ static bool doCallSiteSplitting(Function
// Check if such path is possible before attempting the splitting.
bool IsMustTail = CS.isMustTailCall();
- Changed |= tryToSplitCallSite(CS, TTI, DT);
+ Changed |= tryToSplitCallSite(CS, TTI, DTU);
// There're no interesting instructions after this. The call site
// itself might have been erased on splitting.
Modified: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=346769&r1=346768&r2=346769&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Tue Nov 13 09:54:43 2018
@@ -2654,28 +2654,16 @@ bool JumpThreadingPass::ThreadGuard(Basi
// Duplicate all instructions before the guard and the guard itself to the
// branch where implication is not proved.
BasicBlock *GuardedBlock = DuplicateInstructionsInSplitBetween(
- BB, PredGuardedBlock, AfterGuard, GuardedMapping);
+ BB, PredGuardedBlock, AfterGuard, GuardedMapping, *DTU);
assert(GuardedBlock && "Could not create the guarded block?");
// Duplicate all instructions before the guard in the unguarded branch.
// Since we have successfully duplicated the guarded block and this block
// has fewer instructions, we expect it to succeed.
BasicBlock *UnguardedBlock = DuplicateInstructionsInSplitBetween(
- BB, PredUnguardedBlock, Guard, UnguardedMapping);
+ BB, PredUnguardedBlock, Guard, UnguardedMapping, *DTU);
assert(UnguardedBlock && "Could not create the unguarded block?");
LLVM_DEBUG(dbgs() << "Moved guard " << *Guard << " to block "
<< GuardedBlock->getName() << "\n");
- // DuplicateInstructionsInSplitBetween inserts a new block "BB.split" between
- // PredBB and BB. We need to perform two inserts and one delete for each of
- // the above calls to update Dominators.
- DTU->applyUpdates(
- {// Guarded block split.
- {DominatorTree::Delete, PredGuardedBlock, BB},
- {DominatorTree::Insert, PredGuardedBlock, GuardedBlock},
- {DominatorTree::Insert, GuardedBlock, BB},
- // Unguarded block split.
- {DominatorTree::Delete, PredUnguardedBlock, BB},
- {DominatorTree::Insert, PredUnguardedBlock, UnguardedBlock},
- {DominatorTree::Insert, UnguardedBlock, BB}});
// Some instructions before the guard may still have uses. For them, we need
// to create Phi nodes merging their copies in both guarded and unguarded
// branches. Those instructions that have no uses can be just removed.
Modified: llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp?rev=346769&r1=346768&r2=346769&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp Tue Nov 13 09:54:43 2018
@@ -18,11 +18,11 @@
#include "llvm/Analysis/ConstantFolding.h"
#include "llvm/Analysis/InstructionSimplify.h"
#include "llvm/Analysis/LoopInfo.h"
-#include "llvm/Transforms/Utils/Local.h"
#include "llvm/IR/CFG.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/DebugInfo.h"
#include "llvm/IR/DerivedTypes.h"
+#include "llvm/IR/DomTreeUpdater.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/GlobalVariable.h"
#include "llvm/IR/Instructions.h"
@@ -32,6 +32,7 @@
#include "llvm/IR/Module.h"
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
#include "llvm/Transforms/Utils/Cloning.h"
+#include "llvm/Transforms/Utils/Local.h"
#include "llvm/Transforms/Utils/ValueMapper.h"
#include <map>
using namespace llvm;
@@ -795,11 +796,12 @@ Loop *llvm::cloneLoopWithPreheader(Basic
/// Duplicate non-Phi instructions from the beginning of block up to
/// StopAt instruction into a split block between BB and its predecessor.
-BasicBlock *
-llvm::DuplicateInstructionsInSplitBetween(BasicBlock *BB, BasicBlock *PredBB,
- Instruction *StopAt,
- ValueToValueMapTy &ValueMapping,
- DominatorTree *DT) {
+BasicBlock *llvm::DuplicateInstructionsInSplitBetween(
+ BasicBlock *BB, BasicBlock *PredBB, Instruction *StopAt,
+ ValueToValueMapTy &ValueMapping, DomTreeUpdater &DTU) {
+
+ assert(count(successors(PredBB), BB) == 1 &&
+ "There must be a single edge between PredBB and BB!");
// We are going to have to map operands from the original BB block to the new
// copy of the block 'NewBB'. If there are PHI nodes in BB, evaluate them to
// account for entry from PredBB.
@@ -807,10 +809,16 @@ llvm::DuplicateInstructionsInSplitBetwee
for (; PHINode *PN = dyn_cast<PHINode>(BI); ++BI)
ValueMapping[PN] = PN->getIncomingValueForBlock(PredBB);
- BasicBlock *NewBB = SplitEdge(PredBB, BB, DT);
+ BasicBlock *NewBB = SplitEdge(PredBB, BB);
NewBB->setName(PredBB->getName() + ".split");
Instruction *NewTerm = NewBB->getTerminator();
+ // FIXME: SplitEdge does not yet take a DTU, so we include the split edge
+ // in the update set here.
+ DTU.applyUpdates({{DominatorTree::Delete, PredBB, BB},
+ {DominatorTree::Insert, PredBB, NewBB},
+ {DominatorTree::Insert, NewBB, BB}});
+
// Clone the non-phi instructions of BB into NewBB, keeping track of the
// mapping and using it to remap operands in the cloned instructions.
// Stop once we see the terminator too. This covers the case where BB's
Modified: llvm/trunk/test/Transforms/CallSiteSplitting/musttail.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CallSiteSplitting/musttail.ll?rev=346769&r1=346768&r2=346769&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/CallSiteSplitting/musttail.ll (original)
+++ llvm/trunk/test/Transforms/CallSiteSplitting/musttail.ll Tue Nov 13 09:54:43 2018
@@ -1,4 +1,4 @@
-; RUN: opt < %s -callsite-splitting -S | FileCheck %s
+; RUN: opt < %s -callsite-splitting -verify-dom-info -S | FileCheck %s
;CHECK-LABEL: @caller
;CHECK-LABEL: Top.split:
Modified: llvm/trunk/unittests/Transforms/Utils/CloningTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/CloningTest.cpp?rev=346769&r1=346768&r2=346769&view=diff
==============================================================================
--- llvm/trunk/unittests/Transforms/Utils/CloningTest.cpp (original)
+++ llvm/trunk/unittests/Transforms/Utils/CloningTest.cpp Tue Nov 13 09:54:43 2018
@@ -14,6 +14,7 @@
#include "llvm/IR/Constant.h"
#include "llvm/IR/DIBuilder.h"
#include "llvm/IR/DebugInfo.h"
+#include "llvm/IR/DomTreeUpdater.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/IRBuilder.h"
#include "llvm/IR/InstIterator.h"
@@ -224,9 +225,11 @@ TEST_F(CloneInstruction, DuplicateInstru
Instruction *SubInst = cast<Instruction>(Builder2.CreateSub(MulInst, V));
Builder2.CreateRetVoid();
+ // Dummy DTU.
ValueToValueMapTy Mapping;
-
- auto Split = DuplicateInstructionsInSplitBetween(BB2, BB1, SubInst, Mapping);
+ DomTreeUpdater DTU(DomTreeUpdater::UpdateStrategy::Lazy);
+ auto Split =
+ DuplicateInstructionsInSplitBetween(BB2, BB1, SubInst, Mapping, DTU);
EXPECT_TRUE(Split);
EXPECT_EQ(Mapping.size(), 2u);
@@ -271,9 +274,11 @@ TEST_F(CloneInstruction, DuplicateInstru
Instruction *SubInst = cast<Instruction>(Builder2.CreateSub(MulInst, V));
Builder2.CreateBr(BB2);
+ // Dummy DTU.
+ DomTreeUpdater DTU(DomTreeUpdater::UpdateStrategy::Lazy);
ValueToValueMapTy Mapping;
-
- auto Split = DuplicateInstructionsInSplitBetween(BB2, BB2, BB2->getTerminator(), Mapping);
+ auto Split = DuplicateInstructionsInSplitBetween(
+ BB2, BB2, BB2->getTerminator(), Mapping, DTU);
EXPECT_TRUE(Split);
EXPECT_EQ(Mapping.size(), 3u);
@@ -322,9 +327,11 @@ TEST_F(CloneInstruction, DuplicateInstru
Instruction *SubInst = cast<Instruction>(Builder2.CreateSub(MulInst, V));
Builder2.CreateBr(BB2);
+ // Dummy DTU.
+ DomTreeUpdater DTU(DomTreeUpdater::UpdateStrategy::Lazy);
ValueToValueMapTy Mapping;
-
- auto Split = DuplicateInstructionsInSplitBetween(BB2, BB2, SubInst, Mapping);
+ auto Split =
+ DuplicateInstructionsInSplitBetween(BB2, BB2, SubInst, Mapping, DTU);
EXPECT_TRUE(Split);
EXPECT_EQ(Mapping.size(), 2u);
More information about the llvm-commits
mailing list