<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jul 21, 2016, at 4:04 PM, George Burgess IV <<a href="mailto:george.burgess.iv@gmail.com" class="">george.burgess.iv@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Thanks for bringing this up! And yeah, the first thing that `def_chain` should iterate over is `Target` itself. So, as long as `Target != nullptr` (which should always be true), we should be fine here.<br class=""><div class=""><br class=""></div><div class="">Is there something we could add to make coverity happy?</div></div></div></blockquote><div><br class=""></div><div>I don’t know if we can add something, but I’m not even sure we “should”. </div><div>It is reporting issue on new code every week or so, which is enough to review newly added code, and our existing codebase have thousands of such “errors” in Coverity.</div><div><br class=""></div><div>— </div><div>Mehdi</div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Thu, Jul 21, 2016 at 3:50 PM, Mehdi Amini <span dir="ltr" class=""><<a href="mailto:mehdi.amini@apple.com" target="_blank" class="">mehdi.amini@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class=""><div class=""><br class="">
> On Jul 18, 2016, at 6:29 PM, George Burgess IV via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class="">
><br class="">
> Author: gbiv<br class="">
> Date: Mon Jul 18 20:29:15 2016<br class="">
> New Revision: 275940<br class="">
><br class="">
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=275940&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=275940&view=rev</a><br class="">
> Log:<br class="">
> [MemorySSA] Update to the new shiny walker.<br class="">
><br class="">
> This patch updates MemorySSA's use-optimizing walker to be more<br class="">
> accurate and, in some cases, faster.<br class="">
><br class="">
> Essentially, this changed our core walking algorithm from a<br class="">
> cache-as-you-go DFS to an iteratively expanded DFS, with all of the<br class="">
> caching happening at the end. Said expansion happens when we hit a Phi,<br class="">
> P; we'll try to do the smallest amount of work possible to see if<br class="">
> optimizing above that Phi is legal in the first place. If so, we'll<br class="">
> expand the search to see if we can optimize to the next phi, etc.<br class="">
><br class="">
> An iteratively expanded DFS lets us potentially quit earlier (because we<br class="">
> don't assume that we can optimize above all phis) than our old walker.<br class="">
> Additionally, because we don't cache as we go, we can now optimize above<br class="">
> loops.<br class="">
><br class="">
> As an added bonus, this patch adds a ton of verification (if<br class="">
> EXPENSIVE_CHECKS are enabled), so finding bugs is easier.<br class="">
><br class="">
> Differential Revision: <a href="https://reviews.llvm.org/D21777" rel="noreferrer" target="_blank" class="">https://reviews.llvm.org/D21777</a><br class="">
><br class="">
> Modified:<br class="">
> llvm/trunk/include/llvm/Transforms/Utils/MemorySSA.h<br class="">
> llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp<br class="">
> llvm/trunk/test/Transforms/Util/MemorySSA/cyclicphi.ll<br class="">
> llvm/trunk/test/Transforms/Util/MemorySSA/phi-translation.ll<br class="">
><br class="">
> Modified: llvm/trunk/include/llvm/Transforms/Utils/MemorySSA.h<br class="">
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/MemorySSA.h?rev=275940&r1=275939&r2=275940&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/MemorySSA.h?rev=275940&r1=275939&r2=275940&view=diff</a><br class="">
> ==============================================================================<br class="">
> --- llvm/trunk/include/llvm/Transforms/Utils/MemorySSA.h (original)<br class="">
> +++ llvm/trunk/include/llvm/Transforms/Utils/MemorySSA.h Mon Jul 18 20:29:15 2016<br class="">
> @@ -580,6 +580,10 @@ public:<br class="">
> /// whether MemoryAccess \p A dominates MemoryAccess \p B.<br class="">
> bool locallyDominates(const MemoryAccess *A, const MemoryAccess *B) const;<br class="">
><br class="">
> + /// \brief Given two memory accesses in potentially different blocks,<br class="">
> + /// determine whether MemoryAccess \p A dominates MemoryAccess \p B.<br class="">
> + bool dominates(const MemoryAccess *A, const MemoryAccess *B) const;<br class="">
> +<br class="">
> /// \brief Verify that MemorySSA is self consistent (IE definitions dominate<br class="">
> /// all uses, uses appear in the right places). This is used by unit tests.<br class="">
> void verifyMemorySSA() const;<br class="">
> @@ -594,6 +598,8 @@ protected:<br class="">
><br class="">
> private:<br class="">
> class CachingWalker;<br class="">
> +<br class="">
> + CachingWalker *getWalkerImpl();<br class="">
> void buildMemorySSA();<br class="">
> void verifyUseInDefs(MemoryAccess *, MemoryAccess *) const;<br class="">
> using AccessMap = DenseMap<const BasicBlock *, std::unique_ptr<AccessList>>;<br class="">
><br class="">
> Modified: llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp<br class="">
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp?rev=275940&r1=275939&r2=275940&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp?rev=275940&r1=275939&r2=275940&view=diff</a><br class="">
> ==============================================================================<br class="">
> --- llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp (original)<br class="">
> +++ llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp Mon Jul 18 20:29:15 2016<br class="">
> @@ -17,6 +17,7 @@<br class="">
> #include "llvm/ADT/GraphTraits.h"<br class="">
> #include "llvm/ADT/PostOrderIterator.h"<br class="">
> #include "llvm/ADT/STLExtras.h"<br class="">
> +#include "llvm/ADT/SmallBitVector.h"<br class="">
> #include "llvm/ADT/SmallPtrSet.h"<br class="">
> #include "llvm/ADT/SmallSet.h"<br class="">
> #include "llvm/ADT/Statistic.h"<br class="">
> @@ -86,7 +87,777 @@ public:<br class="">
> OS << "; " << *MA << "\n";<br class="">
> }<br class="">
> };<br class="">
> +}<br class="">
> +<br class="">
> +namespace {<br class="">
> +struct UpwardsMemoryQuery {<br class="">
> + // True if our original query started off as a call<br class="">
> + bool IsCall;<br class="">
> + // The pointer location we started the query with. This will be empty if<br class="">
> + // IsCall is true.<br class="">
> + MemoryLocation StartingLoc;<br class="">
> + // This is the instruction we were querying about.<br class="">
> + const Instruction *Inst;<br class="">
> + // The MemoryAccess we actually got called with, used to test local domination<br class="">
> + const MemoryAccess *OriginalAccess;<br class="">
> +<br class="">
> + UpwardsMemoryQuery()<br class="">
> + : IsCall(false), Inst(nullptr), OriginalAccess(nullptr) {}<br class="">
> +<br class="">
> + UpwardsMemoryQuery(const Instruction *Inst, const MemoryAccess *Access)<br class="">
> + : IsCall(ImmutableCallSite(Inst)), Inst(Inst), OriginalAccess(Access) {<br class="">
> + if (!IsCall)<br class="">
> + StartingLoc = MemoryLocation::get(Inst);<br class="">
> + }<br class="">
> +};<br class="">
> +<br class="">
> +static bool instructionClobbersQuery(MemoryDef *MD, const MemoryLocation &Loc,<br class="">
> + const UpwardsMemoryQuery &Query,<br class="">
> + AliasAnalysis &AA) {<br class="">
> + Instruction *DefMemoryInst = MD->getMemoryInst();<br class="">
> + assert(DefMemoryInst && "Defining instruction not actually an instruction");<br class="">
> +<br class="">
> + if (!Query.IsCall)<br class="">
> + return AA.getModRefInfo(DefMemoryInst, Loc) & MRI_Mod;<br class="">
> +<br class="">
> + ModRefInfo I = AA.getModRefInfo(DefMemoryInst, ImmutableCallSite(Query.Inst));<br class="">
> + return I != MRI_NoModRef;<br class="">
> +}<br class="">
> +<br class="">
> +/// Cache for our caching MemorySSA walker.<br class="">
> +class WalkerCache {<br class="">
> + DenseMap<ConstMemoryAccessPair, MemoryAccess *> Accesses;<br class="">
> + DenseMap<const MemoryAccess *, MemoryAccess *> Calls;<br class="">
> +<br class="">
> +public:<br class="">
> + MemoryAccess *lookup(const MemoryAccess *MA, const MemoryLocation &Loc,<br class="">
> + bool IsCall) const {<br class="">
> + ++NumClobberCacheLookups;<br class="">
> + MemoryAccess *R = IsCall ? Calls.lookup(MA) : Accesses.lookup({MA, Loc});<br class="">
> + if (R)<br class="">
> + ++NumClobberCacheHits;<br class="">
> + return R;<br class="">
> + }<br class="">
> +<br class="">
> + bool insert(const MemoryAccess *MA, MemoryAccess *To,<br class="">
> + const MemoryLocation &Loc, bool IsCall) {<br class="">
> + // This is fine for Phis, since there are times where we can't optimize<br class="">
> + // them. Making a def its own clobber is never correct, though.<br class="">
> + assert((MA != To || isa<MemoryPhi>(MA)) &&<br class="">
> + "Something can't clobber itself!");<br class="">
> +<br class="">
> + ++NumClobberCacheInserts;<br class="">
> + bool Inserted;<br class="">
> + if (IsCall)<br class="">
> + Inserted = Calls.insert({MA, To}).second;<br class="">
> + else<br class="">
> + Inserted = Accesses.insert({{MA, Loc}, To}).second;<br class="">
> +<br class="">
> + return Inserted;<br class="">
> + }<br class="">
> +<br class="">
> + bool remove(const MemoryAccess *MA, const MemoryLocation &Loc, bool IsCall) {<br class="">
> + return IsCall ? Calls.erase(MA) : Accesses.erase({MA, Loc});<br class="">
> + }<br class="">
> +<br class="">
> + void clear() {<br class="">
> + Accesses.clear();<br class="">
> + Calls.clear();<br class="">
> + }<br class="">
> +<br class="">
> + bool contains(const MemoryAccess *MA) const {<br class="">
> + for (auto &P : Accesses)<br class="">
> + if (P.first.first == MA || P.second == MA)<br class="">
> + return true;<br class="">
> + for (auto &P : Calls)<br class="">
> + if (P.first == MA || P.second == MA)<br class="">
> + return true;<br class="">
> + return false;<br class="">
> + }<br class="">
> +};<br class="">
> +<br class="">
> +/// Walks the defining uses of MemoryDefs. Stops after we hit something that has<br class="">
> +/// no defining use (e.g. a MemoryPhi or liveOnEntry). Note that, when comparing<br class="">
> +/// against a null def_chain_iterator, this will compare equal only after<br class="">
> +/// walking said Phi/liveOnEntry.<br class="">
> +struct def_chain_iterator<br class="">
> + : public iterator_facade_base<def_chain_iterator, std::forward_iterator_tag,<br class="">
> + MemoryAccess *> {<br class="">
> + def_chain_iterator() : MA(nullptr) {}<br class="">
> + def_chain_iterator(MemoryAccess *MA) : MA(MA) {}<br class="">
> +<br class="">
> + MemoryAccess *operator*() const { return MA; }<br class="">
> +<br class="">
> + def_chain_iterator &operator++() {<br class="">
> + // N.B. liveOnEntry has a null defining access.<br class="">
> + if (auto *MUD = dyn_cast<MemoryUseOrDef>(MA))<br class="">
> + MA = MUD->getDefiningAccess();<br class="">
> + else<br class="">
> + MA = nullptr;<br class="">
> + return *this;<br class="">
> + }<br class="">
> +<br class="">
> + bool operator==(const def_chain_iterator &O) const { return MA == <a href="http://o.ma/" rel="noreferrer" target="_blank" class="">O.MA</a>; }<br class="">
> +<br class="">
> +private:<br class="">
> + MemoryAccess *MA;<br class="">
> +};<br class="">
> +<br class="">
> +static iterator_range<def_chain_iterator><br class="">
> +def_chain(MemoryAccess *MA, MemoryAccess *UpTo = nullptr) {<br class="">
> +#ifdef EXPENSIVE_CHECKS<br class="">
> + assert((!UpTo || find(def_chain(MA), UpTo) != def_chain_iterator()) &&<br class="">
> + "UpTo isn't in the def chain!");<br class="">
> +#endif<br class="">
> + return make_range(def_chain_iterator(MA), def_chain_iterator(UpTo));<br class="">
> +}<br class="">
> +<br class="">
> +/// Verifies that `Start` is clobbered by `ClobberAt`, and that nothing<br class="">
> +/// inbetween `Start` and `ClobberAt` can clobbers `Start`.<br class="">
> +///<br class="">
> +/// This is meant to be as simple and self-contained as possible. Because it<br class="">
> +/// uses no cache, etc., it can be relatively expensive.<br class="">
> +///<br class="">
> +/// \param Start The MemoryAccess that we want to walk from.<br class="">
> +/// \param ClobberAt A clobber for Start.<br class="">
> +/// \param StartLoc The MemoryLocation for Start.<br class="">
> +/// \param MSSA The MemorySSA isntance that Start and ClobberAt belong to.<br class="">
> +/// \param Query The UpwardsMemoryQuery we used for our search.<br class="">
> +/// \param AA The AliasAnalysis we used for our search.<br class="">
> +static void LLVM_ATTRIBUTE_UNUSED<br class="">
> +checkClobberSanity(MemoryAccess *Start, MemoryAccess *ClobberAt,<br class="">
> + const MemoryLocation &StartLoc, const MemorySSA &MSSA,<br class="">
> + const UpwardsMemoryQuery &Query, AliasAnalysis &AA) {<br class="">
> + assert(MSSA.dominates(ClobberAt, Start) && "Clobber doesn't dominate start?");<br class="">
> +<br class="">
> + if (MSSA.isLiveOnEntryDef(Start)) {<br class="">
> + assert(MSSA.isLiveOnEntryDef(ClobberAt) &&<br class="">
> + "liveOnEntry must clobber itself");<br class="">
> + return;<br class="">
> + }<br class="">
> +<br class="">
> + assert((isa<MemoryPhi>(Start) || Start != ClobberAt) &&<br class="">
> + "Start can't clobber itself!");<br class="">
> +<br class="">
> + bool FoundClobber = false;<br class="">
> + DenseSet<MemoryAccessPair> VisitedPhis;<br class="">
> + SmallVector<MemoryAccessPair, 8> Worklist;<br class="">
> + Worklist.emplace_back(Start, StartLoc);<br class="">
> + // Walk all paths from Start to ClobberAt, while looking for clobbers. If one<br class="">
> + // is found, complain.<br class="">
> + while (!Worklist.empty()) {<br class="">
> + MemoryAccessPair MAP = Worklist.pop_back_val();<br class="">
> + // All we care about is that nothing from Start to ClobberAt clobbers Start.<br class="">
> + // We learn nothing from revisiting nodes.<br class="">
> + if (!VisitedPhis.insert(MAP).second)<br class="">
> + continue;<br class="">
> +<br class="">
> + for (MemoryAccess *MA : def_chain(MAP.first)) {<br class="">
> + if (MA == ClobberAt) {<br class="">
> + if (auto *MD = dyn_cast<MemoryDef>(MA)) {<br class="">
> + // instructionClobbersQuery isn't essentially free, so don't use `|=`,<br class="">
> + // since it won't let us short-circuit.<br class="">
> + //<br class="">
> + // Also, note that this can't be hoisted out of the `Worklist` loop,<br class="">
> + // since MD may only act as a clobber for 1 of N MemoryLocations.<br class="">
> + FoundClobber = FoundClobber || MSSA.isLiveOnEntryDef(MD) ||<br class="">
> + instructionClobbersQuery(MD, MAP.second, Query, AA);<br class="">
> + }<br class="">
> + break;<br class="">
> + }<br class="">
> +<br class="">
> + // We should never hit liveOnEntry, unless it's the clobber.<br class="">
> + assert(!MSSA.isLiveOnEntryDef(MA) && "Hit liveOnEntry before clobber?");<br class="">
> +<br class="">
> + if (auto *MD = dyn_cast<MemoryDef>(MA)) {<br class="">
> + (void)MD;<br class="">
> + assert(!instructionClobbersQuery(MD, MAP.second, Query, AA) &&<br class="">
> + "Found clobber before reaching ClobberAt!");<br class="">
> + continue;<br class="">
> + }<br class="">
> +<br class="">
> + assert(isa<MemoryPhi>(MA));<br class="">
> + Worklist.append(upward_defs_begin({MA, MAP.second}), upward_defs_end());<br class="">
> + }<br class="">
> + }<br class="">
> +<br class="">
> + // If ClobberAt is a MemoryPhi, we can assume something above it acted as a<br class="">
> + // clobber. Otherwise, `ClobberAt` should've acted as a clobber at some point.<br class="">
> + assert((isa<MemoryPhi>(ClobberAt) || FoundClobber) &&<br class="">
> + "ClobberAt never acted as a clobber");<br class="">
> +}<br class="">
> +<br class="">
> +/// Our algorithm for walking (and trying to optimize) clobbers, all wrapped up<br class="">
> +/// in one class.<br class="">
> +class ClobberWalker {<br class="">
> + /// Save a few bytes by using unsigned instead of size_t.<br class="">
> + using ListIndex = unsigned;<br class="">
> +<br class="">
> + /// Represents a span of contiguous MemoryDefs, potentially ending in a<br class="">
> + /// MemoryPhi.<br class="">
> + struct DefPath {<br class="">
> + MemoryLocation Loc;<br class="">
> + // Note that, because we always walk in reverse, Last will always dominate<br class="">
> + // First. Also note that First and Last are inclusive.<br class="">
> + MemoryAccess *First;<br class="">
> + MemoryAccess *Last;<br class="">
> + // N.B. Blocker is currently basically unused. The goal is to use it to make<br class="">
> + // cache invalidation better, but we're not there yet.<br class="">
> + MemoryAccess *Blocker;<br class="">
> + Optional<ListIndex> Previous;<br class="">
> +<br class="">
> + DefPath(const MemoryLocation &Loc, MemoryAccess *First, MemoryAccess *Last,<br class="">
> + Optional<ListIndex> Previous)<br class="">
> + : Loc(Loc), First(First), Last(Last), Previous(Previous) {}<br class="">
> +<br class="">
> + DefPath(const MemoryLocation &Loc, MemoryAccess *Init,<br class="">
> + Optional<ListIndex> Previous)<br class="">
> + : DefPath(Loc, Init, Init, Previous) {}<br class="">
> + };<br class="">
> +<br class="">
> + const MemorySSA &MSSA;<br class="">
> + AliasAnalysis &AA;<br class="">
> + DominatorTree &DT;<br class="">
> + WalkerCache &WC;<br class="">
> + UpwardsMemoryQuery *Query;<br class="">
> + bool UseCache;<br class="">
> +<br class="">
> + // Phi optimization bookkeeping<br class="">
> + SmallVector<DefPath, 32> Paths;<br class="">
> + DenseSet<ConstMemoryAccessPair> VisitedPhis;<br class="">
> + DenseMap<const BasicBlock *, MemoryAccess *> WalkTargetCache;<br class="">
> +<br class="">
> + void setUseCache(bool Use) { UseCache = Use; }<br class="">
> + bool shouldIgnoreCache() const {<br class="">
> + // UseCache will only be false when we're debugging, or when expensive<br class="">
> + // checks are enabled. In either case, we don't care deeply about speed.<br class="">
> + return LLVM_UNLIKELY(!UseCache);<br class="">
> + }<br class="">
> +<br class="">
> + void addCacheEntry(const MemoryAccess *What, MemoryAccess *To,<br class="">
> + const MemoryLocation &Loc) const {<br class="">
> +// EXPENSIVE_CHECKS because most of these queries are redundant, and if What<br class="">
> +// and To are in the same BB, that gives us n^2 behavior.<br class="">
> +#ifdef EXPENSIVE_CHECKS<br class="">
> + assert(MSSA.dominates(To, What));<br class="">
> +#endif<br class="">
> + if (shouldIgnoreCache())<br class="">
> + return;<br class="">
> + WC.insert(What, To, Loc, Query->IsCall);<br class="">
> + }<br class="">
> +<br class="">
> + MemoryAccess *lookupCache(const MemoryAccess *MA, const MemoryLocation &Loc) {<br class="">
> + return shouldIgnoreCache() ? nullptr : WC.lookup(MA, Loc, Query->IsCall);<br class="">
> + }<br class="">
> +<br class="">
> + void cacheDefPath(const DefPath &DN, MemoryAccess *Target) const {<br class="">
> + if (shouldIgnoreCache())<br class="">
> + return;<br class="">
> +<br class="">
> + for (MemoryAccess *MA : def_chain(DN.First, DN.Last))<br class="">
> + addCacheEntry(MA, Target, DN.Loc);<br class="">
> +<br class="">
> + // DefPaths only express the path we walked. So, DN.Last could either be a<br class="">
> + // thing we want to cache, or not.<br class="">
> + if (DN.Last != Target)<br class="">
> + addCacheEntry(DN.Last, Target, DN.Loc);<br class="">
> + }<br class="">
> +<br class="">
> + /// Find the nearest def or phi that `From` can legally be optimized to.<br class="">
> + ///<br class="">
> + /// FIXME: Deduplicate this with MSSA::findDominatingDef. Ideally, MSSA should<br class="">
> + /// keep track of this information for us, and allow us O(1) lookups of this<br class="">
> + /// info.<br class="">
> + MemoryAccess *getWalkTarget(const MemoryPhi *From) {<br class="">
> + assert(!MSSA.isLiveOnEntryDef(From) && "liveOnEntry has no target.");<br class="">
> + assert(From->getNumOperands() && "Phi with no operands?");<br class="">
> +<br class="">
> + BasicBlock *BB = From->getBlock();<br class="">
> + auto At = WalkTargetCache.find(BB);<br class="">
> + if (At != WalkTargetCache.end())<br class="">
> + return At->second;<br class="">
> +<br class="">
> + SmallVector<const BasicBlock *, 8> ToCache;<br class="">
> + ToCache.push_back(BB);<br class="">
> +<br class="">
> + MemoryAccess *Result = MSSA.getLiveOnEntryDef();<br class="">
> + DomTreeNode *Node = DT.getNode(BB);<br class="">
> + while ((Node = Node->getIDom())) {<br class="">
> + auto At = WalkTargetCache.find(BB);<br class="">
> + if (At != WalkTargetCache.end()) {<br class="">
> + Result = At->second;<br class="">
> + break;<br class="">
> + }<br class="">
> +<br class="">
> + auto *Accesses = MSSA.getBlockAccesses(Node->getBlock());<br class="">
> + if (Accesses) {<br class="">
> + auto Iter = find_if(reverse(*Accesses), [](const MemoryAccess &MA) {<br class="">
> + return !isa<MemoryUse>(MA);<br class="">
> + });<br class="">
> + if (Iter != Accesses->rend()) {<br class="">
> + Result = const_cast<MemoryAccess *>(&*Iter);<br class="">
> + break;<br class="">
> + }<br class="">
> + }<br class="">
> +<br class="">
> + ToCache.push_back(Node->getBlock());<br class="">
> + }<br class="">
> +<br class="">
> + for (const BasicBlock *BB : ToCache)<br class="">
> + WalkTargetCache.insert({BB, Result});<br class="">
> + return Result;<br class="">
> + }<br class="">
> +<br class="">
> + /// Result of calling walkToPhiOrClobber.<br class="">
> + struct UpwardsWalkResult {<br class="">
> + /// The "Result" of the walk. Either a clobber, the last thing we walked, or<br class="">
> + /// both.<br class="">
> + MemoryAccess *Result;<br class="">
> + bool IsKnownClobber;<br class="">
> + bool FromCache;<br class="">
> + };<br class="">
> +<br class="">
> + /// Walk to the next Phi or Clobber in the def chain starting at Desc.Last.<br class="">
> + /// This will update Desc.Last as it walks. It will (optionally) also stop at<br class="">
> + /// StopAt.<br class="">
> + ///<br class="">
> + /// This does not test for whether StopAt is a clobber<br class="">
> + UpwardsWalkResult walkToPhiOrClobber(DefPath &Desc,<br class="">
> + MemoryAccess *StopAt = nullptr) {<br class="">
> + assert(!isa<MemoryUse>(Desc.Last) && "Uses don't exist in my world");<br class="">
> +<br class="">
> + for (MemoryAccess *Current : def_chain(Desc.Last)) {<br class="">
> + Desc.Last = Current;<br class="">
> + if (Current == StopAt)<br class="">
> + return {Current, false, false};<br class="">
> +<br class="">
> + if (auto *MD = dyn_cast<MemoryDef>(Current))<br class="">
> + if (MSSA.isLiveOnEntryDef(MD) ||<br class="">
> + instructionClobbersQuery(MD, Desc.Loc, *Query, AA))<br class="">
> + return {MD, true, false};<br class="">
> +<br class="">
> + // Cache checks must be done last, because if Current is a clobber, the<br class="">
> + // cache will contain the clobber for Current.<br class="">
> + if (MemoryAccess *MA = lookupCache(Current, Desc.Loc))<br class="">
> + return {MA, true, true};<br class="">
> + }<br class="">
> +<br class="">
> + assert(isa<MemoryPhi>(Desc.Last) &&<br class="">
> + "Ended at a non-clobber that's not a phi?");<br class="">
> + return {Desc.Last, false, false};<br class="">
> + }<br class="">
> +<br class="">
> + void addSearches(MemoryPhi *Phi, SmallVectorImpl<ListIndex> &PausedSearches,<br class="">
> + ListIndex PriorNode) {<br class="">
> + auto UpwardDefs = make_range(upward_defs_begin({Phi, Paths[PriorNode].Loc}),<br class="">
> + upward_defs_end());<br class="">
> + for (const MemoryAccessPair &P : UpwardDefs) {<br class="">
> + PausedSearches.push_back(Paths.size());<br class="">
> + Paths.emplace_back(P.second, P.first, PriorNode);<br class="">
> + }<br class="">
> + }<br class="">
> +<br class="">
> + /// Represents a search that terminated after finding a clobber. This clobber<br class="">
> + /// may or may not be present in the path of defs from LastNode..SearchStart,<br class="">
> + /// since it may have been retrieved from cache.<br class="">
> + struct TerminatedPath {<br class="">
> + MemoryAccess *Clobber;<br class="">
> + ListIndex LastNode;<br class="">
> + };<br class="">
> +<br class="">
> + /// Get an access that keeps us from optimizing to the given phi.<br class="">
> + ///<br class="">
> + /// PausedSearches is an array of indices into the Paths array. Its incoming<br class="">
> + /// value is the indices of searches that stopped at the last phi optimization<br class="">
> + /// target. It's left in an unspecified state.<br class="">
> + ///<br class="">
> + /// If this returns None, NewPaused is a vector of searches that terminated<br class="">
> + /// at StopWhere. Otherwise, NewPaused is left in an unspecified state.<br class="">
> + Optional<ListIndex><br class="">
> + getBlockingAccess(MemoryAccess *StopWhere,<br class="">
> + SmallVectorImpl<ListIndex> &PausedSearches,<br class="">
> + SmallVectorImpl<ListIndex> &NewPaused,<br class="">
> + SmallVectorImpl<TerminatedPath> &Terminated) {<br class="">
> + assert(!PausedSearches.empty() && "No searches to continue?");<br class="">
> +<br class="">
> + // BFS vs DFS really doesn't make a difference here, so just do a DFS with<br class="">
> + // PausedSearches as our stack.<br class="">
> + while (!PausedSearches.empty()) {<br class="">
> + ListIndex PathIndex = PausedSearches.pop_back_val();<br class="">
> + DefPath &Node = Paths[PathIndex];<br class="">
> +<br class="">
> + // If we've already visited this path with this MemoryLocation, we don't<br class="">
> + // need to do so again.<br class="">
> + //<br class="">
> + // NOTE: That we just drop these paths on the ground makes caching<br class="">
> + // behavior sporadic. e.g. given a diamond:<br class="">
> + // A<br class="">
> + // B C<br class="">
> + // D<br class="">
> + //<br class="">
> + // ...If we walk D, B, A, C, we'll only cache the result of phi<br class="">
> + // optimization for A, B, and D; C will be skipped because it dies here.<br class="">
> + // This arguably isn't the worst thing ever, since:<br class="">
> + // - We generally query things in a top-down order, so if we got below D<br class="">
> + // without needing cache entries for {C, MemLoc}, then chances are<br class="">
> + // that those cache entries would end up ultimately unused.<br class="">
> + // - We still cache things for A, so C only needs to walk up a bit.<br class="">
> + // If this behavior becomes problematic, we can fix without a ton of extra<br class="">
> + // work.<br class="">
> + if (!VisitedPhis.insert({Node.Last, Node.Loc}).second)<br class="">
> + continue;<br class="">
> +<br class="">
> + UpwardsWalkResult Res = walkToPhiOrClobber(Node, /*StopAt=*/StopWhere);<br class="">
> + if (Res.IsKnownClobber) {<br class="">
> + assert(Res.Result != StopWhere || Res.FromCache);<br class="">
> + // If this wasn't a cache hit, we hit a clobber when walking. That's a<br class="">
> + // failure.<br class="">
> + if (!Res.FromCache || !MSSA.dominates(Res.Result, StopWhere))<br class="">
> + return PathIndex;<br class="">
> +<br class="">
> + // Otherwise, it's a valid thing to potentially optimize to.<br class="">
> + Terminated.push_back({Res.Result, PathIndex});<br class="">
> + continue;<br class="">
> + }<br class="">
> +<br class="">
> + if (Res.Result == StopWhere) {<br class="">
> + // We've hit our target. Save this path off for if we want to continue<br class="">
> + // walking.<br class="">
> + NewPaused.push_back(PathIndex);<br class="">
> + continue;<br class="">
> + }<br class="">
> +<br class="">
> + assert(!MSSA.isLiveOnEntryDef(Res.Result) && "liveOnEntry is a clobber");<br class="">
> + addSearches(cast<MemoryPhi>(Res.Result), PausedSearches, PathIndex);<br class="">
> + }<br class="">
> +<br class="">
> + return None;<br class="">
> + }<br class="">
> +<br class="">
> + template <typename T, typename Walker><br class="">
> + struct generic_def_path_iterator<br class="">
> + : public iterator_facade_base<generic_def_path_iterator<T, Walker>,<br class="">
> + std::forward_iterator_tag, T *> {<br class="">
> + generic_def_path_iterator() : W(nullptr), N(None) {}<br class="">
> + generic_def_path_iterator(Walker *W, ListIndex N) : W(W), N(N) {}<br class="">
> +<br class="">
> + T &operator*() const { return curNode(); }<br class="">
> +<br class="">
> + generic_def_path_iterator &operator++() {<br class="">
> + N = curNode().Previous;<br class="">
> + return *this;<br class="">
> + }<br class="">
> +<br class="">
> + bool operator==(const generic_def_path_iterator &O) const {<br class="">
> + if (N.hasValue() != O.N.hasValue())<br class="">
> + return false;<br class="">
> + return !N.hasValue() || *N == *O.N;<br class="">
> + }<br class="">
> +<br class="">
> + private:<br class="">
> + T &curNode() const { return W->Paths[*N]; }<br class="">
> +<br class="">
> + Walker *W;<br class="">
> + Optional<ListIndex> N;<br class="">
> + };<br class="">
> +<br class="">
> + using def_path_iterator = generic_def_path_iterator<DefPath, ClobberWalker>;<br class="">
> + using const_def_path_iterator =<br class="">
> + generic_def_path_iterator<const DefPath, const ClobberWalker>;<br class="">
> +<br class="">
> + iterator_range<def_path_iterator> def_path(ListIndex From) {<br class="">
> + return make_range(def_path_iterator(this, From), def_path_iterator());<br class="">
> + }<br class="">
> +<br class="">
> + iterator_range<const_def_path_iterator> const_def_path(ListIndex From) const {<br class="">
> + return make_range(const_def_path_iterator(this, From),<br class="">
> + const_def_path_iterator());<br class="">
> + }<br class="">
> +<br class="">
> + struct OptznResult {<br class="">
> + /// The path that contains our result.<br class="">
> + TerminatedPath PrimaryClobber;<br class="">
> + /// The paths that we can legally cache back from, but that aren't<br class="">
> + /// necessarily the result of the Phi optimization.<br class="">
> + SmallVector<TerminatedPath, 4> OtherClobbers;<br class="">
> + };<br class="">
> +<br class="">
> + ListIndex defPathIndex(const DefPath &N) const {<br class="">
> + // The assert looks nicer if we don't need to do &N<br class="">
> + const DefPath *NP = &N;<br class="">
> + assert(!Paths.empty() && NP >= &Paths.front() && NP <= &Paths.back() &&<br class="">
> + "Out of bounds DefPath!");<br class="">
> + return NP - &Paths.front();<br class="">
> + }<br class="">
> +<br class="">
> + /// Try to optimize a phi as best as we can. Returns a SmallVector of Paths<br class="">
> + /// that act as legal clobbers. Note that this won't return *all* clobbers.<br class="">
> + ///<br class="">
> + /// Phi optimization algorithm tl;dr:<br class="">
> + /// - Find the earliest def/phi, A, we can optimize to<br class="">
> + /// - Find if all paths from the starting memory access ultimately reach A<br class="">
> + /// - If not, optimization isn't possible.<br class="">
> + /// - Otherwise, walk from A to another clobber or phi, A'.<br class="">
> + /// - If A' is a def, we're done.<br class="">
> + /// - If A' is a phi, try to optimize it.<br class="">
> + ///<br class="">
> + /// A path is a series of {MemoryAccess, MemoryLocation} pairs. A path<br class="">
> + /// terminates when a MemoryAccess that clobbers said MemoryLocation is found.<br class="">
> + OptznResult tryOptimizePhi(MemoryPhi *Phi, MemoryAccess *Start,<br class="">
> + const MemoryLocation &Loc) {<br class="">
> + assert(Paths.empty() && VisitedPhis.empty() &&<br class="">
> + "Reset the optimization state.");<br class="">
> +<br class="">
> + Paths.emplace_back(Loc, Start, Phi, None);<br class="">
> + // Stores how many "valid" optimization nodes we had prior to calling<br class="">
> + // addSearches/getBlockingAccess. Necessary for caching if we had a blocker.<br class="">
> + auto PriorPathsSize = Paths.size();<br class="">
> +<br class="">
> + SmallVector<ListIndex, 16> PausedSearches;<br class="">
> + SmallVector<ListIndex, 8> NewPaused;<br class="">
> + SmallVector<TerminatedPath, 4> TerminatedPaths;<br class="">
> +<br class="">
> + addSearches(Phi, PausedSearches, 0);<br class="">
> +<br class="">
> + // Moves the TerminatedPath with the "most dominated" Clobber to the end of<br class="">
> + // Paths.<br class="">
> + auto MoveDominatedPathToEnd = [&](SmallVectorImpl<TerminatedPath> &Paths) {<br class="">
> + assert(!Paths.empty() && "Need a path to move");<br class="">
> + // FIXME: This is technically n^2 (n = distance(DefPath.First,<br class="">
> + // DefPath.Last)) because of local dominance checks.<br class="">
> + auto Dom = Paths.begin();<br class="">
> + for (auto I = std::next(Dom), E = Paths.end(); I != E; ++I)<br class="">
> + if (!MSSA.dominates(I->Clobber, Dom->Clobber))<br class="">
> + Dom = I;<br class="">
> + auto Last = Paths.end() - 1;<br class="">
> + if (Last != Dom)<br class="">
> + std::iter_swap(Last, Dom);<br class="">
> + };<br class="">
> +<br class="">
> + MemoryPhi *Current = Phi;<br class="">
> + while (1) {<br class="">
> + assert(!MSSA.isLiveOnEntryDef(Current) &&<br class="">
> + "liveOnEntry wasn't treated as a clobber?");<br class="">
> +<br class="">
> + MemoryAccess *Target = getWalkTarget(Current);<br class="">
> + // If a TerminatedPath doesn't dominate Target, then it wasn't a legal<br class="">
> + // optimization for the prior phi.<br class="">
> + assert(all_of(TerminatedPaths, [&](const TerminatedPath &P) {<br class="">
> + return MSSA.dominates(P.Clobber, Target);<br class="">
> + }));<br class="">
> +<br class="">
> + // FIXME: This is broken, because the Blocker may be reported to be<br class="">
> + // liveOnEntry, and we'll happily wait for that to disappear (read: never)<br class="">
> + // For the moment, this is fine, since we do basically nothing with<br class="">
> + // blocker info.<br class="">
> + if (Optional<ListIndex> Blocker = getBlockingAccess(<br class="">
> + Target, PausedSearches, NewPaused, TerminatedPaths)) {<br class="">
> + MemoryAccess *BlockingAccess = Paths[*Blocker].Last;<br class="">
> + // Cache our work on the blocking node, since we know that's correct.<br class="">
> + cacheDefPath(Paths[*Blocker], BlockingAccess);<br class="">
> +<br class="">
> + // Find the node we started at. We can't search based on N->Last, since<br class="">
> + // we may have gone around a loop with a different MemoryLocation.<br class="">
> + auto Iter = find_if(def_path(*Blocker), [&](const DefPath &N) {<br class="">
> + return defPathIndex(N) < PriorPathsSize;<br class="">
> + });<br class="">
> + assert(Iter != def_path_iterator());<br class="">
> +<br class="">
> + DefPath &CurNode = *Iter;<br class="">
> + assert(CurNode.Last == Current);<br class="">
> + CurNode.Blocker = BlockingAccess;<br class="">
> +<br class="">
> + // Two things:<br class="">
> + // A. We can't reliably cache all of NewPaused back. Consider a case<br class="">
> + // where we have two paths in NewPaused; one of which can't optimize<br class="">
> + // above this phi, whereas the other can. If we cache the second path<br class="">
> + // back, we'll end up with suboptimal cache entries. We can handle<br class="">
> + // cases like this a bit better when we either try to find all<br class="">
> + // clobbers that block phi optimization, or when our cache starts<br class="">
> + // supporting unfinished searches.<br class="">
> + // B. We can't reliably cache TerminatedPaths back here without doing<br class="">
> + // extra checks; consider a case like:<br class="">
> + // T<br class="">
> + // / \<br class="">
> + // D C<br class="">
> + // \ /<br class="">
> + // S<br class="">
> + // Where T is our target, C is a node with a clobber on it, D is a<br class="">
> + // diamond (with a clobber *only* on the left or right node, N), and<br class="">
> + // S is our start. Say we walk to D, through the node opposite N<br class="">
> + // (read: ignoring the clobber), and see a cache entry in the top<br class="">
> + // node of D. That cache entry gets put into TerminatedPaths. We then<br class="">
> + // walk up to C (N is later in our worklist), find the clobber, and<br class="">
> + // quit. If we append TerminatedPaths to OtherClobbers, we'll cache<br class="">
> + // the bottom part of D to the cached clobber, ignoring the clobber<br class="">
> + // in N. Again, this problem goes away if we start tracking all<br class="">
> + // blockers for a given phi optimization.<br class="">
> + TerminatedPath Result{CurNode.Last, defPathIndex(CurNode)};<br class="">
> + return {Result, {}};<br class="">
> + }<br class="">
> +<br class="">
> + // If there's nothing left to search, then all paths led to valid clobbers<br class="">
> + // that we got from our cache; pick the nearest to the start, and allow<br class="">
> + // the rest to be cached back.<br class="">
> + if (NewPaused.empty()) {<br class="">
> + MoveDominatedPathToEnd(TerminatedPaths);<br class="">
> + TerminatedPath Result = TerminatedPaths.pop_back_val();<br class="">
> + return {Result, std::move(TerminatedPaths)};<br class="">
> + }<br class="">
> +<br class="">
> + MemoryAccess *DefChainEnd = nullptr;<br class="">
> + SmallVector<TerminatedPath, 4> Clobbers;<br class="">
> + for (ListIndex Paused : NewPaused) {<br class="">
> + UpwardsWalkResult WR = walkToPhiOrClobber(Paths[Paused]);<br class="">
> + if (WR.IsKnownClobber)<br class="">
> + Clobbers.push_back({WR.Result, Paused});<br class="">
> + else<br class="">
> + // Micro-opt: If we hit the end of the chain, save it.<br class="">
> + DefChainEnd = WR.Result;<br class="">
> + }<br class="">
> +<br class="">
> + if (!TerminatedPaths.empty()) {<br class="">
> + // If we couldn't find the dominating phi/liveOnEntry in the above loop,<br class="">
> + // do it now.<br class="">
> + if (!DefChainEnd)<br class="">
</div></div>> + for (MemoryAccess *MA : def_chain(Target)))<br class="">
> + DefChainEnd = MA;<br class="">
<br class="">
Is it guaranteed that `def_chain(Target)` isn’t empty?<br class="">
Otherwise you have nullptr deref on the next line (Reported by coverity who can’t know this invariant).<br class="">
<br class="">
—<br class="">
<span class=""><font color="#888888" class="">Mehdi<br class="">
</font></span><div class=""><div class=""><br class="">
> +<br class="">
> + // If any of the terminated paths don't dominate the phi we'll try to<br class="">
> + // optimize, we need to figure out what they are and quit.<br class="">
> + const BasicBlock *ChainBB = DefChainEnd->getBlock();<br class="">
> + for (const TerminatedPath &TP : TerminatedPaths) {<br class="">
> + // Because we know that DefChainEnd is as "high" as we can go, we<br class="">
> + // don't need local dominance checks; BB dominance is sufficient.<br class="">
> + if (DT.dominates(ChainBB, TP.Clobber->getBlock()))<br class="">
> + Clobbers.push_back(TP);<br class="">
> + }<br class="">
> + }<br class="">
> +<br class="">
> + // If we have clobbers in the def chain, find the one closest to Current<br class="">
> + // and quit.<br class="">
> + if (!Clobbers.empty()) {<br class="">
> + MoveDominatedPathToEnd(Clobbers);<br class="">
> + TerminatedPath Result = Clobbers.pop_back_val();<br class="">
> + return {Result, std::move(Clobbers)};<br class="">
> + }<br class="">
> +<br class="">
> + assert(all_of(NewPaused,<br class="">
> + [&](ListIndex I) { return Paths[I].Last == DefChainEnd; }));<br class="">
> +<br class="">
> + // Because liveOnEntry is a clobber, this must be a phi.<br class="">
> + auto *DefChainPhi = cast<MemoryPhi>(DefChainEnd);<br class="">
> +<br class="">
> + PriorPathsSize = Paths.size();<br class="">
> + PausedSearches.clear();<br class="">
> + for (ListIndex I : NewPaused)<br class="">
> + addSearches(DefChainPhi, PausedSearches, I);<br class="">
> + NewPaused.clear();<br class="">
> +<br class="">
> + Current = DefChainPhi;<br class="">
> + }<br class="">
> + }<br class="">
> +<br class="">
> + /// Caches everything in an OptznResult.<br class="">
> + void cacheOptResult(const OptznResult &R) {<br class="">
> + if (R.OtherClobbers.empty()) {<br class="">
> + // If we're not going to be caching OtherClobbers, don't bother with<br class="">
> + // marking visited/etc.<br class="">
> + for (const DefPath &N : const_def_path(R.PrimaryClobber.LastNode))<br class="">
> + cacheDefPath(N, R.PrimaryClobber.Clobber);<br class="">
> + return;<br class="">
> + }<br class="">
> +<br class="">
> + // PrimaryClobber is our answer. If we can cache anything back, we need to<br class="">
> + // stop caching when we visit PrimaryClobber.<br class="">
> + SmallBitVector Visited(Paths.size());<br class="">
> + for (const DefPath &N : const_def_path(R.PrimaryClobber.LastNode)) {<br class="">
> + Visited[defPathIndex(N)] = true;<br class="">
> + cacheDefPath(N, R.PrimaryClobber.Clobber);<br class="">
> + }<br class="">
> +<br class="">
> + for (const TerminatedPath &P : R.OtherClobbers) {<br class="">
> + for (const DefPath &N : const_def_path(P.LastNode)) {<br class="">
> + ListIndex NIndex = defPathIndex(N);<br class="">
> + if (Visited[NIndex])<br class="">
> + break;<br class="">
> + Visited[NIndex] = true;<br class="">
> + cacheDefPath(N, P.Clobber);<br class="">
> + }<br class="">
> + }<br class="">
> + }<br class="">
> +<br class="">
> + void verifyOptResult(const OptznResult &R) const {<br class="">
> + assert(all_of(R.OtherClobbers, [&](const TerminatedPath &P) {<br class="">
> + return MSSA.dominates(P.Clobber, R.PrimaryClobber.Clobber);<br class="">
> + }));<br class="">
> + }<br class="">
> +<br class="">
> + void resetPhiOptznState() {<br class="">
> + Paths.clear();<br class="">
> + VisitedPhis.clear();<br class="">
> + }<br class="">
> +<br class="">
> +public:<br class="">
> + ClobberWalker(const MemorySSA &MSSA, AliasAnalysis &AA, DominatorTree &DT,<br class="">
> + WalkerCache &WC)<br class="">
> + : MSSA(MSSA), AA(AA), DT(DT), WC(WC), UseCache(true) {}<br class="">
> +<br class="">
> + void reset() { WalkTargetCache.clear(); }<br class="">
> +<br class="">
> + /// Finds the nearest clobber for the given query, optimizing phis if<br class="">
> + /// possible.<br class="">
> + MemoryAccess *findClobber(MemoryAccess *Start, UpwardsMemoryQuery &Q,<br class="">
> + bool UseWalkerCache = true) {<br class="">
> + setUseCache(UseWalkerCache);<br class="">
> + Query = &Q;<br class="">
> +<br class="">
> + MemoryAccess *Current = Start;<br class="">
> + // This walker pretends uses don't exist. If we're handed one, silently grab<br class="">
> + // its def. (This has the nice side-effect of ensuring we never cache uses)<br class="">
> + if (auto *MU = dyn_cast<MemoryUse>(Start))<br class="">
> + Current = MU->getDefiningAccess();<br class="">
> +<br class="">
> + DefPath FirstDesc(Q.StartingLoc, Current, Current, None);<br class="">
> + // Fast path for the overly-common case (no crazy phi optimization<br class="">
> + // necessary)<br class="">
> + UpwardsWalkResult WalkResult = walkToPhiOrClobber(FirstDesc);<br class="">
> + if (WalkResult.IsKnownClobber) {<br class="">
> + cacheDefPath(FirstDesc, WalkResult.Result);<br class="">
> + return WalkResult.Result;<br class="">
> + }<br class="">
> +<br class="">
> + OptznResult OptRes =<br class="">
> + tryOptimizePhi(cast<MemoryPhi>(FirstDesc.Last), Current, Q.StartingLoc);<br class="">
> + verifyOptResult(OptRes);<br class="">
> + cacheOptResult(OptRes);<br class="">
> + resetPhiOptznState();<br class="">
> +<br class="">
> +#ifdef EXPENSIVE_CHECKS<br class="">
> + checkClobberSanity(Current, OptRes.PrimaryClobber.Clobber, Q.StartingLoc,<br class="">
> + MSSA, Q, AA);<br class="">
> +#endif<br class="">
> + return OptRes.PrimaryClobber.Clobber;<br class="">
> + }<br class="">
> +};<br class="">
> +<br class="">
> +struct RenamePassData {<br class="">
> + DomTreeNode *DTN;<br class="">
> + DomTreeNode::const_iterator ChildIt;<br class="">
> + MemoryAccess *IncomingVal;<br class="">
><br class="">
> + RenamePassData(DomTreeNode *D, DomTreeNode::const_iterator It,<br class="">
> + MemoryAccess *M)<br class="">
> + : DTN(D), ChildIt(It), IncomingVal(M) {}<br class="">
> + void swap(RenamePassData &RHS) {<br class="">
> + std::swap(DTN, RHS.DTN);<br class="">
> + std::swap(ChildIt, RHS.ChildIt);<br class="">
> + std::swap(IncomingVal, RHS.IncomingVal);<br class="">
> + }<br class="">
> +};<br class="">
> +} // anonymous namespace<br class="">
> +<br class="">
> +namespace llvm {<br class="">
> /// \brief A MemorySSAWalker that does AA walks and caching of lookups to<br class="">
> /// disambiguate accesses.<br class="">
> ///<br class="">
> @@ -121,6 +892,13 @@ public:<br class="">
> /// ret i32 %r<br class="">
> /// }<br class="">
> class MemorySSA::CachingWalker final : public MemorySSAWalker {<br class="">
> + WalkerCache Cache;<br class="">
> + ClobberWalker Walker;<br class="">
> + bool AutoResetWalker;<br class="">
> +<br class="">
> + MemoryAccess *getClobberingMemoryAccess(MemoryAccess *, UpwardsMemoryQuery &);<br class="">
> + void verifyRemoved(MemoryAccess *);<br class="">
> +<br class="">
> public:<br class="">
> CachingWalker(MemorySSA *, AliasAnalysis *, DominatorTree *);<br class="">
> ~CachingWalker() override;<br class="">
> @@ -130,50 +908,17 @@ public:<br class="">
> MemoryLocation &) override;<br class="">
> void invalidateInfo(MemoryAccess *) override;<br class="">
><br class="">
> -protected:<br class="">
> - struct UpwardsMemoryQuery;<br class="">
> - MemoryAccess *doCacheLookup(const MemoryAccess *, const UpwardsMemoryQuery &,<br class="">
> - const MemoryLocation &);<br class="">
> -<br class="">
> - void doCacheInsert(const MemoryAccess *, MemoryAccess *,<br class="">
> - const UpwardsMemoryQuery &, const MemoryLocation &);<br class="">
> -<br class="">
> - void doCacheRemove(const MemoryAccess *, const UpwardsMemoryQuery &,<br class="">
> - const MemoryLocation &);<br class="">
> -<br class="">
> -private:<br class="">
> - MemoryAccessPair UpwardsDFSWalk(MemoryAccess *, const MemoryLocation &,<br class="">
> - UpwardsMemoryQuery &, bool);<br class="">
> - MemoryAccess *getClobberingMemoryAccess(MemoryAccess *, UpwardsMemoryQuery &);<br class="">
> - bool instructionClobbersQuery(const MemoryDef *, UpwardsMemoryQuery &,<br class="">
> - const MemoryLocation &Loc) const;<br class="">
> - void verifyRemoved(MemoryAccess *);<br class="">
> - SmallDenseMap<ConstMemoryAccessPair, MemoryAccess *><br class="">
> - CachedUpwardsClobberingAccess;<br class="">
> - DenseMap<const MemoryAccess *, MemoryAccess *> CachedUpwardsClobberingCall;<br class="">
> - AliasAnalysis *AA;<br class="">
> - DominatorTree *DT;<br class="">
> + /// Whether we call resetClobberWalker() after each time we *actually* walk to<br class="">
> + /// answer a clobber query.<br class="">
> + void setAutoResetWalker(bool AutoReset) { AutoResetWalker = AutoReset; }<br class="">
> +<br class="">
> + /// Drop the walker's persistent data structures. At the moment, this means<br class="">
> + /// "drop the walker's cache of BasicBlocks -><br class="">
> + /// earliest-MemoryAccess-we-can-optimize-to". This is necessary if we're<br class="">
> + /// going to have DT updates, if we remove MemoryAccesses, etc.<br class="">
> + void resetClobberWalker() { Walker.reset(); }<br class="">
> };<br class="">
> -}<br class="">
><br class="">
> -namespace {<br class="">
> -struct RenamePassData {<br class="">
> - DomTreeNode *DTN;<br class="">
> - DomTreeNode::const_iterator ChildIt;<br class="">
> - MemoryAccess *IncomingVal;<br class="">
> -<br class="">
> - RenamePassData(DomTreeNode *D, DomTreeNode::const_iterator It,<br class="">
> - MemoryAccess *M)<br class="">
> - : DTN(D), ChildIt(It), IncomingVal(M) {}<br class="">
> - void swap(RenamePassData &RHS) {<br class="">
> - std::swap(DTN, RHS.DTN);<br class="">
> - std::swap(ChildIt, RHS.ChildIt);<br class="">
> - std::swap(IncomingVal, RHS.IncomingVal);<br class="">
> - }<br class="">
> -};<br class="">
> -}<br class="">
> -<br class="">
> -namespace llvm {<br class="">
> /// \brief Rename a single basic block into MemorySSA form.<br class="">
> /// Uses the standard SSA renaming algorithm.<br class="">
> /// \returns The new incoming value.<br class="">
> @@ -417,7 +1162,10 @@ void MemorySSA::buildMemorySSA() {<br class="">
> SmallPtrSet<BasicBlock *, 16> Visited;<br class="">
> renamePass(DT->getRootNode(), LiveOnEntryDef.get(), Visited);<br class="">
><br class="">
> - MemorySSAWalker *Walker = getWalker();<br class="">
> + CachingWalker *Walker = getWalkerImpl();<br class="">
> +<br class="">
> + // We're doing a batch of updates; don't drop useful caches between them.<br class="">
> + Walker->setAutoResetWalker(false);<br class="">
><br class="">
> // Now optimize the MemoryUse's defining access to point to the nearest<br class="">
> // dominating clobbering def.<br class="">
> @@ -437,6 +1185,9 @@ void MemorySSA::buildMemorySSA() {<br class="">
> }<br class="">
> }<br class="">
><br class="">
> + Walker->setAutoResetWalker(true);<br class="">
> + Walker->resetClobberWalker();<br class="">
> +<br class="">
> // Mark the uses in unreachable blocks as live on entry, so that they go<br class="">
> // somewhere.<br class="">
> for (auto &BB : F)<br class="">
> @@ -444,7 +1195,9 @@ void MemorySSA::buildMemorySSA() {<br class="">
> markUnreachableAsLiveOnEntry(&BB);<br class="">
> }<br class="">
><br class="">
> -MemorySSAWalker *MemorySSA::getWalker() {<br class="">
> +MemorySSAWalker *MemorySSA::getWalker() { return getWalkerImpl(); }<br class="">
> +<br class="">
> +MemorySSA::CachingWalker *MemorySSA::getWalkerImpl() {<br class="">
> if (Walker)<br class="">
> return Walker.get();<br class="">
><br class="">
> @@ -820,7 +1573,6 @@ MemoryPhi *MemorySSA::getMemoryAccess(co<br class="">
> /// \returns True if \p Dominator dominates \p Dominatee.<br class="">
> bool MemorySSA::locallyDominates(const MemoryAccess *Dominator,<br class="">
> const MemoryAccess *Dominatee) const {<br class="">
> -<br class="">
> assert((Dominator->getBlock() == Dominatee->getBlock()) &&<br class="">
> "Asking for local domination when accesses are in different blocks!");<br class="">
><br class="">
> @@ -848,6 +1600,19 @@ bool MemorySSA::locallyDominates(const M<br class="">
> [&](const MemoryAccess &MA) { return &MA == Dominatee; });<br class="">
> }<br class="">
><br class="">
> +bool MemorySSA::dominates(const MemoryAccess *Dominator,<br class="">
> + const MemoryAccess *Dominatee) const {<br class="">
> + if (Dominator == Dominatee)<br class="">
> + return true;<br class="">
> +<br class="">
> + if (isLiveOnEntryDef(Dominatee))<br class="">
> + return false;<br class="">
> +<br class="">
> + if (Dominator->getBlock() != Dominatee->getBlock())<br class="">
> + return DT->dominates(Dominator->getBlock(), Dominatee->getBlock());<br class="">
> + return locallyDominates(Dominator, Dominatee);<br class="">
> +}<br class="">
> +<br class="">
> const static char LiveOnEntryStr[] = "liveOnEntry";<br class="">
><br class="">
> void MemoryDef::print(raw_ostream &OS) const {<br class="">
> @@ -978,41 +1743,12 @@ MemorySSAWalker::MemorySSAWalker(MemoryS<br class="">
><br class="">
> MemorySSA::CachingWalker::CachingWalker(MemorySSA *M, AliasAnalysis *A,<br class="">
> DominatorTree *D)<br class="">
> - : MemorySSAWalker(M), AA(A), DT(D) {}<br class="">
> + : MemorySSAWalker(M), Walker(*M, *A, *D, Cache),<br class="">
> + AutoResetWalker(true) {}<br class="">
><br class="">
> MemorySSA::CachingWalker::~CachingWalker() {}<br class="">
><br class="">
> -struct MemorySSA::CachingWalker::UpwardsMemoryQuery {<br class="">
> - // True if we saw a phi whose predecessor was a backedge<br class="">
> - bool SawBackedgePhi;<br class="">
> - // True if our original query started off as a call<br class="">
> - bool IsCall;<br class="">
> - // The pointer location we started the query with. This will be empty if<br class="">
> - // IsCall is true.<br class="">
> - MemoryLocation StartingLoc;<br class="">
> - // This is the instruction we were querying about.<br class="">
> - const Instruction *Inst;<br class="">
> - // Set of visited Instructions for this query.<br class="">
> - DenseSet<MemoryAccessPair> Visited;<br class="">
> - // Vector of visited call accesses for this query. This is separated out<br class="">
> - // because you can always cache and lookup the result of call queries (IE when<br class="">
> - // IsCall == true) for every call in the chain. The calls have no AA location<br class="">
> - // associated with them with them, and thus, no context dependence.<br class="">
> - SmallVector<const MemoryAccess *, 32> VisitedCalls;<br class="">
> - // The MemoryAccess we actually got called with, used to test local domination<br class="">
> - const MemoryAccess *OriginalAccess;<br class="">
> -<br class="">
> - UpwardsMemoryQuery()<br class="">
> - : SawBackedgePhi(false), IsCall(false), Inst(nullptr),<br class="">
> - OriginalAccess(nullptr) {}<br class="">
> -<br class="">
> - UpwardsMemoryQuery(const Instruction *Inst, const MemoryAccess *Access)<br class="">
> - : SawBackedgePhi(false), IsCall(ImmutableCallSite(Inst)), Inst(Inst),<br class="">
> - OriginalAccess(Access) {}<br class="">
> -};<br class="">
> -<br class="">
> void MemorySSA::CachingWalker::invalidateInfo(MemoryAccess *MA) {<br class="">
> -<br class="">
> // TODO: We can do much better cache invalidation with differently stored<br class="">
> // caches. For now, for MemoryUses, we simply remove them<br class="">
> // from the cache, and kill the entire call/non-call cache for everything<br class="">
> @@ -1026,217 +1762,33 @@ void MemorySSA::CachingWalker::invalidat<br class="">
> // itself.<br class="">
><br class="">
> if (MemoryUse *MU = dyn_cast<MemoryUse>(MA)) {<br class="">
> - UpwardsMemoryQuery Q;<br class="">
> - Instruction *I = MU->getMemoryInst();<br class="">
> - Q.IsCall = bool(ImmutableCallSite(I));<br class="">
> - Q.Inst = I;<br class="">
> - if (!Q.IsCall)<br class="">
> - Q.StartingLoc = MemoryLocation::get(I);<br class="">
> - doCacheRemove(MA, Q, Q.StartingLoc);<br class="">
> + UpwardsMemoryQuery Q(MU->getMemoryInst(), MU);<br class="">
> + Cache.remove(MU, Q.StartingLoc, Q.IsCall);<br class="">
> } else {<br class="">
> // If it is not a use, the best we can do right now is destroy the cache.<br class="">
> - CachedUpwardsClobberingCall.clear();<br class="">
> - CachedUpwardsClobberingAccess.clear();<br class="">
> + Cache.clear();<br class="">
> }<br class="">
><br class="">
> #ifdef EXPENSIVE_CHECKS<br class="">
> - // Run this only when expensive checks are enabled.<br class="">
> verifyRemoved(MA);<br class="">
> #endif<br class="">
> }<br class="">
><br class="">
> -void MemorySSA::CachingWalker::doCacheRemove(const MemoryAccess *M,<br class="">
> - const UpwardsMemoryQuery &Q,<br class="">
> - const MemoryLocation &Loc) {<br class="">
> - if (Q.IsCall)<br class="">
> - CachedUpwardsClobberingCall.erase(M);<br class="">
> - else<br class="">
> - CachedUpwardsClobberingAccess.erase({M, Loc});<br class="">
> -}<br class="">
> -<br class="">
> -void MemorySSA::CachingWalker::doCacheInsert(const MemoryAccess *M,<br class="">
> - MemoryAccess *Result,<br class="">
> - const UpwardsMemoryQuery &Q,<br class="">
> - const MemoryLocation &Loc) {<br class="">
> - // This is fine for Phis, since there are times where we can't optimize them.<br class="">
> - // Making a def its own clobber is never correct, though.<br class="">
> - assert((Result != M || isa<MemoryPhi>(M)) &&<br class="">
> - "Something can't clobber itself!");<br class="">
> - ++NumClobberCacheInserts;<br class="">
> - if (Q.IsCall)<br class="">
> - CachedUpwardsClobberingCall[M] = Result;<br class="">
> - else<br class="">
> - CachedUpwardsClobberingAccess[{M, Loc}] = Result;<br class="">
> -}<br class="">
> -<br class="">
> -MemoryAccess *<br class="">
> -MemorySSA::CachingWalker::doCacheLookup(const MemoryAccess *M,<br class="">
> - const UpwardsMemoryQuery &Q,<br class="">
> - const MemoryLocation &Loc) {<br class="">
> - ++NumClobberCacheLookups;<br class="">
> - MemoryAccess *Result;<br class="">
> -<br class="">
> - if (Q.IsCall)<br class="">
> - Result = CachedUpwardsClobberingCall.lookup(M);<br class="">
> - else<br class="">
> - Result = CachedUpwardsClobberingAccess.lookup({M, Loc});<br class="">
> -<br class="">
> - if (Result)<br class="">
> - ++NumClobberCacheHits;<br class="">
> - return Result;<br class="">
> -}<br class="">
> -<br class="">
> -bool MemorySSA::CachingWalker::instructionClobbersQuery(<br class="">
> - const MemoryDef *MD, UpwardsMemoryQuery &Q,<br class="">
> - const MemoryLocation &Loc) const {<br class="">
> - Instruction *DefMemoryInst = MD->getMemoryInst();<br class="">
> - assert(DefMemoryInst && "Defining instruction not actually an instruction");<br class="">
> -<br class="">
> - if (!Q.IsCall)<br class="">
> - return AA->getModRefInfo(DefMemoryInst, Loc) & MRI_Mod;<br class="">
> -<br class="">
> - // If this is a call, mark it for caching<br class="">
> - if (ImmutableCallSite(DefMemoryInst))<br class="">
> - Q.VisitedCalls.push_back(MD);<br class="">
> - ModRefInfo I = AA->getModRefInfo(DefMemoryInst, ImmutableCallSite(Q.Inst));<br class="">
> - return I != MRI_NoModRef;<br class="">
> -}<br class="">
> -<br class="">
> -MemoryAccessPair MemorySSA::CachingWalker::UpwardsDFSWalk(<br class="">
> - MemoryAccess *StartingAccess, const MemoryLocation &Loc,<br class="">
> - UpwardsMemoryQuery &Q, bool FollowingBackedge) {<br class="">
> - MemoryAccess *ModifyingAccess = nullptr;<br class="">
> -<br class="">
> - auto DFI = df_begin(StartingAccess);<br class="">
> - for (auto DFE = df_end(StartingAccess); DFI != DFE;) {<br class="">
> - MemoryAccess *CurrAccess = *DFI;<br class="">
> - if (MSSA->isLiveOnEntryDef(CurrAccess))<br class="">
> - return {CurrAccess, Loc};<br class="">
> - // If this is a MemoryDef, check whether it clobbers our current query. This<br class="">
> - // needs to be done before consulting the cache, because the cache reports<br class="">
> - // the clobber for CurrAccess. If CurrAccess is a clobber for this query,<br class="">
> - // and we ask the cache for information first, then we might skip this<br class="">
> - // clobber, which is bad.<br class="">
> - if (auto *MD = dyn_cast<MemoryDef>(CurrAccess)) {<br class="">
> - // If we hit the top, stop following this path.<br class="">
> - // While we can do lookups, we can't sanely do inserts here unless we were<br class="">
> - // to track everything we saw along the way, since we don't know where we<br class="">
> - // will stop.<br class="">
> - if (instructionClobbersQuery(MD, Q, Loc)) {<br class="">
> - ModifyingAccess = CurrAccess;<br class="">
> - break;<br class="">
> - }<br class="">
> - }<br class="">
> - if (auto CacheResult = doCacheLookup(CurrAccess, Q, Loc))<br class="">
> - return {CacheResult, Loc};<br class="">
> -<br class="">
> - // We need to know whether it is a phi so we can track backedges.<br class="">
> - // Otherwise, walk all upward defs.<br class="">
> - if (!isa<MemoryPhi>(CurrAccess)) {<br class="">
> - ++DFI;<br class="">
> - continue;<br class="">
> - }<br class="">
> -<br class="">
> -#ifndef NDEBUG<br class="">
> - // The loop below visits the phi's children for us. Because phis are the<br class="">
> - // only things with multiple edges, skipping the children should always lead<br class="">
> - // us to the end of the loop.<br class="">
> - //<br class="">
> - // Use a copy of DFI because skipChildren would kill our search stack, which<br class="">
> - // would make caching anything on the way back impossible.<br class="">
> - auto DFICopy = DFI;<br class="">
> - assert(DFICopy.skipChildren() == DFE &&<br class="">
> - "Skipping phi's children doesn't end the DFS?");<br class="">
> -#endif<br class="">
> -<br class="">
> - const MemoryAccessPair PHIPair(CurrAccess, Loc);<br class="">
> -<br class="">
> - // Don't try to optimize this phi again if we've already tried to do so.<br class="">
> - if (!Q.Visited.insert(PHIPair).second) {<br class="">
> - ModifyingAccess = CurrAccess;<br class="">
> - break;<br class="">
> - }<br class="">
> -<br class="">
> - std::size_t InitialVisitedCallSize = Q.VisitedCalls.size();<br class="">
> -<br class="">
> - // Recurse on PHI nodes, since we need to change locations.<br class="">
> - // TODO: Allow graphtraits on pairs, which would turn this whole function<br class="">
> - // into a normal single depth first walk.<br class="">
> - MemoryAccess *FirstDef = nullptr;<br class="">
> - for (auto MPI = upward_defs_begin(PHIPair), MPE = upward_defs_end();<br class="">
> - MPI != MPE; ++MPI) {<br class="">
> - bool Backedge =<br class="">
> - !FollowingBackedge &&<br class="">
> - DT->dominates(CurrAccess->getBlock(), MPI.getPhiArgBlock());<br class="">
> -<br class="">
> - MemoryAccessPair CurrentPair =<br class="">
> - UpwardsDFSWalk(MPI->first, MPI->second, Q, Backedge);<br class="">
> - // All the phi arguments should reach the same point if we can bypass<br class="">
> - // this phi. The alternative is that they hit this phi node, which<br class="">
> - // means we can skip this argument.<br class="">
> - if (FirstDef && CurrentPair.first != PHIPair.first &&<br class="">
> - CurrentPair.first != FirstDef) {<br class="">
> - ModifyingAccess = CurrAccess;<br class="">
> - break;<br class="">
> - }<br class="">
> -<br class="">
> - if (!FirstDef)<br class="">
> - FirstDef = CurrentPair.first;<br class="">
> - }<br class="">
> -<br class="">
> - // If we exited the loop early, go with the result it gave us.<br class="">
> - if (!ModifyingAccess) {<br class="">
> - assert(FirstDef && "Found a Phi with no upward defs?");<br class="">
> - ModifyingAccess = FirstDef;<br class="">
> - } else {<br class="">
> - // If we can't optimize this Phi, then we can't safely cache any of the<br class="">
> - // calls we visited when trying to optimize it. Wipe them out now.<br class="">
> - Q.VisitedCalls.resize(InitialVisitedCallSize);<br class="">
> - }<br class="">
> - break;<br class="">
> - }<br class="">
> -<br class="">
> - if (!ModifyingAccess)<br class="">
> - return {MSSA->getLiveOnEntryDef(), Q.StartingLoc};<br class="">
> -<br class="">
> - const BasicBlock *OriginalBlock = StartingAccess->getBlock();<br class="">
> - assert(DFI.getPathLength() > 0 && "We dropped our path?");<br class="">
> - unsigned N = DFI.getPathLength();<br class="">
> - // If we found a clobbering def, the last element in the path will be our<br class="">
> - // clobber, so we don't want to cache that to itself. OTOH, if we optimized a<br class="">
> - // phi, we can add the last thing in the path to the cache, since that won't<br class="">
> - // be the result.<br class="">
> - if (DFI.getPath(N - 1) == ModifyingAccess)<br class="">
> - --N;<br class="">
> - for (; N > 1; --N) {<br class="">
> - MemoryAccess *CacheAccess = DFI.getPath(N - 1);<br class="">
> - BasicBlock *CurrBlock = CacheAccess->getBlock();<br class="">
> - if (!FollowingBackedge)<br class="">
> - doCacheInsert(CacheAccess, ModifyingAccess, Q, Loc);<br class="">
> - if (DT->dominates(CurrBlock, OriginalBlock) &&<br class="">
> - (CurrBlock != OriginalBlock || !FollowingBackedge ||<br class="">
> - MSSA->locallyDominates(CacheAccess, StartingAccess)))<br class="">
> - break;<br class="">
> - }<br class="">
> -<br class="">
> - // Cache everything else on the way back. The caller should cache<br class="">
> - // StartingAccess for us.<br class="">
> - for (; N > 1; --N) {<br class="">
> - MemoryAccess *CacheAccess = DFI.getPath(N - 1);<br class="">
> - doCacheInsert(CacheAccess, ModifyingAccess, Q, Loc);<br class="">
> - }<br class="">
> - assert(Q.Visited.size() < 1000 && "Visited too much");<br class="">
> -<br class="">
> - return {ModifyingAccess, Loc};<br class="">
> -}<br class="">
> -<br class="">
> /// \brief Walk the use-def chains starting at \p MA and find<br class="">
> /// the MemoryAccess that actually clobbers Loc.<br class="">
> ///<br class="">
> /// \returns our clobbering memory access<br class="">
> MemoryAccess *MemorySSA::CachingWalker::getClobberingMemoryAccess(<br class="">
> MemoryAccess *StartingAccess, UpwardsMemoryQuery &Q) {<br class="">
> - return UpwardsDFSWalk(StartingAccess, Q.StartingLoc, Q, false).first;<br class="">
> + MemoryAccess *New = Walker.findClobber(StartingAccess, Q);<br class="">
> +#ifdef EXPENSIVE_CHECKS<br class="">
> + MemoryAccess *NewNoCache =<br class="">
> + Walker.findClobber(StartingAccess, Q, /*UseWalkerCache=*/false);<br class="">
> + assert(NewNoCache == New && "Cache made us hand back a different result?");<br class="">
> +#endif<br class="">
> + if (AutoResetWalker)<br class="">
> + resetClobberWalker();<br class="">
> + return New;<br class="">
> }<br class="">
><br class="">
> MemoryAccess *MemorySSA::CachingWalker::getClobberingMemoryAccess(<br class="">
> @@ -1258,10 +1810,10 @@ MemoryAccess *MemorySSA::CachingWalker::<br class="">
> UpwardsMemoryQuery Q;<br class="">
> Q.OriginalAccess = StartingUseOrDef;<br class="">
> Q.StartingLoc = Loc;<br class="">
> - Q.Inst = StartingUseOrDef->getMemoryInst();<br class="">
> + Q.Inst = I;<br class="">
> Q.IsCall = false;<br class="">
><br class="">
> - if (auto CacheResult = doCacheLookup(StartingUseOrDef, Q, Q.StartingLoc))<br class="">
> + if (auto *CacheResult = Cache.lookup(StartingUseOrDef, Loc, Q.IsCall))<br class="">
> return CacheResult;<br class="">
><br class="">
> // Unlike the other function, do not walk to the def of a def, because we are<br class="">
> @@ -1271,9 +1823,6 @@ MemoryAccess *MemorySSA::CachingWalker::<br class="">
> : StartingUseOrDef;<br class="">
><br class="">
> MemoryAccess *Clobber = getClobberingMemoryAccess(DefiningAccess, Q);<br class="">
> - // Only cache this if it wouldn't make Clobber point to itself.<br class="">
> - if (Clobber != StartingAccess)<br class="">
> - doCacheInsert(Q.OriginalAccess, Clobber, Q, Q.StartingLoc);<br class="">
> DEBUG(dbgs() << "Starting Memory SSA clobber for " << *I << " is ");<br class="">
> DEBUG(dbgs() << *StartingUseOrDef << "\n");<br class="">
> DEBUG(dbgs() << "Final Memory SSA clobber for " << *I << " is ");<br class="">
> @@ -1287,21 +1836,14 @@ MemorySSA::CachingWalker::getClobberingM<br class="">
> // access, since we only map BB's to PHI's. So, this must be a use or def.<br class="">
> auto *StartingAccess = cast<MemoryUseOrDef>(MSSA->getMemoryAccess(I));<br class="">
><br class="">
> - bool IsCall = bool(ImmutableCallSite(I));<br class="">
> -<br class="">
> + UpwardsMemoryQuery Q(I, StartingAccess);<br class="">
> // We can't sanely do anything with a fences, they conservatively<br class="">
> // clobber all memory, and have no locations to get pointers from to<br class="">
> // try to disambiguate.<br class="">
> - if (!IsCall && I->isFenceLike())<br class="">
> + if (!Q.IsCall && I->isFenceLike())<br class="">
> return StartingAccess;<br class="">
><br class="">
> - UpwardsMemoryQuery Q;<br class="">
> - Q.OriginalAccess = StartingAccess;<br class="">
> - Q.IsCall = IsCall;<br class="">
> - if (!Q.IsCall)<br class="">
> - Q.StartingLoc = MemoryLocation::get(I);<br class="">
> - Q.Inst = I;<br class="">
> - if (auto CacheResult = doCacheLookup(StartingAccess, Q, Q.StartingLoc))<br class="">
> + if (auto *CacheResult = Cache.lookup(StartingAccess, Q.StartingLoc, Q.IsCall))<br class="">
> return CacheResult;<br class="">
><br class="">
> // Start with the thing we already think clobbers this location<br class="">
> @@ -1313,18 +1855,6 @@ MemorySSA::CachingWalker::getClobberingM<br class="">
> return DefiningAccess;<br class="">
><br class="">
> MemoryAccess *Result = getClobberingMemoryAccess(DefiningAccess, Q);<br class="">
> - // DFS won't cache a result for DefiningAccess. So, if DefiningAccess isn't<br class="">
> - // our clobber, be sure that it gets a cache entry, too.<br class="">
> - if (Result != DefiningAccess)<br class="">
> - doCacheInsert(DefiningAccess, Result, Q, Q.StartingLoc);<br class="">
> - doCacheInsert(Q.OriginalAccess, Result, Q, Q.StartingLoc);<br class="">
> - // TODO: When this implementation is more mature, we may want to figure out<br class="">
> - // what this additional caching buys us. It's most likely A Good Thing.<br class="">
> - if (Q.IsCall)<br class="">
> - for (const MemoryAccess *MA : Q.VisitedCalls)<br class="">
> - if (MA != Result)<br class="">
> - doCacheInsert(MA, Result, Q, Q.StartingLoc);<br class="">
> -<br class="">
> DEBUG(dbgs() << "Starting Memory SSA clobber for " << *I << " is ");<br class="">
> DEBUG(dbgs() << *DefiningAccess << "\n");<br class="">
> DEBUG(dbgs() << "Final Memory SSA clobber for " << *I << " is ");<br class="">
> @@ -1335,14 +1865,7 @@ MemorySSA::CachingWalker::getClobberingM<br class="">
><br class="">
> // Verify that MA doesn't exist in any of the caches.<br class="">
> void MemorySSA::CachingWalker::verifyRemoved(MemoryAccess *MA) {<br class="">
> -#ifndef NDEBUG<br class="">
> - for (auto &P : CachedUpwardsClobberingAccess)<br class="">
> - assert(P.first.first != MA && P.second != MA &&<br class="">
> - "Found removed MemoryAccess in cache.");<br class="">
> - for (auto &P : CachedUpwardsClobberingCall)<br class="">
> - assert(P.first != MA && P.second != MA &&<br class="">
> - "Found removed MemoryAccess in cache.");<br class="">
> -#endif // !NDEBUG<br class="">
> + assert(!Cache.contains(MA) && "Found removed MemoryAccess in cache.");<br class="">
> }<br class="">
><br class="">
> MemoryAccess *<br class="">
> @@ -1359,4 +1882,4 @@ MemoryAccess *DoNothingMemorySSAWalker::<br class="">
> return Use->getDefiningAccess();<br class="">
> return StartingAccess;<br class="">
> }<br class="">
> -}<br class="">
> +} // namespace llvm<br class="">
><br class="">
> Modified: llvm/trunk/test/Transforms/Util/MemorySSA/cyclicphi.ll<br class="">
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Util/MemorySSA/cyclicphi.ll?rev=275940&r1=275939&r2=275940&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Util/MemorySSA/cyclicphi.ll?rev=275940&r1=275939&r2=275940&view=diff</a><br class="">
> ==============================================================================<br class="">
> --- llvm/trunk/test/Transforms/Util/MemorySSA/cyclicphi.ll (original)<br class="">
> +++ llvm/trunk/test/Transforms/Util/MemorySSA/cyclicphi.ll Mon Jul 18 20:29:15 2016<br class="">
> @@ -56,8 +56,7 @@ bb68:<br class="">
><br class="">
> bb77: ; preds = %bb68, %bb26<br class="">
> ; CHECK: 2 = MemoryPhi({bb26,3},{bb68,1})<br class="">
> -; FIXME: This should be MemoryUse(liveOnEntry)<br class="">
> -; CHECK: MemoryUse(3)<br class="">
> +; CHECK: MemoryUse(liveOnEntry)<br class="">
> ; CHECK-NEXT: %tmp78 = load i64*, i64** %tmp25, align 8<br class="">
> %tmp78 = load i64*, i64** %tmp25, align 8<br class="">
> br label %bb26<br class="">
><br class="">
> Modified: llvm/trunk/test/Transforms/Util/MemorySSA/phi-translation.ll<br class="">
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Util/MemorySSA/phi-translation.ll?rev=275940&r1=275939&r2=275940&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Util/MemorySSA/phi-translation.ll?rev=275940&r1=275939&r2=275940&view=diff</a><br class="">
> ==============================================================================<br class="">
> --- llvm/trunk/test/Transforms/Util/MemorySSA/phi-translation.ll (original)<br class="">
> +++ llvm/trunk/test/Transforms/Util/MemorySSA/phi-translation.ll Mon Jul 18 20:29:15 2016<br class="">
> @@ -138,8 +138,7 @@ loop.3:<br class="">
> ; CHECK: 4 = MemoryDef(5)<br class="">
> ; CHECK-NEXT: store i8 2, i8* %p2<br class="">
> store i8 2, i8* %p2<br class="">
> -; FIXME: This should be MemoryUse(1)<br class="">
> -; CHECK: MemoryUse(5)<br class="">
> +; CHECK: MemoryUse(1)<br class="">
> ; CHECK-NEXT: load i8, i8* %p1<br class="">
> load i8, i8* %p1<br class="">
> br i1 undef, label %loop.2, label %loop.1<br class="">
><br class="">
><br class="">
> _______________________________________________<br class="">
> llvm-commits mailing list<br class="">
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a><br class="">
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="">
<br class="">
</div></div></blockquote></div><br class=""></div></div>
</div></blockquote></div><br class=""></body></html>