[PATCH] D132657: [DSE] Eliminate noop store even through has clobbering between LoadI and StoreI

[luxufan via Phabricator via llvm-commits]

StephenFan added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1896
+                      CurrentDef->getDefiningAccess(),
+                      MemoryLocation::get(LoadI)));
+                  continue;
----------------
nikic wrote:
> This should probably use the Store location -- the location itself is the same, but they may differ in AAMDNodes.
> 
> Is this code correct in the presence of backedges? We are looking through MemoryPhis above, so we might be looking at a previous loop iteration. This should probably have a loop invariance check on the location.
I think the loop invariance check is not needed. The loop invariance check is needed when we do optimization that depends on AliasAnalysis. In noop-stores detection, aliasing analysis is not needed. Although MemorySSA needs alias analysis, it will do the loop invariance check when it searches for clobbering memory access.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132657/new/

https://reviews.llvm.org/D132657