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

[Matt Arsenault via llvm-commits]

arsenm added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11400
@@ +11399,3 @@
+    // adjacent stores.
+    if (findBetterNeighborChains(ST)) {
+      // replaceStoreChain uses CombineTo, which handled all of the worklist
----------------
hfinkel wrote:
> Could we be consistent and do this even when UseAA is false? This code ends up calling isAlias, but isAlias does things when UseAA is false (it just does more things when UseAA is true). If we did that, could be remove the special case for UseAA in getStoreMergeAndAliasCandidates (because we'd again have a single canonical form for the store chains).
> 
That just exposes more instances of the same fundamental problem, that different combines currently expect different chain layouts. Ideally we could probably do this, however that will require more work. Doing this without AA will move more chains, but most of the code is not ready for this (which is why I assume CombinerAA is not on by default now).

If you remove the UseAA check here (and in current code), you many of the same test failures as you do if you enable combiner-aa by default. Eventually when more combines are fixed to look at the same chain, we could probably do this.


http://reviews.llvm.org/D11166