From 1fc026af4bd967c33115fa64d8e9af76e84e69f5 Mon Sep 17 00:00:00 2001 From: Tom Stellard Date: Tue, 8 Apr 2014 21:47:12 +0000 Subject: Merging r200201: ------------------------------------------------------------------------ r200201 | stpworld | 2014-01-27 04:18:31 -0500 (Mon, 27 Jan 2014) | 31 lines Fix for PR18102. Issue outcomes from DAGCombiner::MergeConsequtiveStores, more precisely from mem-ops sequence sorting. Consider, how MergeConsequtiveStores works for next example: store i8 1, a[0] store i8 2, a[1] store i8 3, a[1] ; a[1] again. return ; DAG starts here 1. Method will collect all the 3 stores. 2. It sorts them by distance from the base pointer (farthest with highest index). 3. It takes first consecutive non-overlapping stores and (if possible) replaces them with a single store instruction. The point is, we can't determine here which 'store' instruction would be the second after sorting ('store 2' or 'store 3'). It happens that 'store 3' would be the second, and 'store 2' would be the third. So after merging we have the next result: store i16 (1 | 3 << 8), base ; is a[0] but bit-casted to i16 store i8 2, a[1] So actually we swapped 'store 3' and 'store 2' and got wrong contents in a[1]. Fix: In sort routine just also take into account mem-op sequence number. ------------------------------------------------------------------------ git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_34@205806 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 5 ++++- test/CodeGen/Generic/stores-merging.ll | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 test/CodeGen/Generic/stores-merging.ll diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 43f72c5ef9..69cf8d9a90 100644 --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -8547,7 +8547,10 @@ struct MemOpLink { // base ptr. struct ConsecutiveMemoryChainSorter { bool operator()(MemOpLink LHS, MemOpLink RHS) { - return LHS.OffsetFromBase < RHS.OffsetFromBase; + return + LHS.OffsetFromBase < RHS.OffsetFromBase || + (LHS.OffsetFromBase == RHS.OffsetFromBase && + LHS.SequenceNum > RHS.SequenceNum); } }; diff --git a/test/CodeGen/Generic/stores-merging.ll b/test/CodeGen/Generic/stores-merging.ll new file mode 100644 index 0000000000..61dea08899 --- /dev/null +++ b/test/CodeGen/Generic/stores-merging.ll @@ -0,0 +1,23 @@ +; RUN: llc < %s | FileCheck %s + +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +%structTy = type { i8, i32, i32 } + +@e = common global %structTy zeroinitializer, align 4 + +; CHECK-LABEL: f +define void @f() { +entry: + +; CHECK: movabsq $528280977409, %rax +; CHECK: movq %rax, e+4(%rip) +; CHECK: movl $456, e+8(%rip) + + store i32 1, i32* getelementptr inbounds (%structTy* @e, i64 0, i32 1), align 4 + store i32 123, i32* getelementptr inbounds (%structTy* @e, i64 0, i32 2), align 4 + store i32 456, i32* getelementptr inbounds (%structTy* @e, i64 0, i32 2), align 4 + ret void +} + -- cgit v1.2.3