[PATCH] D86486: [DSE,MemorySSA] Limit walker steps.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1825
+      DomAccess = MSSA.getSkipSelfWalker()->getClobberingMemoryAccess(
+          CurrentUD, DefLoc, ClobberSteps);
       if (MSSA.isLiveOnEntryDef(DomAccess))
----------------
fhahn wrote:
> asbirlea wrote:
> > use: /*WalkerUpwardsLimit=*/0
> > remove ClobberSteps.
> > 
> > Why use 0?
> The argument is taken by reference, and the walker decrements it as it goes along I think. I meant to use 1 as limit (single step), added a comment as to why. I think the behavior is actually the same whether 0 or 1 is passed, as I think if it is 0 initially it gets bumped to 1 internally.
Yes, I missed it needs a reference there.

You're right that 0 and 1 mean the same, I should have phrased the question clearer with what I meant.
If the cap you set is 0, this isn't getting more information than `getDefiningAccess`. The `getClobberingAccess` call should never reach the `tryOptimizePhi` call for the 0/1 caps.
If the intention is to deal with phi translation (as the comment says), you need a cap of 2+, and it will be dependent on the number of defs per block if it actually reaches the Phi.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86486