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

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1812
         return None;
+      unsigned StepV = KillingDef->getBlock() == Current->getBlock() ? 1 : 5;
+      if (WalkerStepLimit <= StepV)
----------------
asbirlea wrote:
> Use cl::opts instead of limit numbers in code (e.g. "SameBlockLimit")
Done. Also added a comment on why there are different costs.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1825
+      DomAccess = MSSA.getSkipSelfWalker()->getClobberingMemoryAccess(
+          CurrentUD, DefLoc, ClobberSteps);
       if (MSSA.isLiveOnEntryDef(DomAccess))
----------------
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.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:2260
 
+    unsigned WalkerStepLimit = 50;
     DSEState::CheckCache Cache;
----------------
asbirlea wrote:
> Can you make this a cl::opt?
Turned into another option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86486