[PATCH] D11166: WIP: Make MergeConsecutiveStores look at other stores on same chain

[Matt Arsenault]

arsenm added a comment.

In http://reviews.llvm.org/D11166#212218, @hfinkel wrote:

> Looking for stores sharing the same chain for merging seems like the right thing to do. This is what you should get from memcpy legalization, etc. anyway.
>
> Shouldn't, however, this be part of the visitor loop below that walks the chain instead of only searching the top (or bottom, depending on how you look at it) store?


The idea was if we can rely on FindBetterChain canonicalizing the chain layout, we wouldn't have to worry about stores on other chains and could delete the existing loop here. I'll post another rough version in a minute which more or less copies the loop in MergeConsecutiveStores and changes it so that visitSTORE tries to FindBetterChain on possible store merging candidates, which should make MergeConsecutiveStores only need to consider other users of the same chain. Since this only happens if combiner-alias-analysis is enabled, my questions are why isn't it on by default? Would it be OK to say that if you want consecutive store merging combiner AA should be enabled, or should I try to keep these supporting both?

I would prefer to canonicalize how non-aliasing load/store chains are arranged, rather than having MergeConsecutiveStores do it because I would like to write target DAG combines for more exotic load/store merging. For example I would like to merge certain non-adjacent loads and certain cases of load one address space and store to another.


http://reviews.llvm.org/D11166