[PATCH] D35901: [DAG] Improve candidate pruning in store merge failure case. NFC.

[Nirav Dave via Phabricator via llvm-commits]

niravd updated this revision to Diff 108336.
niravd edited the summary of this revision.
niravd added a comment.

In addition to alignment, we less restrictive of const zero stores over other constant stores. Add logic to prevent skipping zero cases.


https://reviews.llvm.org/D35901

Files:
  lib/CodeGen/SelectionDAG/DAGCombiner.cpp


Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -12780,18 +12780,22 @@
       unsigned LastLegalVectorType = 1;
       bool LastIntegerTrunc = false;
       bool NonZero = false;
+      unsigned FirstZeroAfterNonZero = NumConsecutiveStores;
       for (unsigned i = 0; i < NumConsecutiveStores; ++i) {
         StoreSDNode *ST = cast<StoreSDNode>(StoreNodes[i].MemNode);
         SDValue StoredVal = ST->getValue();
-
-        if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(StoredVal)) {
-          NonZero |= !C->isNullValue();
-        } else if (ConstantFPSDNode *C =
-                       dyn_cast<ConstantFPSDNode>(StoredVal)) {
-          NonZero |= !C->getConstantFPValue()->isNullValue();
+        bool IsElementZero = false;
+        if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(StoredVal))
+          IsElementZero = C->isNullValue();
+        else if (ConstantFPSDNode *C = dyn_cast<ConstantFPSDNode>(StoredVal))
+          IsElementZero = C->getConstantFPValue()->isNullValue();
+        if (IsElementZero) {
+          if (NonZero && FirstZeroAfterNonZero == NumConsecutiveStores)
+            FirstZeroAfterNonZero = i;
         } else {
-          // Non-constant.
-          break;
+          if (NonZero)
+            FirstZeroAfterNonZero = i;
+          NonZero = true;
         }
 
         // Find a legal type for the constant store.
@@ -12844,7 +12848,21 @@
 
       // Check if we found a legal integer type that creates a meaningful merge.
       if (LastLegalType < 2 && LastLegalVectorType < 2) {
-        StoreNodes.erase(StoreNodes.begin(), StoreNodes.begin() + 1);
+        // We have already checked that candidate stores are in order
+        // and of correct shape. While there is no mergable sequence
+        // from the beggining one may start later in the sequence. The
+        // only reason a merge of size N could have failed where
+        // another of the same size would not have does not is if the
+        // alignment has improved or we've dropped a non-zero
+        // value. Drop as many candidates as we can here.
+        unsigned NumSkip = 1;
+        while (
+            (NumSkip < NumConsecutiveStores) &&
+            (NumSkip < FirstZeroAfterNonZero) &&
+            (StoreNodes[NumSkip].MemNode->getAlignment() <= FirstStoreAlign)) {
+          NumSkip++;
+        }
+        StoreNodes.erase(StoreNodes.begin(), StoreNodes.begin() + NumSkip);
         continue;
       }
 
@@ -12900,6 +12918,23 @@
           NumStoresToMerge = i + 1;
       }
 
+      // Check if we found a legal integer type that creates a meaningful merge.
+      if (NumStoresToMerge < 2) {
+        // We have already checked that candidate stores are in order
+        // and of correct shape. While there is no mergable sequence
+        // from the beggining one may start later in the sequence. The
+        // only reason a merge of size N could have failed where
+        // another of the same size would not have does not is if the
+        // alignment has improved. Drop as many candidates as we can here.
+        unsigned NumSkip = 1;
+        while ((NumSkip < NumConsecutiveStores) &&
+               (StoreNodes[NumSkip].MemNode->getAlignment() <= FirstStoreAlign))
+          NumSkip++;
+
+        StoreNodes.erase(StoreNodes.begin(), StoreNodes.begin() + NumSkip);
+        continue;
+      }
+
       bool Merged = MergeStoresOfConstantsOrVecElts(
           StoreNodes, MemVT, NumStoresToMerge, false, true, false);
       if (!Merged) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D35901.108336.patch
Type: text/x-patch
Size: 3661 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170726/54bd52f3/attachment.bin>