[PATCH] D19821: [EarlyCSE] Optionally use MemorySSA. NFC.

[Geoff Berry via llvm-commits]

gberry added inline comments.

================
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:537
@@ +536,3 @@
+  MemoryAccess *LaterDef =
+      MSSA->getWalker()->getClobberingMemoryAccess(LaterInst);
+  return MSSA->dominates(LaterDef, MSSA->getMemoryAccess(EarlierInst));
----------------
dberlin wrote:
> 1. For loads, you don't have to ask for the clobbering access. It's already optimized such that getDefiningAccess == the clobbering access
> 
> 2. For stores, not sure if you realize this, but 
> 
> given
> store q  (lets's call this a)
> x = load p
> store q (let's call this b)
> 
> 
> if you call getClobberingMemoryAccess on b, it will return a.
> 
> 
For 1., I was not clear on whether this holds true after store removal.

For 2., yeah I get this, I'm not sure what you're getting at though.  The removal of this second store by EarlyCSE doesn't use MemorySSA to check for intervening loads in this change.  It uses the 'LastStore' tracking to know when a store made redundant by a second store can be removed.


https://reviews.llvm.org/D19821