[MemorySSA] Make the visitor more careful with calls.
Prior to this patch, the MemorySSA caching visitor would cache all
calls that it visited. When paired with phi optimization, this can be
problematic. Consider:
define void @foo() {
; 1 = MemoryDef(liveOnEntry)
call void @clobberFunction()
br i1 undef, label %if.end, label %if.then
if.then:
; MemoryUse(??)
call void @readOnlyFunction()
; 2 = MemoryDef(1)
call void @clobberFunction()
br label %if.end
if.end:
; 3 = MemoryPhi(...)
; MemoryUse(?)
call void @readOnlyFunction()
ret void
}
When optimizing MemoryUse(?), we visit defs 1 and 2, so we note to
cache them later. We ultimately end up not being able to optimize
passed the Phi, so we set MemoryUse(?) to point to the Phi. We then
cache the clobbering call for def 1 to be the Phi.
This commit changes this behavior so that we wipe out any calls
added to VisistedCalls while visiting the defs of a phi we couldn't
optimize.
Aside: With this patch, we now can bootstrap clang/LLVM without a
single MemorySSA verifier failure. Woohoo. :)
llvm-svn: 264820
diff --git a/llvm/lib/Transforms/Utils/MemorySSA.cpp b/llvm/lib/Transforms/Utils/MemorySSA.cpp
index b71c194..1ce8970 100644
--- a/llvm/lib/Transforms/Utils/MemorySSA.cpp
+++ b/llvm/lib/Transforms/Utils/MemorySSA.cpp
@@ -763,11 +763,11 @@
const Instruction *Inst;
// Set of visited Instructions for this query.
DenseSet<MemoryAccessPair> Visited;
- // Set of visited call accesses for this query. This is separated out because
- // you can always cache and lookup the result of call queries (IE when IsCall
- // == true) for every call in the chain. The calls have no AA location
+ // Vector of visited call accesses for this query. This is separated out
+ // because you can always cache and lookup the result of call queries (IE when
+ // IsCall == true) for every call in the chain. The calls have no AA location
// associated with them with them, and thus, no context dependence.
- SmallPtrSet<const MemoryAccess *, 32> VisitedCalls;
+ SmallVector<const MemoryAccess *, 32> VisitedCalls;
// The MemoryAccess we actually got called with, used to test local domination
const MemoryAccess *OriginalAccess;
// The Datalayout for the module we started in
@@ -862,7 +862,7 @@
// If this is a call, mark it for caching
if (ImmutableCallSite(DefMemoryInst))
- Q.VisitedCalls.insert(MD);
+ Q.VisitedCalls.push_back(MD);
ModRefInfo I = AA->getModRefInfo(DefMemoryInst, ImmutableCallSite(Q.Inst));
return I != MRI_NoModRef;
}
@@ -918,6 +918,8 @@
break;
}
+ std::size_t InitialVisitedCallSize = Q.VisitedCalls.size();
+
// Recurse on PHI nodes, since we need to change locations.
// TODO: Allow graphtraits on pairs, which would turn this whole function
// into a normal single depth first walk.
@@ -947,6 +949,10 @@
if (!ModifyingAccess) {
assert(FirstDef && "Found a Phi with no upward defs?");
ModifyingAccess = FirstDef;
+ } else {
+ // If we can't optimize this Phi, then we can't safely cache any of the
+ // calls we visited when trying to optimize it. Wipe them out now.
+ Q.VisitedCalls.resize(InitialVisitedCallSize);
}
break;
}