[PATCH] D46600: [MergedLoadStoreMotion] Fix a debug invariant bug in mergeStores

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:303-305
     ++NStores;
     if (NStores * Size1 >= MagicCompileTimeControl)
       break;
----------------
davide wrote:
> Ideally, `MagicCompileTimeControl` shouldn't be a thing, but that's a discussion for another day.
I agree (and yes, I focused on the debug invariance here, since changing the algorithm would take lots more time to implement and test).

One idea, if we think the complexity of finding all store instruction in Pred1 for each store in Pred0 in canSinkFromBlock is a problem, then we can create a list of all store instructions in Pred1 already before starting to iterate over Pred0 (at line 292).  And then iterate over that list in canSinkFromBlock (removing stores that are sunk after each call to canSinkFromBlock).
If we still want to have a (less magic) limit it could be based on the number of store instructions that we allow to sink, rather than the total number of instructions in one of the two predecessors that we sink from.


Repository:
  rL LLVM

https://reviews.llvm.org/D46600