From 96b7e0b5a0c6bd5742af783ed83cc41c299e9472 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Sat, 12 Feb 2022 10:34:51 -0500 Subject: [PATCH] [SDAG] clean up scalarizing load transform I have not found a way to expose a difference for this patch in a test because it only triggers for a one-use load, but this is the code that was adapted into D118376 and caused miscompiles. The new code pattern is the same as what we do in narrowExtractedVectorLoad() (reduces load width for a subvector extract). This removes seemingly unnecessary manual worklist management and fixes the chain updating via "SelectionDAG::makeEquivalentMemoryOrdering()". Differential Revision: https://reviews.llvm.org/D119549 --- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 5ab75a3..dbf2f4e 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -19195,14 +19195,9 @@ SDValue DAGCombiner::scalarizeExtractedVectorLoad(SDNode *EVE, EVT InVecVT, SDValue NewPtr = TLI.getVectorElementPointer(DAG, OriginalLoad->getBasePtr(), InVecVT, EltNo); - // The replacement we need to do here is a little tricky: we need to - // replace an extractelement of a load with a load. - // Use ReplaceAllUsesOfValuesWith to do the replacement. - // Note that this replacement assumes that the extractvalue is the only - // use of the load; that's okay because we don't want to perform this - // transformation in other cases anyway. + // We are replacing a vector load with a scalar load. The new load must have + // identical memory op ordering to the original. SDValue Load; - SDValue Chain; if (ResultVT.bitsGT(VecEltVT)) { // If the result type of vextract is wider than the load, then issue an // extending load instead. @@ -19213,28 +19208,20 @@ SDValue DAGCombiner::scalarizeExtractedVectorLoad(SDNode *EVE, EVT InVecVT, NewPtr, MPI, VecEltVT, Alignment, OriginalLoad->getMemOperand()->getFlags(), OriginalLoad->getAAInfo()); - Chain = Load.getValue(1); + DAG.makeEquivalentMemoryOrdering(OriginalLoad, Load); } else { + // The result type is narrower or the same width as the vector element Load = DAG.getLoad(VecEltVT, DL, OriginalLoad->getChain(), NewPtr, MPI, Alignment, OriginalLoad->getMemOperand()->getFlags(), OriginalLoad->getAAInfo()); - Chain = Load.getValue(1); + DAG.makeEquivalentMemoryOrdering(OriginalLoad, Load); if (ResultVT.bitsLT(VecEltVT)) Load = DAG.getNode(ISD::TRUNCATE, DL, ResultVT, Load); else Load = DAG.getBitcast(ResultVT, Load); } - WorklistRemover DeadNodes(*this); - SDValue From[] = {SDValue(EVE, 0), SDValue(OriginalLoad, 1)}; - SDValue To[] = {Load, Chain}; - DAG.ReplaceAllUsesOfValuesWith(From, To, 2); - // Make sure to revisit this node to clean it up; it will usually be dead. - AddToWorklist(EVE); - // Since we're explicitly calling ReplaceAllUses, add the new node to the - // worklist explicitly as well. - AddToWorklistWithUsers(Load.getNode()); ++OpsNarrowed; - return SDValue(EVE, 0); + return Load; } /// Transform a vector binary operation into a scalar binary operation by moving -- 2.7.4