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

[George Burgess IV via llvm-commits]

george.burgess.iv added a comment.

FWIW, the only way I know of to test this would be to make a unittest.

------

What happens if we have a chain like:

  %1 = alloca i8
  ; 1 = MemoryDef(liveOnEntry)
  store i8 0, i8* %1
  ; 2 = MemoryDef(1)
  store i8 1, i8* %1
  ; 3 = MemoryDef(2)
  store i8 2, i8* %1

And query from top to bottom using `getClobberingMemoryAccess(const Instruction *)`? I *believe* that our behavior will be:

- First store: Get MD#1's clobber, `liveOnEntry`, return it without caching anything.
- Second store: Get MD#2's clobber, MD#1, find nothing in cache, determine the clobber is MD#1, cache `{MD#2, %1} -> MD#1`.
- Third store: Get MD#3's clobber, MD#2, find `{MD#2, %1} -> MD#1` in cache, return MD#1.

...Which isn't correct.

That said, I'm not so sure that having two functions that can cache different results for (what ultimately are) the same queries is a great idea. As a quick fix, maybe just not caching the final result for `getClobberingMemoryAccess(MA*, MemLoc&)` would work? (We'd have to make sure the Walker's caching loop doesn't try to cache this for us as well, if we decide to do this.)


http://reviews.llvm.org/D19695