[Debug info] Teach the SDag type legalizer how to split up debug info for integer values that are split into a hi and lo part.

[JDevlieghere]

Summary

Revival of D4832

It now also includes a test for MSP430 to address for the situation described by @iains, which seems to work with the current implementation.

[aprantl]

Thanks, I have a couple of smaller nitpicks, but this generally seems good to me.

llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp

829

This comment basically just repeats the name of the function. I would either document what the function does, i.e. generate fragment expressions for the split up value, or remove it entirely.

837

llvm coding style prefers early exit over nesting, i.e:

if (Dbg->getKind() !=SDNode)
   break;

838

perhaps change the return type in SDDbgValue instead?

842

why is the &* necessary?

llvm/test/DebugInfo/MSP430/sdagsplit-1.ll

21

Hard-coding the metadata node numbers is not a good idea. Since you aren't checking their contents, why check for them at all?
We *should* be checking for the new DIExpressions on the other hand.

42

can you remove all non-essential attributes (at least everything in quotes)?

llvm/test/DebugInfo/X86/sdagsplit-1.ll

19

Probably better to convert this to checking the MIR output, too.

42

ditto

[JDevlieghere]

Thank you for the feedback @aprantl!

[JDevlieghere]

Fix typo in X86 test.

[aprantl]

Thanks!

llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp

853

No need to do this now, but we might want to think about adding this functionality either to DIBuilder or IR/DebugInfo.cpp

llvm/include/llvm/CodeGen/SelectionDAG.h

#endif
   SDValue getTargetInsertSubreg(int SRIdx, const SDLoc &DL, EVT VT,
                                 SDValue Operand, SDValue Subreg);
 
   /// Get the specified node if it's already available, or else return NULL.
   SDNode *getNodeIfExists(unsigned Opcode, SDVTList VTs, ArrayRef<SDValue> Ops,
                           const SDNodeFlags Flags = SDNodeFlags());
 
   /// Creates a SDDbgValue node.
-  SDDbgValue *getDbgValue(MDNode *Var, MDNode *Expr, SDNode *N, unsigned R,
-                          bool IsIndirect, const DebugLoc &DL, unsigned O);
+  SDDbgValue *getDbgValue(DIVariable *Var, DIExpression *Expr, SDNode *N,
+                          unsigned R, bool IsIndirect, const DebugLoc &DL,
+                          unsigned O);
 
   /// Constant
-  SDDbgValue *getConstantDbgValue(MDNode *Var, MDNode *Expr, const Value *C,
-                                  const DebugLoc &DL, unsigned O);
+  SDDbgValue *getConstantDbgValue(DIVariable *Var, DIExpression *Expr,
+                                  const Value *C, const DebugLoc &DL,
+                                  unsigned O);
 
   /// FrameIndex
-  SDDbgValue *getFrameIndexDbgValue(MDNode *Var, MDNode *Expr, unsigned FI,
-                                    const DebugLoc &DL, unsigned O);
+  SDDbgValue *getFrameIndexDbgValue(DIVariable *Var, DIExpression *Expr,
+                                    unsigned FI, const DebugLoc &DL,
+                                    unsigned O);
 
   /// Remove the specified node from the system. If any of its
   /// operands then becomes dead, remove them as well. Inform UpdateListener
   /// for each node deleted.
   void RemoveDeadNode(SDNode *N);
 
   /// This method deletes the unreachable nodes in the
   /// given list, and any nodes that become unreachable as a result.

llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp

 //===-- LegalizeTypes.cpp - Common code for DAG type legalizer ------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the SelectionDAG::LegalizeTypes method.  It transforms
 // an arbitrary well-formed SelectionDAG to only consist of legal types.  This
 // is common code shared among the LegalizeTypes*.cpp files.
 //
 //===----------------------------------------------------------------------===//
 
 #include "LegalizeTypes.h"
+#include "SDNodeDbgValue.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/IR/CallingConv.h"
+#include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "legalize-types"
 
void DAGTypeLegalizer::GetExpandedInteger(SDValue Op, SDValue &Lo,
   std::pair<SDValue, SDValue> &Entry = ExpandedIntegers[Op];
   RemapValue(Entry.first);
   RemapValue(Entry.second);
   assert(Entry.first.getNode() && "Operand isn't expanded");
   Lo = Entry.first;
   Hi = Entry.second;
 }
 
+/// Transfer debug valies by generating fragment expressions for split-up
+/// values.
+static void transferDbgValues(SelectionDAG &DAG, DIBuilder &DIB, SDValue From,
+                              SDValue To, unsigned OffsetInBits) {
+  SDNode *FromNode = From.getNode();
+  SDNode *ToNode = To.getNode();
+  assert(FromNode != ToNode);
+
+  for (SDDbgValue *Dbg : DAG.GetDbgValues(FromNode)) {
+    if (Dbg->getKind() != SDDbgValue::SDNODE)
+      break;
+
+    DIVariable *Var = Dbg->getVariable();
+    DIExpression *Fragment = DIB.createFragmentExpression(
+        OffsetInBits, To.getValueSizeInBits(),
+        cast_or_null<DIExpression>(Dbg->getExpression()));
+    SDDbgValue *Clone =
+        DAG.getDbgValue(Var, Fragment, ToNode, To.getResNo(), Dbg->isIndirect(),
+                        Dbg->getDebugLoc(), Dbg->getOrder());
+    Dbg->setIsInvalidated();
+    DAG.AddDbgValue(Clone, ToNode, false);
+
+    // Add the expression to the metadata graph so isn't lost in MIR dumps.
+    const Module *M = DAG.getMachineFunction().getMMI().getModule();
+    M->getNamedMetadata("llvm.dbg.mir")->addOperand(Fragment);
+  }
+}
+
 void DAGTypeLegalizer::SetExpandedInteger(SDValue Op, SDValue Lo,
                                           SDValue Hi) {
   assert(Lo.getValueType() ==
          TLI.getTypeToTransformTo(*DAG.getContext(), Op.getValueType()) &&
          Hi.getValueType() == Lo.getValueType() &&
          "Invalid type for expanded integer");
   // Lo/Hi may have been newly allocated, if so, add nodeid's as relevant.
   AnalyzeNewValue(Lo);
   AnalyzeNewValue(Hi);
 
+  // Transfer debug values.
+  const Module *M = DAG.getMachineFunction().getMMI().getModule();
+  DIBuilder DIB(*const_cast<Module *>(M));
+  transferDbgValues(DAG, DIB, Op, Lo, 0);
+  transferDbgValues(DAG, DIB, Op, Hi, Lo.getValueSizeInBits());
+
   // Remember that this is the result of the node.
   std::pair<SDValue, SDValue> &Entry = ExpandedIntegers[Op];
   assert(!Entry.first.getNode() && "Node already expanded");
   Entry.first = Lo;
   Entry.second = Hi;
 }
 
 void DAGTypeLegalizer::GetExpandedFloat(SDValue Op, SDValue &Lo,

llvm/lib/CodeGen/SelectionDAG/SDNodeDbgValue.h

 #define LLVM_LIB_CODEGEN_SELECTIONDAG_SDNODEDBGVALUE_H
 
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/Support/DataTypes.h"
 #include <utility>
 
 namespace llvm {
 
-class MDNode;
+class DIVariable;
+class DIExpression;
 class SDNode;
 class Value;
 
 /// SDDbgValue - Holds the information from a dbg_value node through SDISel.
 /// We do not use SDValue here to avoid including its header.
 
 class SDDbgValue {
 public:
   enum DbgValueKind {
     SDNODE = 0,             // value is the result of an expression
     CONST = 1,              // value is a constant
     FRAMEIX = 2             // value is contents of a stack location
   };
 private:
   union {
     struct {
       SDNode *Node;         // valid for expressions
       unsigned ResNo;       // valid for expressions
     } s;
     const Value *Const;     // valid for constants
     unsigned FrameIx;       // valid for stack objects
   } u;
-  MDNode *Var;
-  MDNode *Expr;
+  DIVariable *Var;
+  DIExpression *Expr;
   DebugLoc DL;
   unsigned Order;
   enum DbgValueKind kind;
   bool IsIndirect;
   bool Invalid = false;
 
 public:
   // Constructor for non-constants.
-  SDDbgValue(MDNode *Var, MDNode *Expr, SDNode *N, unsigned R, bool indir,
-             DebugLoc dl, unsigned O)
+  SDDbgValue(DIVariable *Var, DIExpression *Expr, SDNode *N, unsigned R,
+             bool indir, DebugLoc dl, unsigned O)
       : Var(Var), Expr(Expr), DL(std::move(dl)), Order(O), IsIndirect(indir) {
     kind = SDNODE;
     u.s.Node = N;
     u.s.ResNo = R;
   }
 
   // Constructor for constants.
-  SDDbgValue(MDNode *Var, MDNode *Expr, const Value *C, DebugLoc dl, unsigned O)
+  SDDbgValue(DIVariable *Var, DIExpression *Expr, const Value *C, DebugLoc dl,
+             unsigned O)
       : Var(Var), Expr(Expr), DL(std::move(dl)), Order(O), IsIndirect(false) {
     kind = CONST;
     u.Const = C;
   }
 
   // Constructor for frame indices.
-  SDDbgValue(MDNode *Var, MDNode *Expr, unsigned FI, DebugLoc dl, unsigned O)
+  SDDbgValue(DIVariable *Var, DIExpression *Expr, unsigned FI, DebugLoc dl,
+             unsigned O)
       : Var(Var), Expr(Expr), DL(std::move(dl)), Order(O), IsIndirect(false) {
     kind = FRAMEIX;
     u.FrameIx = FI;
   }
 
   // Returns the kind.
   DbgValueKind getKind() const { return kind; }
 
-  // Returns the MDNode pointer for the variable.
-  MDNode *getVariable() const { return Var; }
+  // Returns the DIVariable pointer for the variable.
+  DIVariable *getVariable() const { return Var; }
 
-  // Returns the MDNode pointer for the expression.
-  MDNode *getExpression() const { return Expr; }
+  // Returns the DIExpression pointer for the expression.
+  DIExpression *getExpression() const { return Expr; }
 
   // Returns the SDNode* for a register ref
   SDNode *getSDNode() const { assert (kind==SDNODE); return u.s.Node; }
 
   // Returns the ResNo for a register ref
   unsigned getResNo() const { assert (kind==SDNODE); return u.s.ResNo; }
 
   // Returns the Value* for a constant

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

if (VTList.VTs[VTList.NumVTs - 1] != MVT::Glue) {
     }
   }
   return nullptr;
 }
 
 /// getDbgValue - Creates a SDDbgValue node.
 ///
 /// SDNode
-SDDbgValue *SelectionDAG::getDbgValue(MDNode *Var, MDNode *Expr, SDNode *N,
-                                      unsigned R, bool IsIndirect,
+SDDbgValue *SelectionDAG::getDbgValue(DIVariable *Var, DIExpression *Expr,
+                                      SDNode *N, unsigned R, bool IsIndirect,
                                       const DebugLoc &DL, unsigned O) {
   assert(cast<DILocalVariable>(Var)->isValidLocationForIntrinsic(DL) &&
          "Expected inlined-at fields to agree");
   return new (DbgInfo->getAlloc())
       SDDbgValue(Var, Expr, N, R, IsIndirect, DL, O);
 }
 
 /// Constant
-SDDbgValue *SelectionDAG::getConstantDbgValue(MDNode *Var, MDNode *Expr,
+SDDbgValue *SelectionDAG::getConstantDbgValue(DIVariable *Var,
+                                              DIExpression *Expr,
                                               const Value *C,
                                               const DebugLoc &DL, unsigned O) {
   assert(cast<DILocalVariable>(Var)->isValidLocationForIntrinsic(DL) &&
          "Expected inlined-at fields to agree");
   return new (DbgInfo->getAlloc()) SDDbgValue(Var, Expr, C, DL, O);
 }
 
 /// FrameIndex
-SDDbgValue *SelectionDAG::getFrameIndexDbgValue(MDNode *Var, MDNode *Expr,
-                                                unsigned FI, const DebugLoc &DL,
+SDDbgValue *SelectionDAG::getFrameIndexDbgValue(DIVariable *Var,
+                                                DIExpression *Expr, unsigned FI,
+                                                const DebugLoc &DL,
                                                 unsigned O) {
   assert(cast<DILocalVariable>(Var)->isValidLocationForIntrinsic(DL) &&
          "Expected inlined-at fields to agree");
   return new (DbgInfo->getAlloc()) SDDbgValue(Var, Expr, FI, DL, O);
 }
 
 namespace {
 
for (ArrayRef<SDDbgValue *>::iterator I = DVs.begin(), E = DVs.end();
        I != E; ++I) {
     SDDbgValue *Dbg = *I;
     // Only add Dbgvalues attached to same ResNo.
     if (Dbg->getKind() == SDDbgValue::SDNODE &&
         Dbg->getSDNode() == From.getNode() &&
         Dbg->getResNo() == From.getResNo() && !Dbg->isInvalidated()) {
       assert(FromNode != ToNode &&
              "Should not transfer Debug Values intranode");
-      SDDbgValue *Clone = getDbgValue(Dbg->getVariable(), Dbg->getExpression(),
-                                      ToNode, To.getResNo(), Dbg->isIndirect(),
-                                      Dbg->getDebugLoc(), Dbg->getOrder());
+      SDDbgValue *Clone = getDbgValue(
+          Dbg->getVariable(), Dbg->getExpression(), ToNode, Dbg->getResNo(),
+          Dbg->isIndirect(), Dbg->getDebugLoc(), Dbg->getOrder());
       ClonedDVs.push_back(Clone);
       Dbg->setIsInvalidated();
     }
   }
   for (SDDbgValue *I : ClonedDVs)
     AddDbgValue(I, ToNode, false);
 }
 

llvm/test/DebugInfo/MSP430/lit.local.cfg

+if not 'MSP430' in config.root.targets:
+    config.unsupported = True

llvm/test/DebugInfo/MSP430/sdagsplit-1.ll

+; RUN: llc %s -stop-after=livedebugvars -o %t
+; RUN: cat %t | FileCheck %s
+;
+; Test that we can emit debug info for large values that are split
+; up across multiple registers by the SelectionDAG type legalizer.
+;
+;    // Compile with -O1 -m32.
+;    long long foo (long long a, long long b)
+;    {
+;      long long res = b+1;
+;      if ( a == b )
+;        return res;
+;      return 0;
+;    }
+;
+; CHECK: ![[MDN1:[0-9]+]] = !DIExpression(DW_OP_LLVM_fragment, 32, 16)
+; CHECK: ![[MDN2:[0-9]+]] = !DIExpression(DW_OP_LLVM_fragment, 48, 16)
+; CHECK: ![[MDN3:[0-9]+]] = !DIExpression(DW_OP_LLVM_fragment, 0, 16)
+; CHECK: ![[MDN4:[0-9]+]] = !DIExpression(DW_OP_LLVM_fragment, 16, 16)
+; CHECK-DAG: DBG_VALUE debug-use %r{{[0-9]+}}, debug-use _, !{{[0-9]+}}, ![[MDN1]], debug-location !{{[0-9]+}}
+; CHECK-DAG: DBG_VALUE debug-use %r{{[0-9]+}}, debug-use _, !{{[0-9]+}}, ![[MDN2]], debug-location !{{[0-9]+}}
+; CHECK-DAG: DBG_VALUE debug-use %r{{[0-9]+}}, debug-use _, !{{[0-9]+}}, ![[MDN3]], debug-location !{{[0-9]+}}
+; CHECK-DAG: DBG_VALUE debug-use %r{{[0-9]+}}, debug-use _, !{{[0-9]+}}, ![[MDN4]], debug-location !{{[0-9]+}}
+
+; ModuleID = 'sdagsplit-1.c'
+target datalayout = "e-m:e-p:16:16-i32:16-i64:16-f32:16-f64:16-a:8-n8:16-S16"
+target triple = "msp430"
+
+; Function Attrs: nounwind readnone
+define i64 @foo(i64 %a, i64 %b) local_unnamed_addr #0 !dbg !7 {
+entry:
+  tail call void @llvm.dbg.value(metadata i64 %a, metadata !12, metadata !15), !dbg !16
+  tail call void @llvm.dbg.value(metadata i64 %b, metadata !13, metadata !15), !dbg !17
+  tail call void @llvm.dbg.value(metadata i64 %add, metadata !14, metadata !15), !dbg !18
+  %cmp = icmp eq i64 %a, %b, !dbg !19
+  %add = add nsw i64 %b, 1, !dbg !21
+  %retval.0 = select i1 %cmp, i64 %add, i64 0, !dbg !22
+  ret i64 %retval.0, !dbg !23
+}
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #0 = { nounwind readnone }
+attributes #1 = { nounwind readnone speculatable }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 6.0.0 ", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "sdagsplit-1.c", directory: "/MSP430")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 2}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{!"clang version 6.0.0 "}
+!7 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 2, type: !8, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !11)
+!8 = !DISubroutineType(types: !9)
+!9 = !{!10, !10, !10}
+!10 = !DIBasicType(name: "long long int", size: 64, encoding: DW_ATE_signed)
+!11 = !{!12, !13, !14}
+!12 = !DILocalVariable(name: "a", arg: 1, scope: !7, file: !1, line: 2, type: !10)
+!13 = !DILocalVariable(name: "b", arg: 2, scope: !7, file: !1, line: 2, type: !10)
+!14 = !DILocalVariable(name: "res", scope: !7, file: !1, line: 4, type: !10)
+!15 = !DIExpression()
+!16 = !DILocation(line: 2, column: 26, scope: !7)
+!17 = !DILocation(line: 2, column: 39, scope: !7)
+!18 = !DILocation(line: 4, column: 12, scope: !7)
+!19 = !DILocation(line: 5, column: 9, scope: !20)
+!20 = distinct !DILexicalBlock(scope: !7, file: !1, line: 5, column: 7)
+!21 = !DILocation(line: 4, column: 19, scope: !7)
+!22 = !DILocation(line: 5, column: 7, scope: !7)
+!23 = !DILocation(line: 8, column: 1, scope: !7)

llvm/test/DebugInfo/X86/sdagsplit-1.ll

+; RUN: llc %s -stop-after=livedebugvars -o %t
+; RUN: cat %t | FileCheck %s
+;
+; Test that we can emit debug info for large values that are split
+; up across multiple registers by the SelectionDAG type legalizer.
+;
+;    // Compile with -O1 -m32.
+;    long long foo (long long a, long long b)
+;    {
+;      long long res = b+1;
+;      if ( a == b )
+;        return res;
+;      return 0;
+;    }
+;
+; CHECK: ![[MDN1:[0-9]+]] = !DIExpression(DW_OP_LLVM_fragment, 0, 32)
+; CHECK: ![[MDN2:[0-9]+]] = !DIExpression(DW_OP_LLVM_fragment, 32, 32)
+; CHECK-DAG: DBG_VALUE debug-use %{{[a-z]+}}, debug-use _, !{{[0-9]+}}, ![[MDN1]], debug-location !{{[0-9]+}}
+; CHECK-DAG: DBG_VALUE debug-use %{{[a-z]+}}, debug-use _, !{{[0-9]+}}, ![[MDN2]], debug-location !{{[0-9]+}}
+
+; ModuleID = 'sdagsplit-1.c'
+target datalayout = "e-m:e-p:32:32-f64:32:64-f80:32-n8:16:32-S128"
+target triple = "i386"
+
+; Function Attrs: nounwind readnone
+define i64 @foo(i64 %a, i64 %b) local_unnamed_addr #0 !dbg !8 {
+entry:
+  tail call void @llvm.dbg.value(metadata i64 %a, metadata !13, metadata !16), !dbg !17
+  tail call void @llvm.dbg.value(metadata i64 %b, metadata !14, metadata !16), !dbg !18
+  tail call void @llvm.dbg.value(metadata i64 %add, metadata !15, metadata !16), !dbg !19
+  %cmp = icmp eq i64 %a, %b, !dbg !20
+  %add = add nsw i64 %b, 1, !dbg !22
+  %retval.0 = select i1 %cmp, i64 %add, i64 0, !dbg !23
+  ret i64 %retval.0, !dbg !24
+}
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #0 = { nounwind readnone }
+attributes #1 = { nounwind readnone speculatable }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 6.0.0 ", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "sdagsplit-1.c", directory: "/X86")
+!2 = !{}
+!3 = !{i32 1, !"NumRegisterParameters", i32 0}
+!4 = !{i32 2, !"Dwarf Version", i32 2}
+!5 = !{i32 2, !"Debug Info Version", i32 3}
+!6 = !{i32 1, !"wchar_size", i32 4}
+!7 = !{!"clang version 6.0.0 "}
+!8 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 2, type: !9, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !12)
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11, !11, !11}
+!11 = !DIBasicType(name: "long long int", size: 64, encoding: DW_ATE_signed)
+!12 = !{!13, !14, !15}
+!13 = !DILocalVariable(name: "a", arg: 1, scope: !8, file: !1, line: 2, type: !11)
+!14 = !DILocalVariable(name: "b", arg: 2, scope: !8, file: !1, line: 2, type: !11)
+!15 = !DILocalVariable(name: "res", scope: !8, file: !1, line: 4, type: !11)
+!16 = !DIExpression()
+!17 = !DILocation(line: 2, column: 26, scope: !8)
+!18 = !DILocation(line: 2, column: 39, scope: !8)
+!19 = !DILocation(line: 4, column: 12, scope: !8)
+!20 = !DILocation(line: 5, column: 9, scope: !21)
+!21 = distinct !DILexicalBlock(scope: !8, file: !1, line: 5, column: 7)
+!22 = !DILocation(line: 4, column: 19, scope: !8)
+!23 = !DILocation(line: 5, column: 7, scope: !8)
+!24 = !DILocation(line: 8, column: 1, scope: !8)