[PATCH] D65174: [DAGCombine] Limit the number of times for a store being considered for merging

Nirav Dave via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 29 08:34:42 PDT 2019


niravd added a comment.

This has come up a few times before (https://reviews.llvm.org/D60133 is the most recent instance). So far it's either been deemed not important enough or there was a way to limit the candidate search to limit the search to the correct size (e.g. checking the in context max valid store size and making sure there was a valid non-trivial store merge).

It would be good to understand why we're finding candidates that cannot be merged and see if the search can be tightened. I vaguely recall that the underlying cause in D60133 <https://reviews.llvm.org/D60133> could be solved by replacing the canMergeStoresTo's method with something expressing maximum mergable size with this store in the current context and that that would also remove a large number of these fruitless merge searches. I suspect this may work for your case as well.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:15461
+    // merging from the same root.
+    if (RootStorePairCountMap[{RootNode, St}] > StoreMergeLimit)
+      return;
----------------
nit: Hoist this and the equal check in the else clause out of the if.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65174





More information about the llvm-commits mailing list