[SelectionDAGBuilder] Remove NodeMap updates from getValueImpl. NFC

[topperc]

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.

[llvmbot]

@llvm/pr-subscribers-llvm-selectiondag

Author: Craig Topper (topperc)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/126849.diff

1 Files Affected:

• (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+4-4)

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<ArrayType>(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<ConstantAggregateZero>(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");

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.