[PATCH] D137791: [SDAG] bail out of mergeTruncStores() if there's an unknown store in the chain

[Sanjay Patel via Phabricator via llvm-commits]

spatel updated this revision to Diff 476499.
spatel added a comment.

Added a RUN line and checks of debug output to make it clearer where this goes wrong.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137791/new/

https://reviews.llvm.org/D137791

Files:
  llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  llvm/test/CodeGen/SystemZ/merge-stores.ll


Index: llvm/test/CodeGen/SystemZ/merge-stores.ll
===================================================================
--- llvm/test/CodeGen/SystemZ/merge-stores.ll
+++ llvm/test/CodeGen/SystemZ/merge-stores.ll
@@ -11,25 +11,25 @@
 @f = dso_local local_unnamed_addr global ptr @e, align 8
 @d = dso_local local_unnamed_addr global i32 0, align 4
 
-; FIXME: This shows a miscompile caused by merging truncated
+; This may show a miscompile caused by merging truncated
 ; stores if there is a 64-bit store (stg) of the load from '@e'.
 
 ; DAGDEBUG: Combining: {{.*}} store<(store (s32) into %ir.f4
-; DAGDEBUG-NEXT: ... into: {{.*}} store<(store (s64) into %ir.t1
+; DAGDEBUG-NOT: ... into: {{.*}} store<(store (s64) into %ir.t1
 
 define signext i32 @main() {
   ; CHECK-LABEL: name: main
   ; CHECK: bb.0 (%ir-block.0):
   ; CHECK-NEXT:   [[LGRL:%[0-9]+]]:gr64bit = LGRL @e :: (dereferenceable load (s64) from @e)
   ; CHECK-NEXT:   [[SRLG:%[0-9]+]]:gr64bit = SRLG [[LGRL]], $noreg, 32
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gr32bit = COPY [[LGRL]].subreg_l32
   ; CHECK-NEXT:   [[LGRL1:%[0-9]+]]:addr64bit = LGRL @f :: (dereferenceable load (s64) from @f)
-  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gr32bit = COPY [[SRLG]].subreg_l32
-  ; CHECK-NEXT:   ST killed [[COPY]], [[LGRL1]], 0, $noreg :: (store (s32) into %ir.t1)
-  ; CHECK-NEXT:   STG [[LGRL]], [[LGRL1]], 0, $noreg :: (store (s64) into %ir.t1, align 4)
+  ; CHECK-NEXT:   [[COPY1:%[0-9]+]]:gr32bit = COPY [[SRLG]].subreg_l32
+  ; CHECK-NEXT:   ST killed [[COPY1]], [[LGRL1]], 0, $noreg :: (store (s32) into %ir.t1)
+  ; CHECK-NEXT:   ST [[COPY]], [[LGRL1]], 4, $noreg :: (store (s32) into %ir.f4)
   ; CHECK-NEXT:   [[LHI:%[0-9]+]]:gr32bit = LHI 0
   ; CHECK-NEXT:   STHRL killed [[LHI]], @e :: (store (s16) into @e, align 8)
-  ; CHECK-NEXT:   [[COPY1:%[0-9]+]]:gr32bit = COPY [[LGRL]].subreg_l32
-  ; CHECK-NEXT:   STRL killed [[COPY1]], @d :: (store (s32) into @d)
+  ; CHECK-NEXT:   STRL [[COPY]], @d :: (store (s32) into @d)
   ; CHECK-NEXT:   [[LGHI:%[0-9]+]]:gr64bit = LGHI 0
   ; CHECK-NEXT:   $r2d = COPY [[LGHI]]
   ; CHECK-NEXT:   Return implicit $r2d
Index: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -8117,6 +8117,19 @@
         Store->isIndexed())
       return SDValue();
     Stores.push_back(Store);
+
+    // If this store is used as a chain operand in a store that is not included
+    // in the set of stores to be merged, then the transform may not be safe.
+    // We must preserve the order of that other store relative to the indivdual
+    // stores in this set.
+    for (SDNode *Use : Store->uses()) {
+      auto *StoreUse = dyn_cast<StoreSDNode>(Use);
+      if (StoreUse && StoreUse->getChain().getNode() == Store &&
+          !is_contained(Stores, StoreUse))
+        return SDValue();
+    }
+
+    // Get the next link in the chain and try to collect another store.
     Chain = Store->getChain();
   }
   // There is no reason to continue if we do not have at least a pair of stores.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D137791.476499.patch
Type: text/x-patch
Size: 3146 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221118/b84d27f5/attachment.bin>