RPCS3/llvm-mirror
1
0
Fork 0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2024-11-22 18:54:02 +01:00
Code Issues Projects Releases Wiki Activity

[DTU] Refine the interface and logic of applyUpdates

Browse Source 
Summary:
This patch separates two semantics of `applyUpdates`:
1. User provides an accurate CFG diff and the dominator tree is updated according to the difference of `the number of edge insertions` and `the number of edge deletions` to infer the status of an edge before and after the update.
2. User provides a sequence of hints. Updates mentioned in this sequence might never happened and even duplicated.

Logic changes:

Previously, removing invalid updates is considered a side-effect of deduplication and is not guaranteed to be reliable. To handle the second semantic, `applyUpdates` does validity checking before deduplication, which can cause updates that have already been applied to be submitted again. Then, different calls to `applyUpdates` might cause unintended consequences, for example,
```
DTU(Lazy) and Edge A->B exists.
1. DTU.applyUpdates({{Delete, A, B}, {Insert, A, B}}) // User expects these 2 updates result in a no-op, but {Insert, A, B} is queued
2. Remove A->B
3. DTU.applyUpdates({{Delete, A, B}}) // DTU cancels this update with {Insert, A, B} mentioned above together (Unintended)
```
But by restricting the precondition that updates of an edge need to be strictly ordered as how CFG changes were made, we can infer the initial status of this edge to resolve this issue.

Interface changes:
The second semantic of `applyUpdates`  is separated to `applyUpdatesPermissive`.
These changes enable DTU(Lazy) to use the first semantic if needed, which is quite useful in `transforms/utils`.

Reviewers: kuhar, brzycki, dmgreen, grosser

Reviewed By: brzycki

Subscribers: hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D58170

llvm-svn: 354669
This commit is contained in:
Chijun Sima 2019-02-22 13:48:38 +00:00
parent 62ceaab06e
commit b98fe759b3
8 changed files with 207 additions and 129 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
include/llvm/Analysis/DomTreeUpdater.h
View File

@ -103,8 +103,24 @@ public:
/// Although GenericDomTree provides several update primitives, /// Although GenericDomTree provides several update primitives,
/// it is not encouraged to use these APIs directly. /// it is not encouraged to use these APIs directly.
/// Submit updates to all available trees. Under Eager UpdateStrategy with /// Submit updates to all available trees.
/// ForceRemoveDuplicates enabled or under Lazy UpdateStrategy, it will /// The Eager Strategy flushes updates immediately while the Lazy Strategy
/// queues the updates.
///
/// Note: The "existence" of an edge in a CFG refers to the CFG which DTU is
/// in sync with + all updates before that single update.
///
/// CAUTION!
/// 1. It is required for the state of the LLVM IR to be updated
/// *before* submitting the updates because the internal update routine will
/// analyze the current state of the CFG to determine whether an update
/// is valid.
/// 2. It is illegal to submit any update that has already been submitted,
/// i.e., you are supposed not to insert an existent edge or delete a
/// nonexistent edge.
void applyUpdates(ArrayRef<DominatorTree::UpdateType> Updates);
/// Submit updates to all available trees. It will also
/// 1. discard duplicated updates, /// 1. discard duplicated updates,
/// 2. remove invalid updates. (Invalid updates means deletion of an edge that /// 2. remove invalid updates. (Invalid updates means deletion of an edge that
/// still exists or insertion of an edge that does not exist.) /// still exists or insertion of an edge that does not exist.)
@ -122,8 +138,10 @@ public:
/// 2. It is illegal to submit any update that has already been submitted, /// 2. It is illegal to submit any update that has already been submitted,
/// i.e., you are supposed not to insert an existent edge or delete a /// i.e., you are supposed not to insert an existent edge or delete a
/// nonexistent edge. /// nonexistent edge.
void applyUpdates(ArrayRef<DominatorTree::UpdateType> Updates, /// 3. It is only legal to submit updates to an edge in the order CFG changes
bool ForceRemoveDuplicates = false); /// are made. The order you submit updates on different edges is not
/// restricted.
void applyUpdatesPermissive(ArrayRef<DominatorTree::UpdateType> Updates);
/// Notify DTU that the entry block was replaced. /// Notify DTU that the entry block was replaced.
/// Recalculate all available trees and flush all BasicBlocks /// Recalculate all available trees and flush all BasicBlocks
@ -149,7 +167,7 @@ public:
/// submitted. } /// submitted. }
LLVM_ATTRIBUTE_DEPRECATED(void insertEdgeRelaxed(BasicBlock *From, LLVM_ATTRIBUTE_DEPRECATED(void insertEdgeRelaxed(BasicBlock *From,
BasicBlock *To), BasicBlock *To),
"Use applyUpdates() instead."); "Use applyUpdatesPermissive() instead.");
/// \deprecated { Submit an edge deletion to all available trees. The Eager /// \deprecated { Submit an edge deletion to all available trees. The Eager
/// Strategy flushes this update immediately while the Lazy Strategy queues /// Strategy flushes this update immediately while the Lazy Strategy queues
@ -171,7 +189,7 @@ public:
/// submitted. } /// submitted. }
LLVM_ATTRIBUTE_DEPRECATED(void deleteEdgeRelaxed(BasicBlock *From, LLVM_ATTRIBUTE_DEPRECATED(void deleteEdgeRelaxed(BasicBlock *From,
BasicBlock *To), BasicBlock *To),
"Use applyUpdates() instead."); "Use applyUpdatesPermissive() instead.");
/// Delete DelBB. DelBB will be removed from its Parent and /// Delete DelBB. DelBB will be removed from its Parent and
/// erased from available trees if it exists and finally get deleted. /// erased from available trees if it exists and finally get deleted.
@ -260,11 +278,6 @@ private:
/// Returns true if at least one BasicBlock is deleted. /// Returns true if at least one BasicBlock is deleted.
bool forceFlushDeletedBB(); bool forceFlushDeletedBB();
/// Deduplicate and remove unnecessary updates (no-ops) when using Lazy
/// UpdateStrategy. Returns true if the update is queued for update.
bool applyLazyUpdate(DominatorTree::UpdateKind Kind, BasicBlock *From,
BasicBlock *To);
/// Helper function to apply all pending DomTree updates. /// Helper function to apply all pending DomTree updates.
void applyDomTreeUpdates(); void applyDomTreeUpdates();

123
lib/Analysis/DomTreeUpdater.cpp
View File

@ -12,11 +12,13 @@
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
#include "llvm/Analysis/DomTreeUpdater.h" #include "llvm/Analysis/DomTreeUpdater.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/Analysis/PostDominators.h" #include "llvm/Analysis/PostDominators.h"
#include "llvm/IR/Dominators.h" #include "llvm/IR/Dominators.h"
#include "llvm/Support/GenericDomTree.h" #include "llvm/Support/GenericDomTree.h"
#include <algorithm> #include <algorithm>
#include <functional> #include <functional>
#include <utility>
namespace llvm { namespace llvm {
@ -53,41 +55,6 @@ bool DomTreeUpdater::isSelfDominance(
return Update.getFrom() == Update.getTo(); return Update.getFrom() == Update.getTo();
} }
bool DomTreeUpdater::applyLazyUpdate(DominatorTree::UpdateKind Kind,
BasicBlock *From, BasicBlock *To) {
assert((DT || PDT) &&
"Call applyLazyUpdate() when both DT and PDT are nullptrs.");
assert(Strategy == DomTreeUpdater::UpdateStrategy::Lazy &&
"Call applyLazyUpdate() with Eager strategy error");
// Analyze pending updates to determine if the update is unnecessary.
const DominatorTree::UpdateType Update = {Kind, From, To};
const DominatorTree::UpdateType Invert = {Kind != DominatorTree::Insert
? DominatorTree::Insert
: DominatorTree::Delete,
From, To};
// Only check duplicates in updates that are not applied by both trees.
auto I =
PendUpdates.begin() + std::max(PendDTUpdateIndex, PendPDTUpdateIndex);
const auto E = PendUpdates.end();
assert(I <= E && "Iterator out of range.");
for (; I != E; ++I) {
if (Update == *I)
return false; // Discard duplicate updates.
if (Invert == *I) {
// Update and Invert are both valid (equivalent to a no-op). Remove
// Invert from PendUpdates and discard the Update.
PendUpdates.erase(I);
return false;
}
}
PendUpdates.push_back(Update); // Save the valid update.
return true;
}
void DomTreeUpdater::applyDomTreeUpdates() { void DomTreeUpdater::applyDomTreeUpdates() {
// No pending DomTreeUpdates. // No pending DomTreeUpdates.
if (Strategy != UpdateStrategy::Lazy || !DT) if (Strategy != UpdateStrategy::Lazy || !DT)
@ -261,31 +228,15 @@ void DomTreeUpdater::validateDeleteBB(BasicBlock *DelBB) {
new UnreachableInst(DelBB->getContext(), DelBB); new UnreachableInst(DelBB->getContext(), DelBB);
} }
void DomTreeUpdater::applyUpdates(ArrayRef<DominatorTree::UpdateType> Updates, void DomTreeUpdater::applyUpdates(ArrayRef<DominatorTree::UpdateType> Updates) {
bool ForceRemoveDuplicates) {
if (!DT && !PDT) if (!DT && !PDT)
return; return;
if (Strategy == UpdateStrategy::Lazy || ForceRemoveDuplicates) { if (Strategy == UpdateStrategy::Lazy) {
SmallVector<DominatorTree::UpdateType, 8> Seen;
for (const auto U : Updates) for (const auto U : Updates)
// For Lazy UpdateStrategy, avoid duplicates to applyLazyUpdate() to save if (!isSelfDominance(U))
// on analysis. PendUpdates.push_back(U);
if (llvm::none_of(
Seen,
[U](const DominatorTree::UpdateType S) { return S == U; }) &&
isUpdateValid(U) && !isSelfDominance(U)) {
Seen.push_back(U);
if (Strategy == UpdateStrategy::Lazy)
applyLazyUpdate(U.getKind(), U.getFrom(), U.getTo());
}
if (Strategy == UpdateStrategy::Lazy)
return;
if (DT)
DT->applyUpdates(Seen);
if (PDT)
PDT->applyUpdates(Seen);
return; return;
} }
@ -295,6 +246,60 @@ void DomTreeUpdater::applyUpdates(ArrayRef<DominatorTree::UpdateType> Updates,
PDT->applyUpdates(Updates); PDT->applyUpdates(Updates);
} }
void DomTreeUpdater::applyUpdatesPermissive(
ArrayRef<DominatorTree::UpdateType> Updates) {
if (!DT && !PDT)
return;
SmallSet<std::pair<BasicBlock *, BasicBlock *>, 8> Seen;
SmallVector<DominatorTree::UpdateType, 8> DeduplicatedUpdates;
for (const auto U : Updates) {
auto Edge = std::make_pair(U.getFrom(), U.getTo());
// Because it is illegal to submit updates that have already been applied
// and updates to an edge need to be strictly ordered,
// it is safe to infer the existence of an edge from the first update
// to this edge.
// If the first update to an edge is "Delete", it means that the edge
// existed before. If the first update to an edge is "Insert", it means
// that the edge didn't exist before.
//
// For example, if the user submits {{Delete, A, B}, {Insert, A, B}},
// because
// 1. it is illegal to submit updates that have already been applied,
// i.e., user cannot delete an nonexistent edge,
// 2. updates to an edge need to be strictly ordered,
// So, initially edge A -> B existed.
// We can then safely ignore future updates to this edge and directly
// inspect the current CFG:
// a. If the edge still exists, because the user cannot insert an existent
// edge, so both {Delete, A, B}, {Insert, A, B} actually happened and
// resulted in a no-op. DTU won't submit any update in this case.
// b. If the edge doesn't exist, we can then infer that {Delete, A, B}
// actually happened but {Insert, A, B} was an invalid update which never
// happened. DTU will submit {Delete, A, B} in this case.
if (!isSelfDominance(U) && Seen.count(Edge) == 0) {
Seen.insert(Edge);
// If the update doesn't appear in the CFG, it means that
// either the change isn't made or relevant operations
// result in a no-op.
if (isUpdateValid(U)) {
if (isLazy())
PendUpdates.push_back(U);
else
DeduplicatedUpdates.push_back(U);
}
}
}
if (Strategy == UpdateStrategy::Lazy)
return;
if (DT)
DT->applyUpdates(DeduplicatedUpdates);
if (PDT)
PDT->applyUpdates(DeduplicatedUpdates);
}
DominatorTree &DomTreeUpdater::getDomTree() { DominatorTree &DomTreeUpdater::getDomTree() {
assert(DT && "Invalid acquisition of a null DomTree"); assert(DT && "Invalid acquisition of a null DomTree");
applyDomTreeUpdates(); applyDomTreeUpdates();
@ -331,7 +336,7 @@ void DomTreeUpdater::insertEdge(BasicBlock *From, BasicBlock *To) {
return; return;
} }
applyLazyUpdate(DominatorTree::Insert, From, To); PendUpdates.push_back({DominatorTree::Insert, From, To});
} }
void DomTreeUpdater::insertEdgeRelaxed(BasicBlock *From, BasicBlock *To) { void DomTreeUpdater::insertEdgeRelaxed(BasicBlock *From, BasicBlock *To) {
@ -352,7 +357,7 @@ void DomTreeUpdater::insertEdgeRelaxed(BasicBlock *From, BasicBlock *To) {
return; return;
} }
applyLazyUpdate(DominatorTree::Insert, From, To); PendUpdates.push_back({DominatorTree::Insert, From, To});
} }
void DomTreeUpdater::deleteEdge(BasicBlock *From, BasicBlock *To) { void DomTreeUpdater::deleteEdge(BasicBlock *From, BasicBlock *To) {
@ -377,7 +382,7 @@ void DomTreeUpdater::deleteEdge(BasicBlock *From, BasicBlock *To) {
return; return;
} }
applyLazyUpdate(DominatorTree::Delete, From, To); PendUpdates.push_back({DominatorTree::Delete, From, To});
} }
void DomTreeUpdater::deleteEdgeRelaxed(BasicBlock *From, BasicBlock *To) { void DomTreeUpdater::deleteEdgeRelaxed(BasicBlock *From, BasicBlock *To) {
@ -398,7 +403,7 @@ void DomTreeUpdater::deleteEdgeRelaxed(BasicBlock *From, BasicBlock *To) {
return; return;
} }
applyLazyUpdate(DominatorTree::Delete, From, To); PendUpdates.push_back({DominatorTree::Delete, From, To});
} }
void DomTreeUpdater::dropOutOfDateUpdates() { void DomTreeUpdater::dropOutOfDateUpdates() {

2
lib/Transforms/Scalar/CallSiteSplitting.cpp
View File

@ -366,7 +366,7 @@ static void splitCallSite(
assert(Splits.size() == 2 && "Expected exactly 2 splits!"); assert(Splits.size() == 2 && "Expected exactly 2 splits!");
for (unsigned i = 0; i < Splits.size(); i++) { for (unsigned i = 0; i < Splits.size(); i++) {
Splits[i]->getTerminator()->eraseFromParent(); Splits[i]->getTerminator()->eraseFromParent();
DTU.applyUpdates({{DominatorTree::Delete, Splits[i], TailBB}}); DTU.applyUpdatesPermissive({{DominatorTree::Delete, Splits[i], TailBB}});
} }
// Erase the tail block once done with musttail patching // Erase the tail block once done with musttail patching

2
lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
View File

@ -373,7 +373,7 @@ static bool processSwitch(SwitchInst *SI, LazyValueInfo *LVI,
++NumDeadCases; ++NumDeadCases;
Changed = true; Changed = true;
if (--SuccessorsCount[Succ] == 0) if (--SuccessorsCount[Succ] == 0)
DTU.applyUpdates({{DominatorTree::Delete, BB, Succ}}); DTU.applyUpdatesPermissive({{DominatorTree::Delete, BB, Succ}});
continue; continue;
} }
if (State == LazyValueInfo::True) { if (State == LazyValueInfo::True) {

19
lib/Transforms/Scalar/JumpThreading.cpp
View File

@ -1091,7 +1091,7 @@ bool JumpThreadingPass::ProcessBlock(BasicBlock *BB) {
<< "' folding undef terminator: " << *BBTerm << '\n'); << "' folding undef terminator: " << *BBTerm << '\n');
BranchInst::Create(BBTerm->getSuccessor(BestSucc), BBTerm); BranchInst::Create(BBTerm->getSuccessor(BestSucc), BBTerm);
BBTerm->eraseFromParent(); BBTerm->eraseFromParent();
DTU->applyUpdates(Updates); DTU->applyUpdatesPermissive(Updates);
return true; return true;
} }
@ -1159,7 +1159,8 @@ bool JumpThreadingPass::ProcessBlock(BasicBlock *BB) {
ConstantInt::getFalse(CondCmp->getType()); ConstantInt::getFalse(CondCmp->getType());
ReplaceFoldableUses(CondCmp, CI); ReplaceFoldableUses(CondCmp, CI);
} }
DTU->applyUpdates({{DominatorTree::Delete, BB, ToRemoveSucc}}); DTU->applyUpdatesPermissive(
{{DominatorTree::Delete, BB, ToRemoveSucc}});
return true; return true;
} }
@ -1246,7 +1247,7 @@ bool JumpThreadingPass::ProcessImpliedCondition(BasicBlock *BB) {
RemoveSucc->removePredecessor(BB); RemoveSucc->removePredecessor(BB);
BranchInst::Create(KeepSucc, BI); BranchInst::Create(KeepSucc, BI);
BI->eraseFromParent(); BI->eraseFromParent();
DTU->applyUpdates({{DominatorTree::Delete, BB, RemoveSucc}}); DTU->applyUpdatesPermissive({{DominatorTree::Delete, BB, RemoveSucc}});
return true; return true;
} }
CurrentBB = CurrentPred; CurrentBB = CurrentPred;
@ -1676,7 +1677,7 @@ bool JumpThreadingPass::ProcessThreadableEdges(Value *Cond, BasicBlock *BB,
Instruction *Term = BB->getTerminator(); Instruction *Term = BB->getTerminator();
BranchInst::Create(OnlyDest, Term); BranchInst::Create(OnlyDest, Term);
Term->eraseFromParent(); Term->eraseFromParent();
DTU->applyUpdates(Updates); DTU->applyUpdatesPermissive(Updates);
// If the condition is now dead due to the removal of the old terminator, // If the condition is now dead due to the removal of the old terminator,
// erase it. // erase it.
@ -2018,7 +2019,7 @@ bool JumpThreadingPass::ThreadEdge(BasicBlock *BB,
} }
// Enqueue required DT updates. // Enqueue required DT updates.
DTU->applyUpdates({{DominatorTree::Insert, NewBB, SuccBB}, DTU->applyUpdatesPermissive({{DominatorTree::Insert, NewBB, SuccBB},
{DominatorTree::Insert, PredBB, NewBB}, {DominatorTree::Insert, PredBB, NewBB},
{DominatorTree::Delete, PredBB, BB}}); {DominatorTree::Delete, PredBB, BB}});
@ -2114,7 +2115,7 @@ BasicBlock *JumpThreadingPass::SplitBlockPreds(BasicBlock *BB,
BFI->setBlockFreq(NewBB, NewBBFreq.getFrequency()); BFI->setBlockFreq(NewBB, NewBBFreq.getFrequency());
} }
DTU->applyUpdates(Updates); DTU->applyUpdatesPermissive(Updates);
return NewBBs[0]; return NewBBs[0];
} }
@ -2387,7 +2388,7 @@ bool JumpThreadingPass::DuplicateCondBranchOnPHIIntoPred(
// Remove the unconditional branch at the end of the PredBB block. // Remove the unconditional branch at the end of the PredBB block.
OldPredBranch->eraseFromParent(); OldPredBranch->eraseFromParent();
DTU->applyUpdates(Updates); DTU->applyUpdatesPermissive(Updates);
++NumDupes; ++NumDupes;
return true; return true;
@ -2423,7 +2424,7 @@ void JumpThreadingPass::UnfoldSelectInstr(BasicBlock *Pred, BasicBlock *BB,
// The select is now dead. // The select is now dead.
SI->eraseFromParent(); SI->eraseFromParent();
DTU->applyUpdates({{DominatorTree::Insert, NewBB, BB}, DTU->applyUpdatesPermissive({{DominatorTree::Insert, NewBB, BB},
{DominatorTree::Insert, Pred, NewBB}}); {DominatorTree::Insert, Pred, NewBB}});
// Update any other PHI nodes in BB. // Update any other PHI nodes in BB.
@ -2601,7 +2602,7 @@ bool JumpThreadingPass::TryToUnfoldSelectInCurrBB(BasicBlock *BB) {
Updates.push_back({DominatorTree::Delete, BB, Succ}); Updates.push_back({DominatorTree::Delete, BB, Succ});
Updates.push_back({DominatorTree::Insert, SplitBB, Succ}); Updates.push_back({DominatorTree::Insert, SplitBB, Succ});
} }
DTU->applyUpdates(Updates); DTU->applyUpdatesPermissive(Updates);
return true; return true;
} }
return false; return false;

4
lib/Transforms/Utils/BasicBlockUtils.cpp
View File

@ -101,7 +101,7 @@ void llvm::DeleteDeadBlocks(ArrayRef <BasicBlock *> BBs, DomTreeUpdater *DTU,
DetatchDeadBlocks(BBs, DTU ? &Updates : nullptr, KeepOneInputPHIs); DetatchDeadBlocks(BBs, DTU ? &Updates : nullptr, KeepOneInputPHIs);
if (DTU) if (DTU)
DTU->applyUpdates(Updates, /*ForceRemoveDuplicates*/ true); DTU->applyUpdatesPermissive(Updates);
for (BasicBlock *BB : BBs) for (BasicBlock *BB : BBs)
if (DTU) if (DTU)
@ -235,7 +235,7 @@ bool llvm::MergeBlockIntoPredecessor(BasicBlock *BB, DomTreeUpdater *DTU,
isa<UnreachableInst>(BB->getTerminator()) && isa<UnreachableInst>(BB->getTerminator()) &&
"The successor list of BB isn't empty before " "The successor list of BB isn't empty before "
"applying corresponding DTU updates."); "applying corresponding DTU updates.");
DTU->applyUpdates(Updates, /*ForceRemoveDuplicates*/ true); DTU->applyUpdatesPermissive(Updates);
DTU->deleteBB(BB); DTU->deleteBB(BB);
} }

32
lib/Transforms/Utils/Local.cpp
View File

@ -128,8 +128,7 @@ bool llvm::ConstantFoldTerminator(BasicBlock *BB, bool DeleteDeadConditions,
Builder.CreateBr(Destination); Builder.CreateBr(Destination);
BI->eraseFromParent(); BI->eraseFromParent();
if (DTU) if (DTU)
DTU->applyUpdates({{DominatorTree::Delete, BB, OldDest}}, DTU->applyUpdatesPermissive({{DominatorTree::Delete, BB, OldDest}});
/*ForceRemoveDuplicates*/ true);
return true; return true;
} }
@ -205,8 +204,8 @@ bool llvm::ConstantFoldTerminator(BasicBlock *BB, bool DeleteDeadConditions,
i = SI->removeCase(i); i = SI->removeCase(i);
e = SI->case_end(); e = SI->case_end();
if (DTU) if (DTU)
DTU->applyUpdates({{DominatorTree::Delete, ParentBB, DefaultDest}}, DTU->applyUpdatesPermissive(
/*ForceRemoveDuplicates*/ true); {{DominatorTree::Delete, ParentBB, DefaultDest}});
continue; continue;
} }
@ -254,7 +253,7 @@ bool llvm::ConstantFoldTerminator(BasicBlock *BB, bool DeleteDeadConditions,
if (DeleteDeadConditions) if (DeleteDeadConditions)
RecursivelyDeleteTriviallyDeadInstructions(Cond, TLI); RecursivelyDeleteTriviallyDeadInstructions(Cond, TLI);
if (DTU) if (DTU)
DTU->applyUpdates(Updates, /*ForceRemoveDuplicates*/ true); DTU->applyUpdatesPermissive(Updates);
return true; return true;
} }
@ -332,7 +331,7 @@ bool llvm::ConstantFoldTerminator(BasicBlock *BB, bool DeleteDeadConditions,
} }
if (DTU) if (DTU)
DTU->applyUpdates(Updates, /*ForceRemoveDuplicates*/ true); DTU->applyUpdatesPermissive(Updates);
return true; return true;
} }
} }
@ -666,8 +665,7 @@ void llvm::RemovePredecessorAndSimplify(BasicBlock *BB, BasicBlock *Pred,
if (PhiIt != OldPhiIt) PhiIt = &BB->front(); if (PhiIt != OldPhiIt) PhiIt = &BB->front();
} }
if (DTU) if (DTU)
DTU->applyUpdates({{DominatorTree::Delete, Pred, BB}}, DTU->applyUpdatesPermissive({{DominatorTree::Delete, Pred, BB}});
/*ForceRemoveDuplicates*/ true);
} }
/// MergeBasicBlockIntoOnlyPred - DestBB is a block with one predecessor and its /// MergeBasicBlockIntoOnlyPred - DestBB is a block with one predecessor and its
@ -736,7 +734,7 @@ void llvm::MergeBasicBlockIntoOnlyPred(BasicBlock *DestBB,
isa<UnreachableInst>(PredBB->getTerminator()) && isa<UnreachableInst>(PredBB->getTerminator()) &&
"The successor list of PredBB isn't empty before " "The successor list of PredBB isn't empty before "
"applying corresponding DTU updates."); "applying corresponding DTU updates.");
DTU->applyUpdates(Updates, /*ForceRemoveDuplicates*/ true); DTU->applyUpdatesPermissive(Updates);
DTU->deleteBB(PredBB); DTU->deleteBB(PredBB);
// Recalculation of DomTree is needed when updating a forward DomTree and // Recalculation of DomTree is needed when updating a forward DomTree and
// the Entry BB is replaced. // the Entry BB is replaced.
@ -1078,7 +1076,7 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
"applying corresponding DTU updates."); "applying corresponding DTU updates.");
if (DTU) { if (DTU) {
DTU->applyUpdates(Updates, /*ForceRemoveDuplicates*/ true); DTU->applyUpdatesPermissive(Updates);
DTU->deleteBB(BB); DTU->deleteBB(BB);
} else { } else {
BB->eraseFromParent(); // Delete the old basic block. BB->eraseFromParent(); // Delete the old basic block.
@ -1942,7 +1940,7 @@ unsigned llvm::changeToUnreachable(Instruction *I, bool UseLLVMTrap,
++NumInstrsRemoved; ++NumInstrsRemoved;
} }
if (DTU) if (DTU)
DTU->applyUpdates(Updates, /*ForceRemoveDuplicates*/ true); DTU->applyUpdatesPermissive(Updates);
return NumInstrsRemoved; return NumInstrsRemoved;
} }
@ -1970,8 +1968,7 @@ static void changeToCall(InvokeInst *II, DomTreeUpdater *DTU = nullptr) {
UnwindDestBB->removePredecessor(BB); UnwindDestBB->removePredecessor(BB);
II->eraseFromParent(); II->eraseFromParent();
if (DTU) if (DTU)
DTU->applyUpdates({{DominatorTree::Delete, BB, UnwindDestBB}}, DTU->applyUpdatesPermissive({{DominatorTree::Delete, BB, UnwindDestBB}});
/*ForceRemoveDuplicates*/ true);
} }
BasicBlock *llvm::changeToInvokeAndSplitBasicBlock(CallInst *CI, BasicBlock *llvm::changeToInvokeAndSplitBasicBlock(CallInst *CI,
@ -2118,8 +2115,8 @@ static bool markAliveBlocks(Function &F,
UnwindDestBB->removePredecessor(II->getParent()); UnwindDestBB->removePredecessor(II->getParent());
II->eraseFromParent(); II->eraseFromParent();
if (DTU) if (DTU)
DTU->applyUpdates({{DominatorTree::Delete, BB, UnwindDestBB}}, DTU->applyUpdatesPermissive(
/*ForceRemoveDuplicates*/ true); {{DominatorTree::Delete, BB, UnwindDestBB}});
} else } else
changeToCall(II, DTU); changeToCall(II, DTU);
Changed = true; Changed = true;
@ -2208,8 +2205,7 @@ void llvm::removeUnwindEdge(BasicBlock *BB, DomTreeUpdater *DTU) {
TI->replaceAllUsesWith(NewTI); TI->replaceAllUsesWith(NewTI);
TI->eraseFromParent(); TI->eraseFromParent();
if (DTU) if (DTU)
DTU->applyUpdates({{DominatorTree::Delete, BB, UnwindDest}}, DTU->applyUpdatesPermissive({{DominatorTree::Delete, BB, UnwindDest}});
/*ForceRemoveDuplicates*/ true);
} }
/// removeUnreachableBlocks - Remove blocks that are not reachable, even /// removeUnreachableBlocks - Remove blocks that are not reachable, even
@ -2274,7 +2270,7 @@ bool llvm::removeUnreachableBlocks(Function &F, LazyValueInfo *LVI,
} }
if (DTU) { if (DTU) {
DTU->applyUpdates(Updates, /*ForceRemoveDuplicates*/ true); DTU->applyUpdatesPermissive(Updates);
bool Deleted = false; bool Deleted = false;
for (auto *BB : DeadBlockSet) { for (auto *BB : DeadBlockSet) {
if (DTU->isBBPendingDeletion(BB)) if (DTU->isBBPendingDeletion(BB))

113
unittests/Analysis/DomTreeUpdaterTest.cpp
View File

@ -71,9 +71,9 @@ TEST(DomTreeUpdater, EagerUpdateBasicOperations) {
SwitchInst *SI = dyn_cast<SwitchInst>(BB0->getTerminator()); SwitchInst *SI = dyn_cast<SwitchInst>(BB0->getTerminator());
ASSERT_NE(SI, nullptr) << "Couldn't get SwitchInst."; ASSERT_NE(SI, nullptr) << "Couldn't get SwitchInst.";
DTU.applyUpdates( DTU.applyUpdatesPermissive(
{{DominatorTree::Insert, BB0, BB0}, {DominatorTree::Delete, BB0, BB0}}, {{DominatorTree::Insert, BB0, BB0}, {DominatorTree::Delete, BB0, BB0}});
/*ForceRemoveDuplicates*/ true); ASSERT_FALSE(DTU.hasPendingUpdates());
// Delete edge bb0 -> bb3 and push the update twice to verify duplicate // Delete edge bb0 -> bb3 and push the update twice to verify duplicate
// entries are discarded. // entries are discarded.
@ -102,14 +102,13 @@ TEST(DomTreeUpdater, EagerUpdateBasicOperations) {
// queued for deletion. // queued for deletion.
ASSERT_FALSE(isa<UnreachableInst>(BB3->getTerminator())); ASSERT_FALSE(isa<UnreachableInst>(BB3->getTerminator()));
EXPECT_FALSE(DTU.isBBPendingDeletion(BB3)); EXPECT_FALSE(DTU.isBBPendingDeletion(BB3));
DTU.applyUpdates(Updates, /*ForceRemoveDuplicates*/ true); DTU.applyUpdatesPermissive(Updates);
ASSERT_FALSE(DTU.hasPendingUpdates()); ASSERT_FALSE(DTU.hasPendingUpdates());
// Invalid Insert: no edge bb1 -> bb2 after change to bb0. // Invalid Insert: no edge bb1 -> bb2 after change to bb0.
// Invalid Delete: edge exists bb0 -> bb1 after change to bb0. // Invalid Delete: edge exists bb0 -> bb1 after change to bb0.
DTU.applyUpdates( DTU.applyUpdatesPermissive(
{{DominatorTree::Insert, BB1, BB2}, {DominatorTree::Delete, BB0, BB1}}, {{DominatorTree::Insert, BB1, BB2}, {DominatorTree::Delete, BB0, BB1}});
/*ForceRemoveDuplicates*/ true);
// DTU working with Eager UpdateStrategy does not need to flush. // DTU working with Eager UpdateStrategy does not need to flush.
ASSERT_TRUE(DT.verify()); ASSERT_TRUE(DT.verify());
@ -184,8 +183,7 @@ TEST(DomTreeUpdater, EagerUpdateReplaceEntryBB) {
EXPECT_EQ(F->begin()->getName(), NewEntry->getName()); EXPECT_EQ(F->begin()->getName(), NewEntry->getName());
EXPECT_TRUE(&F->getEntryBlock() == NewEntry); EXPECT_TRUE(&F->getEntryBlock() == NewEntry);
DTU.applyUpdates({{DominatorTree::Insert, NewEntry, BB0}}, DTU.applyUpdates({{DominatorTree::Insert, NewEntry, BB0}});
/*ForceRemoveDuplicates*/ true);
// Changing the Entry BB requires a full recalculation of DomTree. // Changing the Entry BB requires a full recalculation of DomTree.
DTU.recalculate(*F); DTU.recalculate(*F);
@ -254,7 +252,7 @@ TEST(DomTreeUpdater, LazyUpdateDTBasicOperations) {
BasicBlock *BB3 = &*FI++; BasicBlock *BB3 = &*FI++;
// Test discards of self-domination update. // Test discards of self-domination update.
DTU.applyUpdates({{DominatorTree::Delete, BB0, BB0}}); DTU.applyUpdatesPermissive({{DominatorTree::Insert, BB0, BB0}});
ASSERT_FALSE(DTU.hasPendingDomTreeUpdates()); ASSERT_FALSE(DTU.hasPendingDomTreeUpdates());
// Delete edge bb0 -> bb3 and push the update twice to verify duplicate // Delete edge bb0 -> bb3 and push the update twice to verify duplicate
@ -277,7 +275,7 @@ TEST(DomTreeUpdater, LazyUpdateDTBasicOperations) {
// Verify. Updates to DTU must be applied *after* all changes to the CFG // Verify. Updates to DTU must be applied *after* all changes to the CFG
// (including block deletion). // (including block deletion).
DTU.applyUpdates(Updates); DTU.applyUpdatesPermissive(Updates);
ASSERT_TRUE(DTU.getDomTree().verify()); ASSERT_TRUE(DTU.getDomTree().verify());
// Deletion of a BasicBlock is an immediate event. We remove all uses to the // Deletion of a BasicBlock is an immediate event. We remove all uses to the
@ -362,9 +360,9 @@ TEST(DomTreeUpdater, LazyUpdateDTInheritedPreds) {
// //
// While the final CFG form is functionally identical the updates to // While the final CFG form is functionally identical the updates to
// DTU are not. In the first case we must have // DTU are not. In the first case we must have
// DTU.applyUpdates({{DominatorTree::Insert, Pred1, Succ}}) while in the // DTU.applyUpdates({{DominatorTree::Insert, Pred1, Succ}}) while in
// latter case we must *NOT* have DTU.applyUpdates({{DominatorTree::Insert, // the latter case we must *NOT* have
// Pred1, Succ}}). // DTU.applyUpdates({{DominatorTree::Insert, Pred1, Succ}}).
// CFG Change: bb0 now only has bb0 -> bb1 and bb0 -> bb3. We are preparing to // CFG Change: bb0 now only has bb0 -> bb1 and bb0 -> bb3. We are preparing to
// remove bb2. // remove bb2.
@ -384,9 +382,9 @@ TEST(DomTreeUpdater, LazyUpdateDTInheritedPreds) {
BasicBlocks.end()); BasicBlocks.end());
}; };
ASSERT_EQ(BasicBlocks.size(), static_cast<size_t>(2)); ASSERT_EQ(BasicBlocks.size(), static_cast<size_t>(2));
// Remove bb2 from F. This has to happen before the call to applyUpdates() for // Remove bb2 from F. This has to happen before the call to
// DTU to detect there is no longer an edge between bb2 -> bb3. The deleteBB() // applyUpdates() for DTU to detect there is no longer an edge between
// method converts bb2's TI into "unreachable". // bb2 -> bb3. The deleteBB() method converts bb2's TI into "unreachable".
ASSERT_FALSE(isa<UnreachableInst>(BB2->getTerminator())); ASSERT_FALSE(isa<UnreachableInst>(BB2->getTerminator()));
EXPECT_FALSE(DTU.isBBPendingDeletion(BB2)); EXPECT_FALSE(DTU.isBBPendingDeletion(BB2));
DTU.callbackDeleteBB(BB2, Eraser); DTU.callbackDeleteBB(BB2, Eraser);
@ -407,9 +405,9 @@ TEST(DomTreeUpdater, LazyUpdateDTInheritedPreds) {
BranchInst::Create(BB3, BB0); BranchInst::Create(BB3, BB0);
EXPECT_EQ(BB0->getTerminator()->getNumSuccessors(), 1u); EXPECT_EQ(BB0->getTerminator()->getNumSuccessors(), 1u);
// Remove bb1 from F. This has to happen before the call to applyUpdates() for // Remove bb1 from F. This has to happen before the call to
// DTU to detect there is no longer an edge between bb1 -> bb3. The deleteBB() // applyUpdates() for DTU to detect there is no longer an edge between
// method converts bb1's TI into "unreachable". // bb1 -> bb3. The deleteBB() method converts bb1's TI into "unreachable".
ASSERT_FALSE(isa<UnreachableInst>(BB1->getTerminator())); ASSERT_FALSE(isa<UnreachableInst>(BB1->getTerminator()));
EXPECT_FALSE(DTU.isBBPendingDeletion(BB1)); EXPECT_FALSE(DTU.isBBPendingDeletion(BB1));
DTU.callbackDeleteBB(BB1, Eraser); DTU.callbackDeleteBB(BB1, Eraser);
@ -424,7 +422,7 @@ TEST(DomTreeUpdater, LazyUpdateDTInheritedPreds) {
Updates.push_back({DominatorTree::Delete, BB1, BB3}); Updates.push_back({DominatorTree::Delete, BB1, BB3});
// Verify everything. // Verify everything.
DTU.applyUpdates(Updates); DTU.applyUpdatesPermissive(Updates);
ASSERT_EQ(BasicBlocks.size(), static_cast<size_t>(2)); ASSERT_EQ(BasicBlocks.size(), static_cast<size_t>(2));
DTU.flush(); DTU.flush();
ASSERT_EQ(BasicBlocks.size(), static_cast<size_t>(0)); ASSERT_EQ(BasicBlocks.size(), static_cast<size_t>(0));
@ -509,7 +507,7 @@ TEST(DomTreeUpdater, LazyUpdateBasicOperations) {
// Verify. Updates to DTU must be applied *after* all changes to the CFG // Verify. Updates to DTU must be applied *after* all changes to the CFG
// (including block deletion). // (including block deletion).
DTU.applyUpdates(Updates); DTU.applyUpdatesPermissive(Updates);
ASSERT_TRUE(DTU.getDomTree().verify()); ASSERT_TRUE(DTU.getDomTree().verify());
ASSERT_TRUE(DTU.hasPendingUpdates()); ASSERT_TRUE(DTU.hasPendingUpdates());
ASSERT_TRUE(DTU.hasPendingPostDomTreeUpdates()); ASSERT_TRUE(DTU.hasPendingPostDomTreeUpdates());
@ -648,8 +646,7 @@ TEST(DomTreeUpdater, LazyUpdateStepTest) {
SwitchInst *SI = dyn_cast<SwitchInst>(BB0->getTerminator()); SwitchInst *SI = dyn_cast<SwitchInst>(BB0->getTerminator());
ASSERT_NE(SI, nullptr) << "Couldn't get SwitchInst."; ASSERT_NE(SI, nullptr) << "Couldn't get SwitchInst.";
// Delete edge bb0 -> bb3 and push the update twice to verify duplicate // Delete edge bb0 -> bb3.
// entries are discarded.
std::vector<DominatorTree::UpdateType> Updates; std::vector<DominatorTree::UpdateType> Updates;
Updates.reserve(1); Updates.reserve(1);
Updates.push_back({DominatorTree::Delete, BB0, BB3}); Updates.push_back({DominatorTree::Delete, BB0, BB3});
@ -694,7 +691,7 @@ TEST(DomTreeUpdater, LazyUpdateStepTest) {
++i; ++i;
} }
DTU.applyUpdates(Updates); DTU.applyUpdatesPermissive(Updates);
// flush PostDomTree // flush PostDomTree
ASSERT_TRUE(DTU.getPostDomTree().verify()); ASSERT_TRUE(DTU.getPostDomTree().verify());
ASSERT_FALSE(DTU.hasPendingPostDomTreeUpdates()); ASSERT_FALSE(DTU.hasPendingPostDomTreeUpdates());
@ -729,3 +726,69 @@ TEST(DomTreeUpdater, NoTreeTest) {
DTU.recalculate(*F); DTU.recalculate(*F);
ASSERT_FALSE(DTU.hasPendingDeletedBB()); ASSERT_FALSE(DTU.hasPendingDeletedBB());
} }
TEST(DomTreeUpdater, LazyUpdateDeduplicationTest) {
StringRef FuncName = "f";
StringRef ModuleString = R"(
define i32 @f() {
bb0:
br label %bb1
bb1:
ret i32 1
bb2:
ret i32 1
}
)";
// Make the module.
LLVMContext Context;
std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);
Function *F = M->getFunction(FuncName);
// Make the DTU.
DominatorTree DT(*F);
DomTreeUpdater DTU(&DT, nullptr, DomTreeUpdater::UpdateStrategy::Lazy);
ASSERT_TRUE(DTU.getDomTree().verify());
Function::iterator FI = F->begin();
BasicBlock *BB0 = &*FI++;
BasicBlock *BB1 = &*FI++;
BasicBlock *BB2 = &*FI++;
// CFG Change: remove bb0 -> bb1 and add back bb0 -> bb1.
EXPECT_EQ(BB0->getTerminator()->getNumSuccessors(), 1u);
BB0->getTerminator()->eraseFromParent();
BranchInst::Create(BB1, BB0);
EXPECT_EQ(BB0->getTerminator()->getNumSuccessors(), 1u);
// Update the DTU and simulate duplicates.
DTU.applyUpdatesPermissive({{DominatorTree::Delete, BB0, BB1},
{DominatorTree::Delete, BB0, BB1},
{DominatorTree::Insert, BB0, BB1},
{DominatorTree::Insert, BB0, BB1},
{DominatorTree::Insert, BB0, BB1}});
// The above operations result in a no-op.
ASSERT_FALSE(DTU.hasPendingUpdates());
// Update the DTU. Simulate an invalid update.
DTU.applyUpdatesPermissive({{DominatorTree::Delete, BB0, BB1}});
ASSERT_FALSE(DTU.hasPendingUpdates());
// CFG Change: remove bb0 -> bb1.
EXPECT_EQ(BB0->getTerminator()->getNumSuccessors(), 1u);
BB0->getTerminator()->eraseFromParent();
// Update the DTU and simulate invalid updates.
DTU.applyUpdatesPermissive({{DominatorTree::Delete, BB0, BB1},
{DominatorTree::Insert, BB0, BB1},
{DominatorTree::Delete, BB0, BB1},
{DominatorTree::Insert, BB0, BB1},
{DominatorTree::Insert, BB0, BB1}});
ASSERT_TRUE(DTU.hasPendingUpdates());
// CFG Change: add bb0 -> bb2.
BranchInst::Create(BB2, BB0);
EXPECT_EQ(BB0->getTerminator()->getNumSuccessors(), 1u);
DTU.applyUpdates({{DominatorTree::Insert, BB0, BB2}});
ASSERT_TRUE(DTU.getDomTree().verify());
}