[PATCH] D19695: [MemorySSA] Fix bug in CachingMemorySSAWalker::getClobberingMemoryAccess.

[Geoff Berry via llvm-commits]

gberry created this revision.
gberry added reviewers: dberlin, george.burgess.iv.
gberry added a subscriber: llvm-commits.
Herald added a subscriber: mcrosier.

For the Instruction version of
CachingMemorySSAWalker::getClobberingMemoryAccess, when checking the
cache for previously computed values, the key used was I's MemoryAccess,
instead of its defining access.  If a previous call to the
non-instruction version of getClobberingMemoryAccess has been called, it
will have added I's MemoryAccess itself as the cached result (in the
case it is a MemoryDef).

For example, for the following code:

  1 = MemoryDef(liveOnEntry)
  store %a

before this change (assuming the calls to getClobberingMemoryAccess are
made in order):

  MSSA.getClobberingMemoryAccess(MSSA.getMemoryAccess(I), IMemLoc)
    -> 1 = MemoryDef(liveOnEntry)
  MSSA.getClobberingMemoryAccess(I)
    -> 1 = MemoryDef(liveOnEntry)

after this change, the second call correctly returns 'liveOnEntry':

  MSSA.getClobberingMemoryAccess(MSSA.getMemoryAccess(I), IMemLoc)
    -> 1 = MemoryDef(liveOnEntry)
  MSSA.getClobberingMemoryAccess(I)
    -> liveOnEntry

http://reviews.llvm.org/D19695

Files:
  lib/Transforms/Utils/MemorySSA.cpp

Index: lib/Transforms/Utils/MemorySSA.cpp
===================================================================
--- lib/Transforms/Utils/MemorySSA.cpp
+++ lib/Transforms/Utils/MemorySSA.cpp
@@ -1044,24 +1044,24 @@
   if (isa<FenceInst>(I))
     return StartingAccess;
 
+  // Start with the thing we already think clobbers this location
+  MemoryAccess *DefiningAccess = StartingAccess->getDefiningAccess();
+
+  // At this point, DefiningAccess may be the live on entry def.
+  // If it is, we will not get a better result.
+  if (MSSA->isLiveOnEntryDef(DefiningAccess))
+    return DefiningAccess;
+
   UpwardsMemoryQuery Q;
   Q.OriginalAccess = StartingAccess;
   Q.IsCall = bool(ImmutableCallSite(I));
   if (!Q.IsCall)
     Q.StartingLoc = MemoryLocation::get(I);
   Q.Inst = I;
   Q.DL = &Q.Inst->getModule()->getDataLayout();
-  if (auto CacheResult = doCacheLookup(StartingAccess, Q, Q.StartingLoc))
+  if (auto CacheResult = doCacheLookup(DefiningAccess, Q, Q.StartingLoc))
     return CacheResult;
 
-  // Start with the thing we already think clobbers this location
-  MemoryAccess *DefiningAccess = StartingAccess->getDefiningAccess();
-
-  // At this point, DefiningAccess may be the live on entry def.
-  // If it is, we will not get a better result.
-  if (MSSA->isLiveOnEntryDef(DefiningAccess))
-    return DefiningAccess;
-
   MemoryAccess *Result = getClobberingMemoryAccess(DefiningAccess, Q);
   doCacheInsert(Q.OriginalAccess, Result, Q, Q.StartingLoc);
   // TODO: When this implementation is more mature, we may want to figure out


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D19695.55504.patch
Type: text/x-patch
Size: 1554 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160428/b2654e89/attachment.bin>