[PATCH] D132365: [DSE] Support looking through memory phis at end of function.

[Florian Hahn via Phabricator via llvm-commits]

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


================
Comment at: llvm/test/Transforms/DeadStoreElimination/multiblock-loop-carried-dependence.ll:9
 
 ; Test cases with a loop carried dependence in %loop.2, where %l.2 reads the
 ; value stored by the previous iteration. Hence, the store in %loop.2 is not
----------------
nikic wrote:
> Comment is confusing because there is no `%l.2`.
thanks, the comment should be updated in 197332a1f80db7cbe2bb43d3f5bd45282245dd28


================
Comment at: llvm/test/Transforms/DeadStoreElimination/multiblock-loop-carried-dependence.ll:34
-; CHECK-NEXT:    [[PTR_IV_2_ADD_1:%.*]] = getelementptr inbounds [100 x i32], [100 x i32]* [[A]], i64 0, i64 [[ADD]]
-; CHECK-NEXT:    store i32 10, i32* [[PTR_IV_2_ADD_1]], align 4
 ; CHECK-NEXT:    [[L_1:%.*]] = load i32, i32* [[PTR_IV_2]], align 4
----------------
nikic wrote:
> Isn't this exactly the miscompile we want to prevent? The store is to `A[iv.2 + 1]`, which will be read as `A[iv.2]` on the next iteration?
Yeah I missed that in the original version! Updated to use `isGuaranteedLoopInvariant`


================
Comment at: llvm/test/Transforms/DeadStoreElimination/phi-translation.ll:159
 
 ; TODO: The store in %entry can be removed by translating %p through the phi.
 define void @memoryphi_translate_5(i1 %cond) {
----------------
nikic wrote:
> Drop TODO
Thanks, should be removed in the latest version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132365