For large blocks of code, FindBetterChain will give up leaving memory operations in their initial configuration. A common idiom for this results in a long chain of stores which are generally mergable. Augment the search to consider these stores as merge candidates.
Details
Diff Detail
- Repository
- rL LLVM
- Build Status
Buildable 23886 Build 23885: arc lint + arc unit
Event Timeline
Hi Nirav
I tried reproducing it with the patch and the issue is still not fixed. I'll try to get back with a repro case (for open source targets).
Nirav - I posted the reason for the regression and a quick fix for the issue I saw in https://reviews.llvm.org/D31914.
I'm a little concerned about the selection candidates having a chain loop dependence after mething. Can you get a llvm testcase I can look at myself. Alternatively I can probably cobble one myself if I can get a snapshot of the DAG right before the relevant merge
lib/CodeGen/SelectionDAG/DAGCombiner.cpp | ||
---|---|---|
12413 ↗ | (On Diff #97480) | Please rebase, you'll have to go through CandidateMatch's Offset parameter now. |
test/CodeGen/X86/stores-merging2.ll | ||
16 ↗ | (On Diff #97480) | This test fails to fail before this change when rebased on master. I guess this is because the value of GatherAllAliasesMaxDepth has been changed from 6 to 18. The test quite brittle against that kind of config changes BTW. You should at least leave a comment for the future explaining how to fix the test if changing the value. |
Rebase to head. Some changes to alias make this require a much longer chain of stores. The need is clearly less than when I wrote this patch a year ago, but still has a minor positive effect.
It's certainly less likely than before, but it does happen in the wild, but D53552 should always parallelize these store chains, so this this should be droppable.