From cd4006f3bf5f0da752da8a32d8fec0f56e539ab6 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Tue, 11 Feb 2025 19:37:32 -0800 Subject: [PATCH 1/2] [SelectionDAGBuidler] Remove NodeMap updates from getValueImpl. NFC Both callers already put the result in NodeMap immediately after the call. A long time ago getValueImpl's body was part of getValue. There was reference taken to NodeMap[V] taken at the beginning and that reference was used to update the map. But getValue is recursive for the creation of builder_vector. The recursion may invalidate that reference so the builder_vector code couldn't use that reference and had to update the map directly. Later another recursive case was added that was added and to fix the stale reference, getValueImpl was introduced with the map update being done by the caller. Since build_vector had its own NodeMap update, I guess it was missed in that update. The other NodeMap update this patch removes were probably just copied from the build_vector code without knowing it was unnecessary. --- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index 5a5596a542f72..8ebdb7058357f 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -1855,7 +1855,7 @@ SDValue SelectionDAGBuilder::getValueImpl(const Value *V) { if (isa(CDS->getType())) return DAG.getMergeValues(Ops, getCurSDLoc()); - return NodeMap[V] = DAG.getBuildVector(VT, getCurSDLoc(), Ops); + return DAG.getBuildVector(VT, getCurSDLoc(), Ops); } if (C->getType()->isStructTy() || C->getType()->isArrayTy()) { @@ -1898,7 +1898,7 @@ SDValue SelectionDAGBuilder::getValueImpl(const Value *V) { if (VT.isRISCVVectorTuple()) { assert(C->isNullValue() && "Can only zero this target type!"); - return NodeMap[V] = DAG.getNode( + return DAG.getNode( ISD::BITCAST, getCurSDLoc(), VT, DAG.getNode( ISD::SPLAT_VECTOR, getCurSDLoc(), @@ -1918,7 +1918,7 @@ SDValue SelectionDAGBuilder::getValueImpl(const Value *V) { for (unsigned i = 0; i != NumElements; ++i) Ops.push_back(getValue(CV->getOperand(i))); - return NodeMap[V] = DAG.getBuildVector(VT, getCurSDLoc(), Ops); + return DAG.getBuildVector(VT, getCurSDLoc(), Ops); } if (isa(C)) { @@ -1931,7 +1931,7 @@ SDValue SelectionDAGBuilder::getValueImpl(const Value *V) { else Op = DAG.getConstant(0, getCurSDLoc(), EltVT); - return NodeMap[V] = DAG.getSplat(VT, getCurSDLoc(), Op); + return DAG.getSplat(VT, getCurSDLoc(), Op); } llvm_unreachable("Unknown vector constant"); From 871d2015082fb6580993124906865ab791efa640 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Tue, 11 Feb 2025 22:13:16 -0800 Subject: [PATCH 2/2] fixup! clang-format --- .../CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index 8ebdb7058357f..cac25fd7c1025 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -1899,13 +1899,12 @@ SDValue SelectionDAGBuilder::getValueImpl(const Value *V) { if (VT.isRISCVVectorTuple()) { assert(C->isNullValue() && "Can only zero this target type!"); return DAG.getNode( - ISD::BITCAST, getCurSDLoc(), VT, - DAG.getNode( - ISD::SPLAT_VECTOR, getCurSDLoc(), - EVT::getVectorVT(*DAG.getContext(), MVT::i8, - VT.getSizeInBits().getKnownMinValue() / 8, - true), - DAG.getConstant(0, getCurSDLoc(), MVT::getIntegerVT(8)))); + ISD::BITCAST, getCurSDLoc(), VT, + DAG.getNode( + ISD::SPLAT_VECTOR, getCurSDLoc(), + EVT::getVectorVT(*DAG.getContext(), MVT::i8, + VT.getSizeInBits().getKnownMinValue() / 8, true), + DAG.getConstant(0, getCurSDLoc(), MVT::getIntegerVT(8)))); } VectorType *VecTy = cast(V->getType());