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

[George Burgess IV via llvm-commits]

george.burgess.iv added a comment.

> So will you do a superset or should we approve this patch.


I planned to just fix the walker, but that ended up being a fix for two different-but-related bugs that seemingly also fixes this problem for free, so superset I guess. Sorry for hijacking this, @gberry; it wasn't my intention to do so. :/ (And thanks for bringing this issue up! :) )

I plan to have the fix upstreamed by noon-ish PST tomorrow.

The bugs ended up being:

- `Cache[Foo]` gives us the clobber of `Foo`. So, if `DefiningAccess` is the clobber for `Q.OriginalAccess`, and we start walking with `DefiningAccess`, we look for `Cache[DefiningAccess]` before checking if `DefiningAccess` is a clobber.
- We would (sometimes in the DFS, sometimes in the `getClobbering...` methods) at times cache a MemoryAccess as a clobber for itself. Sometimes this is fine (e.g. Phis we can't optimize), but for Defs, it isn't.

> In retrospect, we should have added caching later with more full unit tests, given how complex it ends up


Agreed. Also, when I was adding the tests, I probably should have added more variety in the tests, as well. Just focusing on MemoryUse optimization turned out to cover way less than I thought it would.


http://reviews.llvm.org/D19695