[llvm] 0037e21 - [SDAG] bail out of mergeTruncStores() if there's any other use in the chain
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 2 07:09:56 PST 2022
Author: Sanjay Patel
Date: 2022-12-02T10:08:19-05:00
New Revision: 0037e21f28adbeb9f8cac30016521d543561645e
URL: https://github.com/llvm/llvm-project/commit/0037e21f28adbeb9f8cac30016521d543561645e
DIFF: https://github.com/llvm/llvm-project/commit/0037e21f28adbeb9f8cac30016521d543561645e.diff
LOG: [SDAG] bail out of mergeTruncStores() if there's any other use in the chain
This fixes the miscompile in issue #58883.
The test demonstrates that we gave up on store merging in that example.
This change should be strictly safe (just adds another clause
to avoid the transform), and it does not prohibit any existing
valid optimizations based on regression tests. I want to believe
that it's also a sufficient fix (possibly overkill), but I'm not
sure how to prove that.
Differential Revision: https://reviews.llvm.org/D137791
Added:
Modified:
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
llvm/test/CodeGen/SystemZ/merge-stores.ll
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 9970ba20610ba..f6d577562e233 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -8150,9 +8150,13 @@ SDValue DAGCombiner::mergeTruncStores(StoreSDNode *N) {
while (auto *Store = dyn_cast<StoreSDNode>(Chain)) {
// All stores must be the same size to ensure that we are writing all of the
// bytes in the wide value.
+ // This store should have exactly one use as a chain operand for another
+ // store in the merging set. If there are other chain uses, then the
+ // transform may not be safe because order of loads/stores outside of this
+ // set may not be preserved.
// TODO: We could allow multiple sizes by tracking each stored byte.
if (Store->getMemoryVT() != MemVT || !Store->isSimple() ||
- Store->isIndexed())
+ Store->isIndexed() || !Store->hasOneUse())
return SDValue();
Stores.push_back(Store);
Chain = Store->getChain();
diff --git a/llvm/test/CodeGen/SystemZ/merge-stores.ll b/llvm/test/CodeGen/SystemZ/merge-stores.ll
index 74732583dd5f1..f09d95cb1f53a 100644
--- a/llvm/test/CodeGen/SystemZ/merge-stores.ll
+++ b/llvm/test/CodeGen/SystemZ/merge-stores.ll
@@ -9,9 +9,10 @@ target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64"
@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
-; stores if the store of 0 (sthrl) to 'e' happens before
-; a 64-bit store (stg) of r0.
+; This shows a miscompile caused by merging truncated
+; stores if the store of 0 (sthrl) to 'e' happens before
+; a 64-bit store (stg) of r0. The store of r0 can follow
+; the store to 'e' only if it is a 32-bit store (st).
define signext i32 @main() {
; CHECK-LABEL: main:
@@ -22,7 +23,7 @@ define signext i32 @main() {
; CHECK-NEXT: st %r2, 0(%r1)
; CHECK-NEXT: lhi %r2, 0
; CHECK-NEXT: sthrl %r2, e
-; CHECK-NEXT: stg %r0, 0(%r1)
+; CHECK-NEXT: st %r0, 4(%r1)
; CHECK-NEXT: lghi %r2, 0
; CHECK-NEXT: strl %r0, d
; CHECK-NEXT: br %r14
More information about the llvm-commits
mailing list