Author: Roman Lebedev Date: 2021-01-23T16:48:35+03:00 New Revision: 022da61f6b30626708e5b4c1c009afb453d12ebe
URL: https://github.com/llvm/llvm-project/commit/022da61f6b30626708e5b4c1c009afb453d12ebe DIFF: https://github.com/llvm/llvm-project/commit/022da61f6b30626708e5b4c1c009afb453d12ebe.diff LOG: [SimplifyCFG] Change 'LoopHeaders' to be ArrayRef<WeakVH>, not a naked set, thus avoiding dangling pointers If i change it to AssertingVH instead, a number of existing tests fail, which means we don't consistently remove from the set when deleting blocks, which means newly-created blocks may happen to appear in that set if they happen to occupy the same memory chunk as did some block that was in the set originally. There are many places where we delete blocks, and while we could probably consistently delete from LoopHeaders when deleting a block in transforms located in SimplifyCFG.cpp itself, transforms located elsewhere (Local.cpp/BasicBlockUtils.cpp) also may delete blocks, and it doesn't seem good to teach them to deal with it. Since we at most only ever delete from LoopHeaders, let's just delegate to WeakVH to do that automatically. But to be honest, personally, i'm not sure that the idea behind LoopHeaders is sound. Added: Modified: llvm/include/llvm/Transforms/Utils/Local.h llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp llvm/lib/Transforms/Utils/SimplifyCFG.cpp Removed: ################################################################################ diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h index ebfb62a48bbf..c712dda483e4 100644 --- a/llvm/include/llvm/Transforms/Utils/Local.h +++ b/llvm/include/llvm/Transforms/Utils/Local.h @@ -16,7 +16,6 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/STLExtras.h" -#include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/TinyPtrVector.h" #include "llvm/Analysis/Utils/Local.h" @@ -177,7 +176,7 @@ extern cl::opt<bool> RequireAndPreserveDomTree; bool simplifyCFG(BasicBlock *BB, const TargetTransformInfo &TTI, DomTreeUpdater *DTU = nullptr, const SimplifyCFGOptions &Options = {}, - SmallPtrSetImpl<BasicBlock *> *LoopHeaders = nullptr); + ArrayRef<WeakVH> LoopHeaders = {}); /// This function is used to flatten a CFG. For example, it uses parallel-and /// and parallel-or mode to collapse if-conditions and merge if-regions with diff --git a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp index 0e1ec7194527..38e7109ead57 100644 --- a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp +++ b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp @@ -36,6 +36,7 @@ #include "llvm/IR/Instructions.h" #include "llvm/IR/IntrinsicInst.h" #include "llvm/IR/Module.h" +#include "llvm/IR/ValueHandle.h" #include "llvm/InitializePasses.h" #include "llvm/Pass.h" #include "llvm/Support/CommandLine.h" @@ -200,9 +201,12 @@ static bool iterativelySimplifyCFG(Function &F, const TargetTransformInfo &TTI, SmallVector<std::pair<const BasicBlock *, const BasicBlock *>, 32> Edges; FindFunctionBackedges(F, Edges); - SmallPtrSet<BasicBlock *, 16> LoopHeaders; + SmallPtrSet<BasicBlock *, 16> UniqueLoopHeaders; for (unsigned i = 0, e = Edges.size(); i != e; ++i) - LoopHeaders.insert(const_cast<BasicBlock *>(Edges[i].second)); + UniqueLoopHeaders.insert(const_cast<BasicBlock *>(Edges[i].second)); + + SmallVector<WeakVH, 16> LoopHeaders(UniqueLoopHeaders.begin(), + UniqueLoopHeaders.end()); while (LocalChange) { LocalChange = false; @@ -219,7 +223,7 @@ static bool iterativelySimplifyCFG(Function &F, const TargetTransformInfo &TTI, while (BBIt != F.end() && DTU->isBBPendingDeletion(&*BBIt)) ++BBIt; } - if (simplifyCFG(&BB, TTI, DTU, Options, &LoopHeaders)) { + if (simplifyCFG(&BB, TTI, DTU, Options, LoopHeaders)) { LocalChange = true; ++NumSimpl; } diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp index bad8d90cda65..deaa0bbcf772 100644 --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp @@ -61,6 +61,7 @@ #include "llvm/IR/Use.h" #include "llvm/IR/User.h" #include "llvm/IR/Value.h" +#include "llvm/IR/ValueHandle.h" #include "llvm/Support/Casting.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/Debug.h" @@ -218,7 +219,7 @@ class SimplifyCFGOpt { const TargetTransformInfo &TTI; DomTreeUpdater *DTU; const DataLayout &DL; - SmallPtrSetImpl<BasicBlock *> *LoopHeaders; + ArrayRef<WeakVH> LoopHeaders; const SimplifyCFGOptions &Options; bool Resimplify; @@ -261,8 +262,7 @@ class SimplifyCFGOpt { public: SimplifyCFGOpt(const TargetTransformInfo &TTI, DomTreeUpdater *DTU, - const DataLayout &DL, - SmallPtrSetImpl<BasicBlock *> *LoopHeaders, + const DataLayout &DL, ArrayRef<WeakVH> LoopHeaders, const SimplifyCFGOptions &Opts) : TTI(TTI), DTU(DTU), DL(DL), LoopHeaders(LoopHeaders), Options(Opts) { assert((!DTU || !DTU->hasPostDomTree()) && @@ -4192,8 +4192,6 @@ bool SimplifyCFGOpt::simplifySingleResume(ResumeInst *RI) { } // The landingpad is now unreachable. Zap it. - if (LoopHeaders) - LoopHeaders->erase(BB); if (DTU) DTU->deleteBB(BB); else @@ -4417,8 +4415,6 @@ bool SimplifyCFGOpt::simplifyReturn(ReturnInst *RI, IRBuilder<> &Builder) { // If we eliminated all predecessors of the block, delete the block now. if (pred_empty(BB)) { // We know there are no successors, so just nuke the block. - if (LoopHeaders) - LoopHeaders->erase(BB); if (DTU) DTU->deleteBB(BB); else @@ -4620,8 +4616,6 @@ bool SimplifyCFGOpt::simplifyUnreachable(UnreachableInst *UI) { // If this block is now dead, remove it. if (pred_empty(BB) && BB != &BB->getParent()->getEntryBlock()) { // We know there are no successors, so just nuke the block. - if (LoopHeaders) - LoopHeaders->erase(BB); if (DTU) DTU->deleteBB(BB); else @@ -6214,8 +6208,8 @@ bool SimplifyCFGOpt::simplifyUncondBranch(BranchInst *BI, // backedge, so we can eliminate BB. bool NeedCanonicalLoop = Options.NeedCanonicalLoop && - (LoopHeaders && BB->hasNPredecessorsOrMore(2) && - (LoopHeaders->count(BB) || LoopHeaders->count(Succ))); + (!LoopHeaders.empty() && BB->hasNPredecessorsOrMore(2) && + (is_contained(LoopHeaders, BB) || is_contained(LoopHeaders, Succ))); BasicBlock::iterator I = BB->getFirstNonPHIOrDbg()->getIterator(); if (I->isTerminator() && BB != &BB->getParent()->getEntryBlock() && !NeedCanonicalLoop && TryToSimplifyUncondBranchFromEmptyBlock(BB, DTU)) @@ -6583,7 +6577,7 @@ bool SimplifyCFGOpt::run(BasicBlock *BB) { bool llvm::simplifyCFG(BasicBlock *BB, const TargetTransformInfo &TTI, DomTreeUpdater *DTU, const SimplifyCFGOptions &Options, - SmallPtrSetImpl<BasicBlock *> *LoopHeaders) { + ArrayRef<WeakVH> LoopHeaders) { return SimplifyCFGOpt(TTI, RequireAndPreserveDomTree ? DTU : nullptr, BB->getModule()->getDataLayout(), LoopHeaders, Options) .run(BB); _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits