From bbd97d273b12f1c7624ad1e2dd456f1ff6252d14 Mon Sep 17 00:00:00 2001 From: "David L. Jones" Date: Wed, 10 May 2017 23:56:21 +0000 Subject: Revert "[SDAG] Relax conditions under stores of loaded values can be merged" This reverts r302712. The change fails with ASAN enabled: ERROR: AddressSanitizer: use-after-poison on address ... at ... READ of size 2 at ... thread T0 #0 ... in llvm::SDNode::getNumValues() const /include/llvm/CodeGen/SelectionDAGNodes.h:855:42 #1 ... in llvm::SDNode::hasAnyUseOfValue(unsigned int) const /lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7270:3 #2 ... in llvm::SDValue::use_empty() const include/llvm/CodeGen/SelectionDAGNodes.h:1042:17 #3 ... in (anonymous namespace)::DAGCombiner::MergeConsecutiveStores(llvm::StoreSDNode*) /lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12944:7 Reviewers: niravd Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D33081 llvm-svn: 302746 --- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 32 +++++++++------------------ 1 file changed, 10 insertions(+), 22 deletions(-) (limited to 'llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp') diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index df776c58001..28354c1f457 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -12783,6 +12783,10 @@ bool DAGCombiner::MergeConsecutiveStores(StoreSDNode *St) { LoadSDNode *Ld = dyn_cast(St->getValue()); if (!Ld) break; + // Loads must only have one use. + if (!Ld->hasNUsesOfValue(1, 0)) + break; + // The memory operands must not be volatile. if (Ld->isVolatile() || Ld->isIndexed()) break; @@ -12791,6 +12795,10 @@ bool DAGCombiner::MergeConsecutiveStores(StoreSDNode *St) { if (Ld->getExtensionType() != ISD::NON_EXTLOAD) break; + // The stored memory type must be the same. + if (Ld->getMemoryVT() != MemVT) + break; + BaseIndexOffset LdPtr = BaseIndexOffset::match(Ld->getBasePtr(), DAG); // If this is not the first ptr that we check. if (LdBasePtr.Base.getNode()) { @@ -12922,28 +12930,8 @@ bool DAGCombiner::MergeConsecutiveStores(StoreSDNode *St) { // Transfer chain users from old loads to the new load. for (unsigned i = 0; i < NumElem; ++i) { LoadSDNode *Ld = cast(LoadNodes[i].MemNode); - if (SDValue(Ld, 0).hasOneUse()) { - // Only the original store used value so just replace chain. - DAG.ReplaceAllUsesOfValueWith(SDValue(Ld, 1), - SDValue(NewLoad.getNode(), 1)); - } else { - // Multiple uses exist. Keep the old load in line with the new - // load, i.e. Replace chains using Ld's chain with a - // TokenFactor. Create a temporary node to serve as a placer so - // we do not replace the reference to original Load's chain in - // the TokenFactor. - SDValue TokenDummy = DAG.getNode(ISD::DummyNode, SDLoc(Ld), MVT::Other); - - // Replace all references to Load's output chain to TokenDummy - CombineTo(Ld, SDValue(Ld, 0), TokenDummy, false); - SDValue Token = - DAG.getNode(ISD::TokenFactor, SDLoc(Ld), MVT::Other, SDValue(Ld, 1), - SDValue(NewLoad.getNode(), 1)); - // Replace all uses of TokenDummy from itself to Ld's output chain. - CombineTo(TokenDummy.getNode(), Token); - assert(TokenDummy.use_empty() && "TokenDummy should be unused"); - AddToWorklist(Ld); - } + DAG.ReplaceAllUsesOfValueWith(SDValue(Ld, 1), + SDValue(NewLoad.getNode(), 1)); } // Replace the all stores with the new store. -- cgit v1.2.3