[llvm] 022da61 - [SimplifyCFG] Change 'LoopHeaders' to be ArrayRef<WeakVH>, not a naked set, thus avoiding dangling pointers
Roman Lebedev via llvm-commits
llvm-commits at lists.llvm.org
Sat Jan 23 05:49:03 PST 2021
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);
More information about the llvm-commits
mailing list