]> granicus.if.org Git - llvm/commitdiff
SDAG: Rename Select->SelectImpl and repurpose Select as returning void
authorJustin Bogner <mail@justinbogner.com>
Thu, 5 May 2016 23:19:08 +0000 (23:19 +0000)
committerJustin Bogner <mail@justinbogner.com>
Thu, 5 May 2016 23:19:08 +0000 (23:19 +0000)
This is a step towards removing the rampant undefined behaviour in
SelectionDAG, which is a part of llvm.org/PR26808.

We rename SelectionDAGISel::Select to SelectImpl and update targets to
match, and then change Select to return void and consolidate the
sketchy behaviour we're trying to get away from there.

Next, we'll update backends to implement `void Select(...)` instead of
SelectImpl and eventually drop the base Select implementation.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@268693 91177308-0d34-0410-b5e6-96231b3b80d8

20 files changed:
docs/ReleaseNotes.rst
include/llvm/CodeGen/SelectionDAGISel.h
lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp
lib/Target/ARM/ARMISelDAGToDAG.cpp
lib/Target/BPF/BPFISelDAGToDAG.cpp
lib/Target/Hexagon/HexagonISelDAGToDAG.cpp
lib/Target/Lanai/LanaiISelDAGToDAG.cpp
lib/Target/MSP430/MSP430ISelDAGToDAG.cpp
lib/Target/Mips/MipsISelDAGToDAG.cpp
lib/Target/Mips/MipsISelDAGToDAG.h
lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp
lib/Target/NVPTX/NVPTXISelDAGToDAG.h
lib/Target/PowerPC/PPCISelDAGToDAG.cpp
lib/Target/Sparc/SparcISelDAGToDAG.cpp
lib/Target/SystemZ/SystemZISelDAGToDAG.cpp
lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
lib/Target/X86/X86ISelDAGToDAG.cpp
lib/Target/XCore/XCoreISelDAGToDAG.cpp

index 6d1f74c7d0a469ee6b8eb92f4d74fd6969850108..1f88ec1048d422938814a76e63ea08dadc622bbe 100644 (file)
@@ -61,6 +61,10 @@ Non-comprehensive list of changes in this release
   iterator to the next instruction instead of ``void``. Targets that previously
   did ``MBB.erase(I); return;`` now probably want ``return MBB.erase(I);``.
 
+* ``SelectionDAGISel::Select`` now returns ``void``. Out of tree targets will
+  need to be updated to replace the argument node and remove any dead nodes in
+  cases where they currently return an ``SDNode *`` from this interface.
+
 .. NOTE
    For small 1-3 sentence descriptions, just add an entry at the end of
    this list. If your description won't fit comfortably in one bullet
index 31927c530f74a872c9d1ca4b3e9a3f7aa4d371ab..a928becd6dca4a35284b671b34368cf44c50e709 100644 (file)
@@ -76,8 +76,40 @@ public:
   /// right after selection.
   virtual void PostprocessISelDAG() {}
 
-  /// Select - Main hook targets implement to select a node.
-  virtual SDNode *Select(SDNode *N) = 0;
+  /// Main hook for targets to transform nodes into machine nodes.
+  ///
+  /// All targets should implement this hook. The default implementation will be
+  /// made abstract once all targets are migrated off of the legacy hook.
+  virtual void Select(SDNode *N) {
+    SDNode *New = SelectImpl(N);
+    // TODO: Checking DELETED_NODE here is undefined behaviour, which will be
+    // fixed by migrating backends to implement the void Select interface
+    // instead or returning a node.
+    if (New == N || N->getOpcode() == ISD::DELETED_NODE)
+      // If we ask to replace the node with itself or if we deleted the original
+      // node, just move on to the next one. This case will go away once
+      // everyone migrates to stop implementing SelectImpl.
+      return;
+    if (New) {
+      // Replace the node with the returned node. Originally, Select would
+      // always return a node and the caller would replace it, but this doesn't
+      // work for more complicated selection schemes.
+      ReplaceUses(N, New);
+      CurDAG->RemoveDeadNode(N);
+    } else if (N->use_empty())
+      // Clean up dangling nodes if the target didn't bother. These are
+      // basically bugs in the targets, but we were lenient in the past and did
+      // this for them.
+      CurDAG->RemoveDeadNode(N);
+  }
+
+  /// Legacy hook to support transitioning to the return-less Select().
+  ///
+  /// This exposes the old style Select hook. New code should implement void
+  /// Select() instead.
+  virtual SDNode *SelectImpl(SDNode *N) {
+    llvm_unreachable("Subclasses must implement one of Select or SelectImpl");
+  }
 
   /// SelectInlineAsmMemoryOperand - Select the specified address as a target
   /// addressing mode, according to the specified constraint.  If this does
index f37cc6f519ab59d55a73f3aaa8463173d39ae7e4..0aafbbae6dbeceadde590e2d686d0210602079b0 100644 (file)
@@ -950,23 +950,7 @@ void SelectionDAGISel::DoInstructionSelection() {
       if (Node->use_empty())
         continue;
 
-      SDNode *ResNode = Select(Node);
-
-      // FIXME: This is pretty gross.  'Select' should be changed to not return
-      // anything at all and this code should be nuked with a tactical strike.
-
-      // If node should not be replaced, continue with the next one.
-      if (ResNode == Node || Node->getOpcode() == ISD::DELETED_NODE)
-        continue;
-      // Replace node.
-      if (ResNode) {
-        ReplaceUses(Node, ResNode);
-      }
-
-      // If after the replacement this node is not used any more,
-      // remove this dead node.
-      if (Node->use_empty()) // Don't delete EntryToken, etc.
-        CurDAG->RemoveDeadNode(Node);
+      Select(Node);
     }
 
     CurDAG->setRoot(Dummy.getValue());
index 15eee80b83779024a53400774e8889111eba2a9c..37c02388550eac9c8dc66ce48bdb5669fb81c0f9 100644 (file)
@@ -57,7 +57,7 @@ public:
     return SelectionDAGISel::runOnMachineFunction(MF);
   }
 
-  SDNode *Select(SDNode *Node) override;
+  SDNode *SelectImpl(SDNode *Node) override;
 
   /// SelectInlineAsmMemoryOperand - Implement addressing mode selection for
   /// inline asm expressions.
@@ -2329,7 +2329,7 @@ void AArch64DAGToDAGISel::SelectCMP_SWAP(SDNode *N) {
   ReplaceUses(SDValue(N, 1), SDValue(CmpSwap, 2));
 }
 
-SDNode *AArch64DAGToDAGISel::Select(SDNode *Node) {
+SDNode *AArch64DAGToDAGISel::SelectImpl(SDNode *Node) {
   // Dump information about the Node being selected
   DEBUG(errs() << "Selecting: ");
   DEBUG(Node->dump(CurDAG));
index 8d78bcf582d99d7dae6777ac98b460bf85c718b0..898b2751608e3faadb9d82bb6ade62c996126a52 100644 (file)
@@ -61,7 +61,7 @@ public:
   AMDGPUDAGToDAGISel(TargetMachine &TM);
   virtual ~AMDGPUDAGToDAGISel();
   bool runOnMachineFunction(MachineFunction &MF) override;
-  SDNode *Select(SDNode *N) override;
+  SDNode *SelectImpl(SDNode *N) override;
   const char *getPassName() const override;
   void PreprocessISelDAG() override;
   void PostprocessISelDAG() override;
@@ -329,7 +329,7 @@ static unsigned selectSGPRVectorRegClassID(unsigned NumVectorElts) {
   llvm_unreachable("invalid vector size");
 }
 
-SDNode *AMDGPUDAGToDAGISel::Select(SDNode *N) {
+SDNode *AMDGPUDAGToDAGISel::SelectImpl(SDNode *N) {
   unsigned int Opc = N->getOpcode();
   if (N->isMachineOpcode()) {
     N->setNodeId(-1);
index 41cd6eaf74c86e1c834f6d63a9eb5bd63bc43c90..e915085309a8aa295ead08bc3a723f64e42df29f 100644 (file)
@@ -87,8 +87,7 @@ public:
     return CurDAG->getTargetConstant(Imm, dl, MVT::i32);
   }
 
-  SDNode *Select(SDNode *N) override;
-
+  SDNode *SelectImpl(SDNode *N) override;
 
   bool hasNoVMLxHazardUse(SDNode *N) const;
   bool isShifterOpProfitable(const SDValue &Shift,
@@ -2634,7 +2633,7 @@ SDNode *ARMDAGToDAGISel::SelectConcatVector(SDNode *N) {
   return createDRegPairNode(VT, N->getOperand(0), N->getOperand(1));
 }
 
-SDNode *ARMDAGToDAGISel::Select(SDNode *N) {
+SDNode *ARMDAGToDAGISel::SelectImpl(SDNode *N) {
   SDLoc dl(N);
 
   if (N->isMachineOpcode()) {
index 05348ae5db03ec836dbe39f51fcbb87867afc770..8d60258322e13b881024b64a1eed6bfe45fdcdcf 100644 (file)
@@ -46,7 +46,7 @@ private:
 // Include the pieces autogenerated from the target description.
 #include "BPFGenDAGISel.inc"
 
-  SDNode *Select(SDNode *N) override;
+  SDNode *SelectImpl(SDNode *N) override;
 
   // Complex Pattern for address selection.
   bool SelectAddr(SDValue Addr, SDValue &Base, SDValue &Offset);
@@ -115,7 +115,7 @@ bool BPFDAGToDAGISel::SelectFIAddr(SDValue Addr, SDValue &Base, SDValue &Offset)
   return false;
 }
 
-SDNode *BPFDAGToDAGISel::Select(SDNode *Node) {
+SDNode *BPFDAGToDAGISel::SelectImpl(SDNode *Node) {
   unsigned Opcode = Node->getOpcode();
 
   // Dump information about the Node being selected
index e36cd8f8d27e9c4b97d59387be06aad679e0ea13..9155f95788cf99f948db1a65f750ba5dde682666 100644 (file)
@@ -70,7 +70,7 @@ public:
   virtual void PreprocessISelDAG() override;
   virtual void EmitFunctionEntryCode() override;
 
-  SDNode *Select(SDNode *N) override;
+  SDNode *SelectImpl(SDNode *N) override;
 
   // Complex Pattern Selectors.
   inline bool SelectAddrGA(SDValue &N, SDValue &R);
@@ -1284,7 +1284,7 @@ SDNode *HexagonDAGToDAGISel::SelectFrameIndex(SDNode *N) {
 }
 
 
-SDNode *HexagonDAGToDAGISel::Select(SDNode *N) {
+SDNode *HexagonDAGToDAGISel::SelectImpl(SDNode *N) {
   if (N->isMachineOpcode()) {
     N->setNodeId(-1);
     return nullptr;   // Already selected.
index 01d5b39e9a3d3e07afde21a982b58214e6a446ff..f37331b6cb48406a4dd3592386af6c1854f0b4bd 100644 (file)
@@ -68,7 +68,7 @@ private:
 #include "LanaiGenDAGISel.inc"
 
   // Instruction Selection not handled by the auto-generated tablgen
-  SDNode *Select(SDNode *N) override;
+  SDNode *SelectImpl(SDNode *N) override;
 
   // Support functions for the opcodes of Instruction Selection
   // not handled by the auto-generated tablgen
@@ -270,7 +270,7 @@ bool LanaiDAGToDAGISel::SelectInlineAsmMemoryOperand(
 
 // Select instructions not customized! Used for
 // expanded, promoted and normal instructions
-SDNode *LanaiDAGToDAGISel::Select(SDNode *Node) {
+SDNode *LanaiDAGToDAGISel::SelectImpl(SDNode *Node) {
   unsigned Opcode = Node->getOpcode();
 
   // Dump information about the Node being selected
index 1af5a83aa6c3f2802c4302e9f7a645b6fc77ce0b..7119c3886e6deef4fdb10746f62cad5110f4ed7d 100644 (file)
@@ -110,7 +110,7 @@ namespace {
   #include "MSP430GenDAGISel.inc"
 
   private:
-    SDNode *Select(SDNode *N) override;
+    SDNode *SelectImpl(SDNode *N) override;
     SDNode *SelectIndexedLoad(SDNode *Op);
     SDNode *SelectIndexedBinOp(SDNode *Op, SDValue N1, SDValue N2,
                                unsigned Opc8, unsigned Opc16);
@@ -376,7 +376,7 @@ SDNode *MSP430DAGToDAGISel::SelectIndexedBinOp(SDNode *Op,
 }
 
 
-SDNode *MSP430DAGToDAGISel::Select(SDNode *Node) {
+SDNode *MSP430DAGToDAGISel::SelectImpl(SDNode *Node) {
   SDLoc dl(Node);
 
   // Dump information about the Node being selected
index 06502397b6b8c826855f339966341439b2cc7d39..a5a127ca4699ffbe990b1e2aa45eb308e2ff6d04 100644 (file)
@@ -182,7 +182,7 @@ bool MipsDAGToDAGISel::selectVSplatMaskR(SDValue N, SDValue &Imm) const {
 
 /// Select instructions not customized! Used for
 /// expanded, promoted and normal instructions
-SDNode* MipsDAGToDAGISel::Select(SDNode *Node) {
+SDNode *MipsDAGToDAGISel::SelectImpl(SDNode *Node) {
   unsigned Opcode = Node->getOpcode();
 
   // Dump information about the Node being selected
index 1426d0fbf5161778985277ae81f4d012795f28ce..cfffa36531fb24ddad33fa6971bd739cbfbcf9ae 100644 (file)
@@ -114,7 +114,7 @@ private:
   /// starting at bit zero.
   virtual bool selectVSplatMaskR(SDValue N, SDValue &Imm) const;
 
-  SDNode *Select(SDNode *N) override;
+  SDNode *SelectImpl(SDNode *N) override;
 
   virtual std::pair<bool, SDNode*> selectNode(SDNode *Node) = 0;
 
index 32bb279f0e7bf30e4317a0d8fd3005fffd16987c..0338678f3ae9a84c5afe8bbc3d25c238ef1ee5fc 100644 (file)
@@ -105,7 +105,7 @@ bool NVPTXDAGToDAGISel::allowFMA() const {
 
 /// Select - Select instructions not customized! Used for
 /// expanded, promoted and normal instructions.
-SDNode *NVPTXDAGToDAGISel::Select(SDNode *N) {
+SDNode *NVPTXDAGToDAGISel::SelectImpl(SDNode *N) {
 
   if (N->isMachineOpcode()) {
     N->setNodeId(-1);
index d62cc304e3cf9dc28326702458110693901014bb..d9fa00e5e52d4c608c365be7bdc82942d4a926d9 100644 (file)
@@ -53,7 +53,7 @@ private:
 // Include the pieces autogenerated from the target description.
 #include "NVPTXGenDAGISel.inc"
 
-  SDNode *Select(SDNode *N) override;
+  SDNode *SelectImpl(SDNode *N) override;
   SDNode *SelectIntrinsicNoChain(SDNode *N);
   SDNode *SelectIntrinsicChain(SDNode *N);
   SDNode *SelectTexSurfHandle(SDNode *N);
@@ -69,7 +69,7 @@ private:
   SDNode *SelectTextureIntrinsic(SDNode *N);
   SDNode *SelectSurfaceIntrinsic(SDNode *N);
   SDNode *SelectBFE(SDNode *N);
-        
+
   inline SDValue getI32Imm(unsigned Imm, SDLoc DL) {
     return CurDAG->getTargetConstant(Imm, DL, MVT::i32);
   }
index 7e8fc3099f76404cb6a3c971fe9fc8fadde6b96b..7e615a4f9cc67685270608e122011351d718b1d0 100644 (file)
@@ -126,7 +126,7 @@ namespace {
 
     // Select - Convert the specified operand from a target-independent to a
     // target-specific node if it hasn't already been changed.
-    SDNode *Select(SDNode *N) override;
+    SDNode *SelectImpl(SDNode *N) override;
 
     SDNode *SelectBitfieldInsert(SDNode *N);
     SDNode *SelectBitPermutation(SDNode *N);
@@ -1209,7 +1209,7 @@ class BitPermutationSelector {
                  "bit group ends at index 63 but there is another?");
           auto IN = BitGroups.begin();
 
-          if (IP->Repl32 && IN->Repl32 && I->V == IP->V && I->V == IN->V && 
+          if (IP->Repl32 && IN->Repl32 && I->V == IP->V && I->V == IN->V &&
               (I->RLAmt % 32) == IP->RLAmt && (I->RLAmt % 32) == IN->RLAmt &&
               IP->EndIdx == 31 && IN->StartIdx == 0 && I != IP &&
               IsAllLow32(*I)) {
@@ -2408,7 +2408,7 @@ SDNode *PPCDAGToDAGISel::transferMemOperands(SDNode *N, SDNode *Result) {
 
 // Select - Convert the specified operand from a target-independent to a
 // target-specific node if it hasn't already been changed.
-SDNode *PPCDAGToDAGISel::Select(SDNode *N) {
+SDNode *PPCDAGToDAGISel::SelectImpl(SDNode *N) {
   SDLoc dl(N);
   if (N->isMachineOpcode()) {
     N->setNodeId(-1);
@@ -4384,4 +4384,3 @@ static void initializePassOnce(PassRegistry &Registry) {
 void llvm::initializePPCDAGToDAGISelPass(PassRegistry &Registry) {
   CALL_ONCE_INITIALIZATION(initializePassOnce);
 }
-
index b87b194f08fdd89e48d0802f3cce8bcf3d168989..990f101f7676254ca20fa44f088b34d4a0d5bb92 100644 (file)
@@ -41,7 +41,7 @@ public:
     return SelectionDAGISel::runOnMachineFunction(MF);
   }
 
-  SDNode *Select(SDNode *N) override;
+  SDNode *SelectImpl(SDNode *N) override;
 
   // Complex Pattern Selectors.
   bool SelectADDRrr(SDValue N, SDValue &R1, SDValue &R2);
@@ -317,7 +317,7 @@ SDNode *SparcDAGToDAGISel::SelectInlineAsm(SDNode *N){
   return New.getNode();
 }
 
-SDNode *SparcDAGToDAGISel::Select(SDNode *N) {
+SDNode *SparcDAGToDAGISel::SelectImpl(SDNode *N) {
   SDLoc dl(N);
   if (N->isMachineOpcode()) {
     N->setNodeId(-1);
index ddf7af73bc292502c1405d5392ff98f719a3a85c..ee1aed740fe300e9161f06b17c818209cda448b9 100644 (file)
@@ -343,7 +343,7 @@ public:
   }
 
   // Override SelectionDAGISel.
-  SDNode *Select(SDNode *Node) override;
+  SDNode *SelectImpl(SDNode *Node) override;
   bool SelectInlineAsmMemoryOperand(const SDValue &Op, unsigned ConstraintID,
                                     std::vector<SDValue> &OutOps) override;
 
@@ -1041,7 +1041,8 @@ SDNode *SystemZDAGToDAGISel::splitLargeImmediate(unsigned Opcode, SDNode *Node,
   SDValue Upper = CurDAG->getConstant(UpperVal, DL, VT);
   if (Op0.getNode())
     Upper = CurDAG->getNode(Opcode, DL, VT, Op0, Upper);
-  Upper = SDValue(Select(Upper.getNode()), 0);
+  // TODO: This is pretty strange. Not sure what it's trying to do...
+  Upper = SDValue(SelectImpl(Upper.getNode()), 0);
 
   SDValue Lower = CurDAG->getConstant(LowerVal, DL, VT);
   SDValue Or = CurDAG->getNode(Opcode, DL, VT, Upper, Lower);
@@ -1171,7 +1172,7 @@ bool SystemZDAGToDAGISel::storeLoadCanUseBlockBinary(SDNode *N,
   return !LoadA->isVolatile() && canUseBlockOperation(StoreA, LoadB);
 }
 
-SDNode *SystemZDAGToDAGISel::Select(SDNode *Node) {
+SDNode *SystemZDAGToDAGISel::SelectImpl(SDNode *Node) {
   // Dump information about the Node being selected
   DEBUG(errs() << "Selecting: "; Node->dump(CurDAG); errs() << "\n");
 
index 8390f797c43ec3cf758f17bd4f926d3dacb3dfe8..235f149d8ca09e35b1a17b4ac4fef93c70afd481 100644 (file)
@@ -54,7 +54,7 @@ public:
     return SelectionDAGISel::runOnMachineFunction(MF);
   }
 
-  SDNode *Select(SDNode *Node) override;
+  SDNode *SelectImpl(SDNode *Node) override;
 
   bool SelectInlineAsmMemoryOperand(const SDValue &Op, unsigned ConstraintID,
                                     std::vector<SDValue> &OutOps) override;
@@ -67,7 +67,7 @@ private:
 };
 } // end anonymous namespace
 
-SDNode *WebAssemblyDAGToDAGISel::Select(SDNode *Node) {
+SDNode *WebAssemblyDAGToDAGISel::SelectImpl(SDNode *Node) {
   // Dump information about the Node being selected.
   DEBUG(errs() << "Selecting: ");
   DEBUG(Node->dump(CurDAG));
index a565a48b5f4633a1b3aa91e9ceac77386af88a8b..9fd3a106c461e5ecd268fd6841bc4a157c5caa89 100644 (file)
@@ -196,7 +196,7 @@ namespace {
 #include "X86GenDAGISel.inc"
 
   private:
-    SDNode *Select(SDNode *N) override;
+    SDNode *SelectImpl(SDNode *N) override;
     SDNode *selectGather(SDNode *N, unsigned Opc);
 
     bool foldOffsetIntoAddress(uint64_t Offset, X86ISelAddressMode &AM);
@@ -326,7 +326,7 @@ namespace {
         // types.
         if (User->getNumOperands() != 2)
           continue;
-        
+
         // Immediates that are used for offsets as part of stack
         // manipulation should be left alone. These are typically
         // used to indicate SP offsets for argument passing and
@@ -1926,7 +1926,7 @@ SDNode *X86DAGToDAGISel::selectGather(SDNode *Node, unsigned Opc) {
   return ResNode;
 }
 
-SDNode *X86DAGToDAGISel::Select(SDNode *Node) {
+SDNode *X86DAGToDAGISel::SelectImpl(SDNode *Node) {
   MVT NVT = Node->getSimpleValueType(0);
   unsigned Opc, MOpc;
   unsigned Opcode = Node->getOpcode();
index 4d3ec538d9fc41385caeb25f9fbc0483c39689fb..d9cff7e88b21bfb9a19b6c7d7bb4b48282e524f6 100644 (file)
@@ -41,7 +41,7 @@ namespace {
     XCoreDAGToDAGISel(XCoreTargetMachine &TM, CodeGenOpt::Level OptLevel)
       : SelectionDAGISel(TM, OptLevel) {}
 
-    SDNode *Select(SDNode *N) override;
+    SDNode *SelectImpl(SDNode *N) override;
     SDNode *SelectBRIND(SDNode *N);
 
     /// getI32Imm - Return a target constant with the specified value, of type
@@ -69,14 +69,14 @@ namespace {
 
     const char *getPassName() const override {
       return "XCore DAG->DAG Pattern Instruction Selection";
-    } 
-    
+    }
+
     // Include the pieces autogenerated from the target description.
   #include "XCoreGenDAGISel.inc"
   };
 }  // end anonymous namespace
 
-/// createXCoreISelDag - This pass converts a legalized DAG into a 
+/// createXCoreISelDag - This pass converts a legalized DAG into a
 /// XCore-specific DAG, ready for instruction scheduling.
 ///
 FunctionPass *llvm::createXCoreISelDag(XCoreTargetMachine &TM,
@@ -129,7 +129,7 @@ SelectInlineAsmMemoryOperand(const SDValue &Op, unsigned ConstraintID,
   return false;
 }
 
-SDNode *XCoreDAGToDAGISel::Select(SDNode *N) {
+SDNode *XCoreDAGToDAGISel::SelectImpl(SDNode *N) {
   SDLoc dl(N);
   switch (N->getOpcode()) {
   default: break;
@@ -204,7 +204,7 @@ SDNode *XCoreDAGToDAGISel::Select(SDNode *N) {
 
 /// Given a chain return a new chain where any appearance of Old is replaced
 /// by New. There must be at most one instruction between Old and Chain and
-/// this instruction must be a TokenFactor. Returns an empty SDValue if 
+/// this instruction must be a TokenFactor. Returns an empty SDValue if
 /// these conditions don't hold.
 static SDValue
 replaceInChain(SelectionDAG *CurDAG, SDValue Chain, SDValue Old, SDValue New)