[PATCH] D82588: [DSE] Look through memory PHI arguments when removing noop stores in MSSA.

[Zoe Carver via Phabricator via llvm-commits]

zoecarver marked 3 inline comments as done.
zoecarver added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1869
+      // If the store is not dead, we need to keep checking.
+      continue;
+    }
----------------
fhahn wrote:
> I think we could also cover more cases here, by skipping no-aliasing MemoryDefs and checking that there are no aliasing reads.. But that's future work. Perhaps a comment/TODO would be good.
Added a TODO. 


================
Comment at: llvm/test/Transforms/DeadStoreElimination/MSSA/simple.ll:294
-; CHECK-NEXT:    [[V:%.*]] = load i32, i32* [[P:%.*]], align 4
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[BB1:%.*]], label [[BB2:%.*]]
 ; CHECK:       bb1:
----------------
fhahn wrote:
> The changes in this test file are not legal transforms I think. In this example, we cannot remove the start as the store at line 311 might overwrite %p, if %p == %p2 and then the store is not dead.
You're right. We shouldn't exit early when we hit a `MemoryDef`. Fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82588