[llvm] r234430 - [DAGCombine] Fix a bug in MergeConsecutiveStores.
Akira Hatanaka
ahatanaka at apple.com
Wed Apr 8 13:34:53 PDT 2015
Author: ahatanak
Date: Wed Apr 8 15:34:53 2015
New Revision: 234430
URL: http://llvm.org/viewvc/llvm-project?rev=234430&view=rev
Log:
[DAGCombine] Fix a bug in MergeConsecutiveStores.
The bug manifests when there are two loads and two stores chained as follows in
a DAG,
(ld v3f32) -> (st f32) -> (ld v3f32) -> (st f32)
and the stores' values are extracted from the preceding vector loads.
MergeConsecutiveStores would replace the first store in the chain with the
merged vector store, which would create a cycle between the merged store node
and the last load node that appears in the chain.
This commits fixes the bug by replacing the last store in the chain instead.
rdar://problem/20275084
Differential Revision: http://reviews.llvm.org/D8849
Added:
llvm/trunk/test/CodeGen/AArch64/merge-store.ll
Modified:
llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
llvm/trunk/test/CodeGen/X86/2012-11-28-merge-store-alias.ll
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=234430&r1=234429&r2=234430&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Wed Apr 8 15:34:53 2015
@@ -10055,19 +10055,19 @@ bool DAGCombiner::MergeStoresOfConstants
int64_t ElementSizeBytes = MemVT.getSizeInBits() / 8;
LSBaseSDNode *FirstInChain = StoreNodes[0].MemNode;
- unsigned EarliestNodeUsed = 0;
+ unsigned LatestNodeUsed = 0;
for (unsigned i=0; i < NumElem; ++i) {
// Find a chain for the new wide-store operand. Notice that some
// of the store nodes that we found may not be selected for inclusion
// in the wide store. The chain we use needs to be the chain of the
- // earliest store node which is *used* and replaced by the wide store.
- if (StoreNodes[i].SequenceNum > StoreNodes[EarliestNodeUsed].SequenceNum)
- EarliestNodeUsed = i;
+ // latest store node which is *used* and replaced by the wide store.
+ if (StoreNodes[i].SequenceNum < StoreNodes[LatestNodeUsed].SequenceNum)
+ LatestNodeUsed = i;
}
- // The earliest Node in the DAG.
- LSBaseSDNode *EarliestOp = StoreNodes[EarliestNodeUsed].MemNode;
+ // The latest Node in the DAG.
+ LSBaseSDNode *LatestOp = StoreNodes[LatestNodeUsed].MemNode;
SDLoc DL(StoreNodes[0].MemNode);
SDValue StoredVal;
@@ -10126,17 +10126,17 @@ bool DAGCombiner::MergeStoresOfConstants
StoredVal = DAG.getConstant(StoreInt, StoreTy);
}
- SDValue NewStore = DAG.getStore(EarliestOp->getChain(), DL, StoredVal,
+ SDValue NewStore = DAG.getStore(LatestOp->getChain(), DL, StoredVal,
FirstInChain->getBasePtr(),
FirstInChain->getPointerInfo(),
false, false,
FirstInChain->getAlignment());
- // Replace the first store with the new store
- CombineTo(EarliestOp, NewStore);
+ // Replace the last store with the new store
+ CombineTo(LatestOp, NewStore);
// Erase all other stores.
for (unsigned i = 0; i < NumElem ; ++i) {
- if (StoreNodes[i].MemNode == EarliestOp)
+ if (StoreNodes[i].MemNode == LatestOp)
continue;
StoreSDNode *St = cast<StoreSDNode>(StoreNodes[i].MemNode);
// ReplaceAllUsesWith will replace all uses that existed when it was
@@ -10513,18 +10513,19 @@ bool DAGCombiner::MergeConsecutiveStores
if (NumElem < 2)
return false;
- // The earliest Node in the DAG.
- unsigned EarliestNodeUsed = 0;
- LSBaseSDNode *EarliestOp = StoreNodes[EarliestNodeUsed].MemNode;
+ // The latest Node in the DAG.
+ unsigned LatestNodeUsed = 0;
for (unsigned i=1; i<NumElem; ++i) {
// Find a chain for the new wide-store operand. Notice that some
// of the store nodes that we found may not be selected for inclusion
// in the wide store. The chain we use needs to be the chain of the
- // earliest store node which is *used* and replaced by the wide store.
- if (StoreNodes[i].SequenceNum > StoreNodes[EarliestNodeUsed].SequenceNum)
- EarliestNodeUsed = i;
+ // latest store node which is *used* and replaced by the wide store.
+ if (StoreNodes[i].SequenceNum < StoreNodes[LatestNodeUsed].SequenceNum)
+ LatestNodeUsed = i;
}
+ LSBaseSDNode *LatestOp = StoreNodes[LatestNodeUsed].MemNode;
+
// Find if it is better to use vectors or integers to load and store
// to memory.
EVT JointMemOpVT;
@@ -10546,7 +10547,7 @@ bool DAGCombiner::MergeConsecutiveStores
false, false, false,
FirstLoad->getAlignment());
- SDValue NewStore = DAG.getStore(EarliestOp->getChain(), StoreDL, NewLoad,
+ SDValue NewStore = DAG.getStore(LatestOp->getChain(), StoreDL, NewLoad,
FirstInChain->getBasePtr(),
FirstInChain->getPointerInfo(), false, false,
FirstInChain->getAlignment());
@@ -10564,12 +10565,12 @@ bool DAGCombiner::MergeConsecutiveStores
DAG.ReplaceAllUsesOfValueWith(SDValue(Ld, 1), Ld->getChain());
}
- // Replace the first store with the new store.
- CombineTo(EarliestOp, NewStore);
+ // Replace the last store with the new store.
+ CombineTo(LatestOp, NewStore);
// Erase all other stores.
for (unsigned i = 0; i < NumElem ; ++i) {
// Remove all Store nodes.
- if (StoreNodes[i].MemNode == EarliestOp)
+ if (StoreNodes[i].MemNode == LatestOp)
continue;
StoreSDNode *St = cast<StoreSDNode>(StoreNodes[i].MemNode);
DAG.ReplaceAllUsesOfValueWith(SDValue(St, 0), St->getChain());
Added: llvm/trunk/test/CodeGen/AArch64/merge-store.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/merge-store.ll?rev=234430&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/merge-store.ll (added)
+++ llvm/trunk/test/CodeGen/AArch64/merge-store.ll Wed Apr 8 15:34:53 2015
@@ -0,0 +1,20 @@
+; RUN: llc -march=aarch64 %s -o - | FileCheck %s
+
+ at g0 = external global <3 x float>, align 16
+ at g1 = external global <3 x float>, align 4
+
+; CHECK: ldr s[[R0:[0-9]+]], {{\[}}[[R1:x[0-9]+]]{{\]}}, #4
+; CHECK: ld1.s { v[[R0]] }[1], {{\[}}[[R1]]{{\]}}
+; CHECK: str d[[R0]]
+
+define void @blam() {
+ %tmp4 = getelementptr inbounds <3 x float>, <3 x float>* @g1, i64 0, i64 0
+ %tmp5 = load <3 x float>, <3 x float>* @g0, align 16
+ %tmp6 = extractelement <3 x float> %tmp5, i64 0
+ store float %tmp6, float* %tmp4
+ %tmp7 = getelementptr inbounds float, float* %tmp4, i64 1
+ %tmp8 = load <3 x float>, <3 x float>* @g0, align 16
+ %tmp9 = extractelement <3 x float> %tmp8, i64 1
+ store float %tmp9, float* %tmp7
+ ret void;
+}
Modified: llvm/trunk/test/CodeGen/X86/2012-11-28-merge-store-alias.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/2012-11-28-merge-store-alias.ll?rev=234430&r1=234429&r2=234430&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/2012-11-28-merge-store-alias.ll (original)
+++ llvm/trunk/test/CodeGen/X86/2012-11-28-merge-store-alias.ll Wed Apr 8 15:34:53 2015
@@ -2,7 +2,7 @@
; CHECK: merge_stores_can
; CHECK: callq foo
-; CHECK-NEXT: xorps %xmm0, %xmm0
+; CHECK: xorps %xmm0, %xmm0
; CHECK-NEXT: movups %xmm0
; CHECK: callq foo
; CHECK: ret
More information about the llvm-commits
mailing list