From fb9d30dd648c04fba2a1eb0c4366924e14eecd46 Mon Sep 17 00:00:00 2001 From: Duncan Sands Date: Sat, 29 Sep 2012 10:25:35 +0000 Subject: [PATCH] Speculatively revert commit 164885 (nadav) in the hope of ressurecting a pile of buildbots. Original commit message: A DAGCombine optimization for merging consecutive stores. This optimization is not profitable in many cases because moden processos can store multiple values in parallel, and preparing the consecutive store requires some work. We only handle these cases: 1. Consecutive stores where the values and consecutive loads. For example: int a = p->a; int b = p->b; q->a = a; q->b = b; 2. Consecutive stores where the values are constants. Foe example: q->a = 4; q->b = 5; llvm-svn: 164890 --- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 252 ------------------------ llvm/test/CodeGen/X86/MergeConsecutiveStores.ll | 150 -------------- llvm/test/CodeGen/X86/loop-strength-reduce-2.ll | 10 +- llvm/test/CodeGen/X86/loop-strength-reduce-3.ll | 5 +- llvm/test/CodeGen/X86/loop-strength-reduce.ll | 5 +- 5 files changed, 12 insertions(+), 410 deletions(-) delete mode 100644 llvm/test/CodeGen/X86/MergeConsecutiveStores.ll diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 4b815a8..da3293b 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -301,10 +301,6 @@ namespace { /// looking for a better chain (aliasing node.) SDValue FindBetterChain(SDNode *N, SDValue Chain); - /// Merge consecutive store operations into a wide store. - /// \return True if some memory operations were changed. - bool MergeConsecutiveStores(StoreSDNode *N); - public: DAGCombiner(SelectionDAG &D, AliasAnalysis &A, CodeGenOpt::Level OL) : DAG(D), TLI(D.getTargetLoweringInfo()), Level(BeforeLegalizeTypes), @@ -7427,248 +7423,6 @@ SDValue DAGCombiner::TransformFPLoadStorePair(SDNode *N) { return SDValue(); } -/// Returns the base pointer and an integer offset from that object. -static std::pair GetPointerBaseAndOffset(SDValue Ptr) { - if (Ptr->getOpcode() == ISD::ADD && isa(Ptr->getOperand(1))) { - int64_t Offset = cast(Ptr->getOperand(1))->getSExtValue(); - SDValue Base = Ptr->getOperand(0); - return std::make_pair(Base, Offset); - } - - return std::make_pair(Ptr, 0); -} - -struct ConsecutiveMemoryChainSorter { - typedef std::pair MemLink; - bool operator()(MemLink LHS, MemLink RHS) { - return LHS.second < RHS.second; - } -}; - -bool DAGCombiner::MergeConsecutiveStores(StoreSDNode* St) { - EVT MemVT = St->getMemoryVT(); - int64_t ElementSizeBytes = MemVT.getSizeInBits()/8; - - // Don't handle vectors. - if (MemVT.isVector() || !MemVT.isSimple()) - return false; - - // Perform an early exit check. Do not bother looking at stored values that - // are not constants or loads. - SDValue StoredVal = St->getValue(); - if (!isa(StoredVal) && !isa(StoredVal) && - !isa(StoredVal)) - return false; - - // Is this a load-to-store or a const-store. - bool IsLoadSrc = isa(StoredVal); - - // Only look at ends of store chains. - SDValue Chain = SDValue(St, 1); - if (Chain->hasOneUse() && Chain->use_begin()->getOpcode() == ISD::STORE) - return false; - - // This holds the base pointer and the offset in bytes from the base pointer. - std::pair BasePtr = - GetPointerBaseAndOffset(St->getBasePtr()); - - // We must have a base and an offset. - if (!BasePtr.first.getNode()) - return false; - - // Do not handle stores to undef base pointers. - if (BasePtr.first.getOpcode() == ISD::UNDEF) - return false; - - SmallVector, 8> StoreNodes; - // Walk up the chain and look for nodes with offsets from the same - // base pointer. Stop when reaching an instruction with a different kind - // or instruction which has a different base pointer. - StoreSDNode *Index = St; - while (Index) { - // If the chain has more than one use, then we can't reorder the mem ops. - if (Index != St && !SDValue(Index, 1)->hasOneUse()) - break; - - // Find the base pointer and offset for this memory node. - std::pair Ptr = - GetPointerBaseAndOffset(Index->getBasePtr()); - - // Check that the base pointer is the same as the original one. - if (Ptr.first.getNode() != BasePtr.first.getNode()) - break; - - // Check that the alignment is the same. - if (Index->getAlignment() != St->getAlignment()) - break; - - // The memory operands must not be volatile. - if (Index->isVolatile() || Index->isIndexed()) - break; - - // No truncation. - if (StoreSDNode *St = dyn_cast(Index)) - if (St->isTruncatingStore()) - break; - - // The stored memory type must be the same. - if (Index->getMemoryVT() != MemVT) - break; - - // We found a potential memory operand to merge. - StoreNodes.push_back(std::make_pair(Index,Ptr.second)); - - // Move up the chain to the next memory operation. - Index = dyn_cast(Index->getChain().getNode()); - } - - // Check if there is anything to merge. - if (StoreNodes.size() < 2) - return false; - - // Remember which node is the earliest node in the chain. - LSBaseSDNode *EarliestOp = StoreNodes.back().first; - - // Sort the memory operands according to their distance from the base pointer. - std::sort(StoreNodes.begin(), StoreNodes.end(), - ConsecutiveMemoryChainSorter()); - - // Scan the memory operations on the chain and find the first non-consecutive - // store memory address. - unsigned LastConsecutiveStore = 0; - int64_t StartAddress = StoreNodes[0].second; - for (unsigned i=1; igetValue(); - bool IsConst = (isa(StoredVal) || isa(StoredVal)); - if (!IsConst) - break; - LastConst = i; - } - unsigned NumElem = std::min(LastConsecutiveStore + 1, LastConst + 1); - if (NumElem < 2) - return false; - - EVT JointMemOpVT = EVT::getVectorVT(*DAG.getContext(), MemVT, NumElem); - DebugLoc DL = StoreNodes[0].first->getDebugLoc(); - SmallVector Ops; - - for (unsigned i = 0; i < NumElem ; ++i) { - StoreSDNode *St = cast(StoreNodes[i].first); - Ops.push_back(St->getValue()); - } - - SDValue BV = DAG.getNode(ISD::BUILD_VECTOR, DL, - JointMemOpVT, &Ops[0], Ops.size()); - - SDValue NewStore = DAG.getStore(EarliestOp->getChain(), DL, BV, - EarliestOp->getBasePtr(), - EarliestOp->getPointerInfo(), false, false, - EarliestOp->getAlignment()); - - for (unsigned i = 0; i < NumElem ; ++i) { - StoreSDNode *St = cast(StoreNodes[i].first); - CombineTo(St, NewStore); - } - return true; - } - - // Look for load nodes wich are used by the stored values. - SmallVector, 8> LoadNodes; - - // Find acceptible loads. Loads need to have the same chain (token factor), - // must not be zext, volatile, indexed, and they must be consecutive. - SDValue LdBasePtr; - for (unsigned i=0; i(StoreNodes[i].first->getValue()); - if (!Ld) break; - - // Loads must only have one use. - if (!Ld->hasNUsesOfValue(1, 0)) - break; - - // Check that the alignment is the same as the stores. - if (Ld->getAlignment() != St->getAlignment()) - break; - - // The memory operands must not be volatile. - if (Ld->isVolatile() || Ld->isIndexed()) - break; - - if (Ld->getExtensionType() != ISD::NON_EXTLOAD) - break; - - // The stored memory type must be the same. - if (Ld->getMemoryVT() != MemVT) - break; - - std::pair LdPtr = - GetPointerBaseAndOffset(Ld->getBasePtr()); - - // If this is not the first ptr that we check. - if (LdBasePtr.getNode()) { - // The base ptr must be the same, - if (LdPtr.first != LdBasePtr) - break; - } else { - LdBasePtr = LdPtr.first; - } - - // We found a potential memory operand to merge. - LoadNodes.push_back(std::make_pair(Ld, LdPtr.second)); - } - - if (LoadNodes.size() < 2) - return false; - - // Scan the memory operations on the chain and find the first non-consecutive - // load memory address. - unsigned LastConsecutiveLoad = 0; - StartAddress = LoadNodes[0].second; - for (unsigned i=1; igetDebugLoc(); - DebugLoc StoreDL = StoreNodes[0].first->getDebugLoc(); - - LoadSDNode *FirstLoad = LoadNodes[0].first; - SDValue NewLoad = DAG.getLoad(JointMemOpVT, LoadDL, - FirstLoad->getChain(), - FirstLoad->getBasePtr(), - FirstLoad->getPointerInfo(), - false, false, false, - FirstLoad->getAlignment()); - - SDValue NewStore = DAG.getStore(EarliestOp->getChain(), StoreDL, NewLoad, - EarliestOp->getBasePtr(), - EarliestOp->getPointerInfo(), false, false, - EarliestOp->getAlignment()); - - for (unsigned i = 0; i < NumElem ; ++i) { - StoreSDNode *St = cast(StoreNodes[i].first); - CombineTo(St, NewStore); - } - - return true; -} - SDValue DAGCombiner::visitSTORE(SDNode *N) { StoreSDNode *ST = cast(N); SDValue Chain = ST->getChain(); @@ -7870,12 +7624,6 @@ SDValue DAGCombiner::visitSTORE(SDNode *N) { ST->getAlignment()); } - - // Only perform this optimization before the types are legal, because we - // don't want to generate illegal types in this optimization. - if (!LegalTypes && MergeConsecutiveStores(ST)) - return SDValue(N, 0); - return ReduceLoadOpStoreWidth(N); } diff --git a/llvm/test/CodeGen/X86/MergeConsecutiveStores.ll b/llvm/test/CodeGen/X86/MergeConsecutiveStores.ll deleted file mode 100644 index 435f38c..0000000 --- a/llvm/test/CodeGen/X86/MergeConsecutiveStores.ll +++ /dev/null @@ -1,150 +0,0 @@ -; RUN: llc -march=x86-64 -mcpu=corei7 < %s | FileCheck %s - -target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128" -target triple = "x86_64-apple-macosx10.8.0" - -%struct.A = type { i8, i8, i8, i8, i8, i8, i8, i8 } - -@a = common global [10000 x %struct.A] zeroinitializer, align 8 - -; Move all of the constants using a single vector store. -; CHECK: merge_const_store -; CHECK: movq %xmm0 -; CHECK: ret -define void @merge_const_store(i32 %count, %struct.A* nocapture %p) nounwind uwtable noinline ssp { - %1 = icmp sgt i32 %count, 0 - br i1 %1, label %.lr.ph, label %._crit_edge -.lr.ph: - %i.02 = phi i32 [ %10, %.lr.ph ], [ 0, %0 ] - %.01 = phi %struct.A* [ %11, %.lr.ph ], [ %p, %0 ] - %2 = getelementptr inbounds %struct.A* %.01, i64 0, i32 0 - store i8 1, i8* %2, align 1 - %3 = getelementptr inbounds %struct.A* %.01, i64 0, i32 1 - store i8 2, i8* %3, align 1 - %4 = getelementptr inbounds %struct.A* %.01, i64 0, i32 2 - store i8 3, i8* %4, align 1 - %5 = getelementptr inbounds %struct.A* %.01, i64 0, i32 3 - store i8 4, i8* %5, align 1 - %6 = getelementptr inbounds %struct.A* %.01, i64 0, i32 4 - store i8 5, i8* %6, align 1 - %7 = getelementptr inbounds %struct.A* %.01, i64 0, i32 5 - store i8 6, i8* %7, align 1 - %8 = getelementptr inbounds %struct.A* %.01, i64 0, i32 6 - store i8 7, i8* %8, align 1 - %9 = getelementptr inbounds %struct.A* %.01, i64 0, i32 7 - store i8 8, i8* %9, align 1 - %10 = add nsw i32 %i.02, 1 - %11 = getelementptr inbounds %struct.A* %.01, i64 1 - %exitcond = icmp eq i32 %10, %count - br i1 %exitcond, label %._crit_edge, label %.lr.ph -._crit_edge: - ret void -} - -; Move the first 4 constants as a single vector. Move the rest as scalars. -; CHECK: merge_nonconst_store -; CHECK: movd %xmm0 -; CHECK: movb -; CHECK: movb -; CHECK: movb -; CHECK: movb -; CHECK: ret -define void @merge_nonconst_store(i32 %count, i8 %zz, %struct.A* nocapture %p) nounwind uwtable noinline ssp { - %1 = icmp sgt i32 %count, 0 - br i1 %1, label %.lr.ph, label %._crit_edge -.lr.ph: - %i.02 = phi i32 [ %10, %.lr.ph ], [ 0, %0 ] - %.01 = phi %struct.A* [ %11, %.lr.ph ], [ %p, %0 ] - %2 = getelementptr inbounds %struct.A* %.01, i64 0, i32 0 - store i8 1, i8* %2, align 1 - %3 = getelementptr inbounds %struct.A* %.01, i64 0, i32 1 - store i8 2, i8* %3, align 1 - %4 = getelementptr inbounds %struct.A* %.01, i64 0, i32 2 - store i8 3, i8* %4, align 1 - %5 = getelementptr inbounds %struct.A* %.01, i64 0, i32 3 - store i8 4, i8* %5, align 1 - %6 = getelementptr inbounds %struct.A* %.01, i64 0, i32 4 - store i8 %zz, i8* %6, align 1 ; <----------- Not a const; - %7 = getelementptr inbounds %struct.A* %.01, i64 0, i32 5 - store i8 6, i8* %7, align 1 - %8 = getelementptr inbounds %struct.A* %.01, i64 0, i32 6 - store i8 7, i8* %8, align 1 - %9 = getelementptr inbounds %struct.A* %.01, i64 0, i32 7 - store i8 8, i8* %9, align 1 - %10 = add nsw i32 %i.02, 1 - %11 = getelementptr inbounds %struct.A* %.01, i64 1 - %exitcond = icmp eq i32 %10, %count - br i1 %exitcond, label %._crit_edge, label %.lr.ph -._crit_edge: - ret void -} - - -;CHECK: merge_loads -; load: -;CHECK: movw -; store: -;CHECK: movw -;CHECK: ret -define void @merge_loads(i32 %count, %struct.A* noalias nocapture %q, %struct.A* noalias nocapture %p) nounwind uwtable noinline ssp { - %1 = icmp sgt i32 %count, 0 - br i1 %1, label %.lr.ph, label %._crit_edge - -.lr.ph: ; preds = %0 - %2 = getelementptr inbounds %struct.A* %q, i64 0, i32 0 - %3 = getelementptr inbounds %struct.A* %q, i64 0, i32 1 - br label %4 - -;