Preserve DebugInfo when replacing values in DAGCombiner

[niravd]

Summary

[DAG] Previously debug values would transfer debuginfo for the selected
start node for a replacement which allows for debug to be dropped.

Push debug value transfer to occur with node/value replacement in
SelectionDAG, remove now extraneous transfers of debug values.

This fixes PR9817 which was only partially checked in the testsuite.

[dblaikie]

Test cases - and also, ideally, it'd be good to find a narrower place to do
this - having to do it in 5 different places/ways seems like we might
easily be missing places (or miss them in the future)?

[niravd]

Test case, remove extra TransferDbgValues calls

[jyknight]

In D21037#450137, @dblaikie wrote:

Also, ideally, it'd be good to find a narrower place to do
this - having to do it in 5 different places/ways seems like we might
easily be missing places (or miss them in the future)?

Given the structure of the existing code, which has 5 different Replace*Uses*With functions implemented next to each-other with a bunch of duplicate-ish code, having a call to this function from each of those -- and not needing to spread it around in other places -- seems good to me.

llvm/trunk/include/llvm/CodeGen/SelectionDAG.h

#endif
   /// value is produced by SD.
   void AddDbgValue(SDDbgValue *DB, SDNode *SD, bool isParameter);
 
   /// Get the debug values which reference the given SDNode.
   ArrayRef<SDDbgValue*> GetDbgValues(const SDNode* SD) {
     return DbgInfo->getSDDbgValues(SD);
   }
 
-  /// Transfer SDDbgValues.
+private:
+  /// Transfer SDDbgValues. Called via ReplaceAllUses{OfValue}?With
   void TransferDbgValues(SDValue From, SDValue To);
 
+public:
   /// Return true if there are any SDDbgValue nodes associated
   /// with this SelectionDAG.
   bool hasDebugValues() const { return !DbgInfo->empty(); }
 
   SDDbgInfo::DbgIterator DbgBegin() { return DbgInfo->DbgBegin(); }
   SDDbgInfo::DbgIterator DbgEnd()   { return DbgInfo->DbgEnd(); }
   SDDbgInfo::DbgIterator ByvalParmDbgBegin() {
     return DbgInfo->ByvalParmDbgBegin();

llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

while (!WorklistMap.empty()) {
 
     assert(N->getOpcode() != ISD::DELETED_NODE &&
            RV.getNode()->getOpcode() != ISD::DELETED_NODE &&
            "Node was deleted but visit returned new node!");
 
     DEBUG(dbgs() << " ... into: ";
           RV.getNode()->dump(&DAG));
 
-    // Transfer debug value.
-    DAG.TransferDbgValues(SDValue(N, 0), RV);
     if (N->getNumValues() == RV.getNode()->getNumValues())
       DAG.ReplaceAllUsesWith(N, RV.getNode());
     else {
       assert(N->getValueType(0) == RV.getValueType() &&
              N->getNumValues() == 1 && "Type mismatch");
       SDValue OpV = RV;
       DAG.ReplaceAllUsesWith(N, &OpV);
     }

llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp

public:
   void ReplaceNode(SDNode *Old, SDNode *New) {
     DEBUG(dbgs() << " ... replacing: "; Old->dump(&DAG);
           dbgs() << "     with:      "; New->dump(&DAG));
 
     assert(Old->getNumValues() == New->getNumValues() &&
            "Replacing one node with another that produces a different number "
            "of values!");
     DAG.ReplaceAllUsesWith(Old, New);
-    for (unsigned i = 0, e = Old->getNumValues(); i != e; ++i)
-      DAG.TransferDbgValues(SDValue(Old, i), SDValue(New, i));
     if (UpdatedNodes)
       UpdatedNodes->insert(New);
     ReplacedNode(Old);
   }
   void ReplaceNode(SDValue Old, SDValue New) {
     DEBUG(dbgs() << " ... replacing: "; Old->dump(&DAG);
           dbgs() << "     with:      "; New->dump(&DAG));
 
     DAG.ReplaceAllUsesWith(Old, New);
-    DAG.TransferDbgValues(Old, New);
     if (UpdatedNodes)
       UpdatedNodes->insert(New.getNode());
     ReplacedNode(Old.getNode());
   }
   void ReplaceNode(SDNode *Old, const SDValue *New) {
     DEBUG(dbgs() << " ... replacing: "; Old->dump(&DAG));
 
     DAG.ReplaceAllUsesWith(Old, New);
     for (unsigned i = 0, e = Old->getNumValues(); i != e; ++i) {
       DEBUG(dbgs() << (i == 0 ? "     with:      "
                               : "      and:      ");
             New[i]->dump(&DAG));
-      DAG.TransferDbgValues(SDValue(Old, i), New[i]);
       if (UpdatedNodes)
         UpdatedNodes->insert(New[i].getNode());
     }
     ReplacedNode(Old);
   }
 };
 }
 

llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

do {
       Use.set(To);
     } while (UI != UE && *UI == User);
 
     // Now that we have modified User, add it back to the CSE maps.  If it
     // already exists there, recursively merge the results together.
     AddModifiedNodeToCSEMaps(User);
   }
 
+  // Preserve Debug Values
+  TransferDbgValues(FromN, To);
+
   // If we just RAUW'd the root, take note.
   if (FromN == getRoot())
     setRoot(To);
 }
 
 /// ReplaceAllUsesWith - Modify anything using 'From' to use 'To' instead.
 /// This can cause recursive merging of nodes in the DAG.
 ///
 /// This version assumes that for each value of From, there is a
 /// corresponding value in To in the same position with the same type.
 ///
 void SelectionDAG::ReplaceAllUsesWith(SDNode *From, SDNode *To) {
 #ifndef NDEBUG
   for (unsigned i = 0, e = From->getNumValues(); i != e; ++i)
     assert((!From->hasAnyUseOfValue(i) ||
             From->getValueType(i) == To->getValueType(i)) &&
            "Cannot use this version of ReplaceAllUsesWith!");
 #endif
 
   // Handle the trivial case.
   if (From == To)
     return;
 
+  // Preserve Debug Info. Only do this if there's a use.
+  for (unsigned i = 0, e = From->getNumValues(); i != e; ++i)
+    if (From->hasAnyUseOfValue(i))
+      TransferDbgValues(SDValue(From, i), SDValue(To, i));
+
   // Iterate over just the existing users of From. See the comments in
   // the ReplaceAllUsesWith above.
   SDNode::use_iterator UI = From->use_begin(), UE = From->use_end();
   RAUWUpdateListener Listener(*this, UI, UE);
   while (UI != UE) {
     SDNode *User = *UI;
 
     // This node is about to morph, remove its old self from the CSE maps.

 /// This can cause recursive merging of nodes in the DAG.
 ///
 /// This version can replace From with any result values.  To must match the
 /// number and types of values returned by From.
 void SelectionDAG::ReplaceAllUsesWith(SDNode *From, const SDValue *To) {
   if (From->getNumValues() == 1)  // Handle the simple case efficiently.
     return ReplaceAllUsesWith(SDValue(From, 0), To[0]);
 
+  // Preserve Debug Info.
+  for (unsigned i = 0, e = From->getNumValues(); i != e; ++i)
+    TransferDbgValues(SDValue(From, i), *To);
+
   // Iterate over just the existing users of From. See the comments in
   // the ReplaceAllUsesWith above.
   SDNode::use_iterator UI = From->use_begin(), UE = From->use_end();
   RAUWUpdateListener Listener(*this, UI, UE);
   while (UI != UE) {
     SDNode *User = *UI;
 
     // This node is about to morph, remove its old self from the CSE maps.
void SelectionDAG::ReplaceAllUsesOfValueWith(SDValue From, SDValue To){
   if (From == To) return;
 
   // Handle the simple, trivial, case efficiently.
   if (From.getNode()->getNumValues() == 1) {
     ReplaceAllUsesWith(From, To);
     return;
   }
 
+  // Preserve Debug Info.
+  TransferDbgValues(From, To);
+
   // Iterate over just the existing users of From. See the comments in
   // the ReplaceAllUsesWith above.
   SDNode::use_iterator UI = From.getNode()->use_begin(),
                        UE = From.getNode()->use_end();
   RAUWUpdateListener Listener(*this, UI, UE);
   while (UI != UE) {
     SDNode *User = *UI;
     bool UserRemovedFromCSEMaps = false;

 /// handled the same way as for ReplaceAllUsesWith.
 void SelectionDAG::ReplaceAllUsesOfValuesWith(const SDValue *From,
                                               const SDValue *To,
                                               unsigned Num){
   // Handle the simple, trivial case efficiently.
   if (Num == 1)
     return ReplaceAllUsesOfValueWith(*From, *To);
 
+  TransferDbgValues(*From, *To);
+
   // Read up all the uses and make records of them. This helps
   // processing new uses that are introduced during the
   // replacement process.
   SmallVector<UseMemo, 4> Uses;
   for (unsigned i = 0; i != Num; ++i) {
     unsigned FromResNo = From[i].getResNo();
     SDNode *FromNode = From[i].getNode();
     for (SDNode::use_iterator UI = FromNode->use_begin(),

 void SelectionDAG::AddDbgValue(SDDbgValue *DB, SDNode *SD, bool isParameter) {
   if (SD) {
     assert(DbgInfo->getSDDbgValues(SD).empty() || SD->getHasDebugValue());
     SD->setHasDebugValue(true);
   }
   DbgInfo->add(DB, SD, isParameter);
 }
 
-/// TransferDbgValues - Transfer SDDbgValues.
+/// TransferDbgValues - Transfer SDDbgValues. Called in replace nodes.
 void SelectionDAG::TransferDbgValues(SDValue From, SDValue To) {
   if (From == To || !From.getNode()->getHasDebugValue())
     return;
   SDNode *FromNode = From.getNode();
   SDNode *ToNode = To.getNode();
   ArrayRef<SDDbgValue *> DVs = GetDbgValues(FromNode);
   SmallVector<SDDbgValue *, 2> ClonedDVs;
   for (ArrayRef<SDDbgValue *>::iterator I = DVs.begin(), E = DVs.end();

llvm/trunk/lib/Target/Hexagon/HexagonISelDAGToDAG.cpp

void HexagonDAGToDAGISel::SelectFrameIndex(SDNode *N) {
   } else {
     auto &HMFI = *MF->getInfo<HexagonMachineFunctionInfo>();
     unsigned AR = HMFI.getStackAlignBaseVReg();
     SDValue CH = CurDAG->getEntryNode();
     SDValue Ops[] = { CurDAG->getCopyFromReg(CH, DL, AR, MVT::i32), FI, Zero };
     R = CurDAG->getMachineNode(Hexagon::TFR_FIA, DL, MVT::i32, Ops);
   }
 
-  if (N->getHasDebugValue())
-    CurDAG->TransferDbgValues(SDValue(N, 0), SDValue(R, 0));
   ReplaceNode(N, R);
 }
 
 
 void HexagonDAGToDAGISel::Select(SDNode *N) {
   if (N->isMachineOpcode()) {
     N->setNodeId(-1);
     return;   // Already selected.

llvm/trunk/lib/Target/Hexagon/HexagonISelLowering.cpp

DEBUG({
     dbgs () << LLVM_FUNCTION_NAME << " Align: " << A << " Size: ";
     Size.getNode()->dump(&DAG);
     dbgs() << "\n";
   });
 
   SDValue AC = DAG.getConstant(A, dl, MVT::i32);
   SDVTList VTs = DAG.getVTList(MVT::i32, MVT::Other);
   SDValue AA = DAG.getNode(HexagonISD::ALLOCA, dl, VTs, Chain, Size, AC);
-  if (Op.getNode()->getHasDebugValue())
-    DAG.TransferDbgValues(Op, AA);
+
+  DAG.ReplaceAllUsesOfValueWith(Op, AA);
   return AA;
 }
 
 SDValue HexagonTargetLowering::LowerFormalArguments(
     SDValue Chain, CallingConv::ID CallConv, bool isVarArg,
     const SmallVectorImpl<ISD::InputArg> &Ins, const SDLoc &dl,
     SelectionDAG &DAG, SmallVectorImpl<SDValue> &InVals) const {
 

llvm/trunk/test/DebugInfo/X86/dbg-value-dag-combine.ll

 ; RUN: llc < %s | FileCheck %s
 target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:32"
 target triple = "i686-apple-darwin"
 ; PR 9817
 
+; There should be a DEBUG_VALUE for each call to llvm.dbg.value
+
+; CHECK:  ##DEBUG_VALUE: __OpenCL_test_kernel:ip <- 
+; CHECK:  ##DEBUG_VALUE: xxx <- 0
+; CHECK:  ##DEBUG_VALUE: gid <- %E{{..$}}
+; CHECK:  ##DEBUG_VALUE: idx <- %E{{..$}}
+; CHECK-NOT:  ##DEBUG_VALUE:
 
 declare <4 x i32> @__amdil_get_global_id_int()
 declare void @llvm.dbg.value(metadata, i64, metadata, metadata)
 define void @__OpenCL_test_kernel(i32 addrspace(1)* %ip) nounwind !dbg !0 {
 entry:
   call void @llvm.dbg.value(metadata i32 addrspace(1)* %ip, i64 0, metadata !7, metadata !DIExpression()), !dbg !8
   %0 = call <4 x i32> @__amdil_get_global_id_int() nounwind
   %1 = extractelement <4 x i32> %0, i32 0
   call void @llvm.dbg.value(metadata i32 %1, i64 0, metadata !9, metadata !DIExpression()), !dbg !11
-  call void @llvm.dbg.value(metadata i32 0, i64 0, metadata !13, metadata !DIExpression()), !dbg !14
+  call void @llvm.dbg.value(metadata i32 0, i64 0, metadata !21, metadata !DIExpression()), !dbg !14
   %tmp2 = load i32, i32 addrspace(1)* %ip, align 4, !dbg !15
   %tmp3 = add i32 0, %tmp2, !dbg !15
-; CHECK:  ##DEBUG_VALUE: idx <- %E{{..$}}
   call void @llvm.dbg.value(metadata i32 %tmp3, i64 0, metadata !13, metadata !DIExpression()), !dbg !15
   %arrayidx = getelementptr i32, i32 addrspace(1)* %ip, i32 %1, !dbg !16
   store i32 %tmp3, i32 addrspace(1)* %arrayidx, align 4, !dbg !16
   ret void, !dbg !17
 }
 !llvm.dbg.cu = !{!2}
 !llvm.module.flags = !{!20}
 

 !12 = !{}
 !13 = !DILocalVariable(name: "idx", line: 4, scope: !10, file: !1, type: !6)
 !14 = !DILocation(line: 4, column: 20, scope: !10)
 !15 = !DILocation(line: 5, column: 15, scope: !10)
 !16 = !DILocation(line: 6, column: 18, scope: !10)
 !17 = !DILocation(line: 7, column: 1, scope: !0)
 !19 = !DIFile(filename: "OCL6368.tmp.cl", directory: "E:\5CUsers\5Cmvillmow.AMD\5CAppData\5CLocal\5CTemp")
 !20 = !{i32 1, !"Debug Info Version", i32 3}
+!21 = !DILocalVariable(name: "xxx", line: 4, scope: !10, file: !1, type: !6)