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

[Matt Arsenault]

arsenm created this revision.
arsenm added a reviewer: hfinkel.
arsenm added a subscriber: llvm-commits.

When combiner AA is enabled, look at stores on the same chain.
Non-aliasing stores are moved to the same chain so the existing
code fails because it expects to find an adajcent store on a consecutive
chain.
    
There are some ordering issues by adding this so the optimal store merge
is not found, and I'm not sure how best to solve them.
    
Because of how DAGCombiner tries these store combines,
MergeConsecutiveStores doesn't see the correct set of stores on the chain
when it visits the other stores. Each store individually has its chain
fixed before trying to merge consecutive stores, and then tries to merge
stores from that point before the other stores have been processed to
have their chains fixed.
    
Suppose you have 4 32-bit stores that should be merged into 1 vector
store. One store is visited first, fixing the chain. What happens is
because not all of the store chains have yet been fixed, 2 of the stores
are merged. The other 2 stores later have their chains fixed,
but because the other stores were already merged, they have different
memory types and merging the two different sized stores is not
supported and would be more difficult to handle.
    
I'm able to get what I want to happen by only performing
MergeConsecutiveStores on the legalized DAG, at which point
all of the nonaliasing stores have had their chains fixed. However,
it is also desirable to have MergeConsecutivStores process stores of illegal
types like it does now, so I'm not sure how to best ensure all stores
have their chains fixed before store merging happens. Should
FindBetterChain search through consecutive chains and try to handle
other consecutive stores at the same time? Should there be some DAG
preprocess step to fix all store chains or something else?


http://reviews.llvm.org/D11166

Files:
  lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -10777,6 +10777,34 @@
   EVT MemVT = St->getMemoryVT();
   unsigned Seq = 0;
   StoreSDNode *Index = St;
+
+
+  bool UseAA = CombinerAA.getNumOccurrences() > 0 ? CombinerAA
+                                                  : DAG.getSubtarget().useAA();
+
+  if (UseAA) {
+    // Look at other users of the same chain. Stores on the same chain do not
+    // alias.
+    // TODO: This should be the common (only?) case if combiner-aa is
+    // enabled. Should we skip or remove the code looking at consecutive chains?
+    SDValue Chain = St->getChain();
+    for (auto I = Chain->use_begin(), E = Chain->use_end(); I != E; ++I) {
+      if (StoreSDNode *OtherST = dyn_cast<StoreSDNode>(*I)) {
+
+        if (OtherST->isVolatile() || OtherST->isIndexed())
+          continue;
+
+        if (OtherST->getMemoryVT() != MemVT)
+          continue;
+
+        BaseIndexOffset Ptr = BaseIndexOffset::match(OtherST->getBasePtr());
+        StoreNodes.push_back(MemOpLink(OtherST, Ptr.Offset, Seq++));
+      }
+    }
+
+    return;
+  }
+
   while (Index) {
     // If the chain has more than one use, then we can't reorder the mem ops.
     if (Index != St && !SDValue(Index, 0)->hasOneUse())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D11166.29624.patch
Type: text/x-patch
Size: 1399 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150713/17e525f1/attachment.bin>