[DebugInfo] Describe call site values for chains of expression produc…

…ing instrs

Summary:
If the describeLoadedValue() hook produced a DIExpression when
describing a instruction, and it was not possible to emit a call site
entry directly (the value operand was not an immediate nor a preserved
register), then that described value could not be inserted into the
worklist, and would instead be dropped, meaning that the parameter's
call site value couldn't be described.

This patch extends the worklist so that each entry has an DIExpression
that is built up when iterating through the instructions.

This allows us to describe instruction chains like this:

  $reg0 = mv $fp
  $reg0 = add $reg0, offset
  call @call_with_offseted_fp

Since DW_OP_LLVM_entry_value operations can't be combined with any other
expression, such call site entries will not be emitted. I have added a
test, dbgcall-site-expr-entry-value.mir, which verifies that we don't
assert or emit broken DWARF in such cases.

Reviewers: djtodoro, aprantl, vsk

Reviewed By: djtodoro, vsk

Subscribers: hiraditya, llvm-commits

Tags: #debug-info, #llvm

Differential Revision: https://reviews.llvm.org/D75036

(cherry picked from commit 6d85716)

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

@@ -558,34 +558,78 @@ DIE &DwarfDebug::constructSubprogramDefinitionDIE(const DISubprogram *SP) {
   return *CU.getOrCreateSubprogramDIE(SP);
 }
 
+/// Represents a parameter whose call site value can be described by applying a
+/// debug expression to a register in the forwarded register worklist.
+struct FwdRegParamInfo {
+  /// The described parameter register.
+  unsigned ParamReg;
+
+  /// Debug expression that has been built up when walking through the
+  /// instruction chain that produces the parameter's value.
+  const DIExpression *Expr;
+};
+
 /// Register worklist for finding call site values.
-using FwdRegWorklist = MapVector<unsigned, SmallVector<unsigned, 2>>;
+using FwdRegWorklist = MapVector<unsigned, SmallVector<FwdRegParamInfo, 2>>;
 
 /// Emit call site parameter entries that are described by the given value and
 /// debug expression.
 template <typename ValT>
 static void finishCallSiteParams(ValT Val, const DIExpression *Expr,
-                                 ArrayRef<unsigned> DescribedParams,
+                                 ArrayRef<FwdRegParamInfo> DescribedParams,
                                  ParamSet &Params) {
-  DbgValueLoc DbgLocVal(Expr, Val);
-  for (auto ParamReg : DescribedParams) {
-    DbgCallSiteParam CSParm(ParamReg, DbgLocVal);
+  for (auto Param : DescribedParams) {
+    bool ShouldCombineExpressions = Expr && Param.Expr->getNumElements() > 0;
+
+    // TODO: Entry value operations can currently not be combined with any
+    // other expressions, so we can't emit call site entries in those cases.
+    if (ShouldCombineExpressions && Expr->isEntryValue())
+      continue;
+
+    // If a parameter's call site value is produced by a chain of
+    // instructions we may have already created an expression for the
+    // parameter when walking through the instructions. Append that to the
+    // base expression.
+    const DIExpression *CombinedExpr =
+        ShouldCombineExpressions
+            ? DIExpression::append(Expr, Param.Expr->getElements())
+            : Expr;
+    assert((!CombinedExpr || CombinedExpr->isValid()) &&
+           "Combined debug expression is invalid");
+
+    DbgValueLoc DbgLocVal(CombinedExpr, Val);
+    DbgCallSiteParam CSParm(Param.ParamReg, DbgLocVal);
     Params.push_back(CSParm);
     ++NumCSParams;
   }
 }
 
 /// Add \p Reg to the worklist, if it's not already present, and mark that the
 /// given parameter registers' values can (potentially) be described using
-/// that register.
+/// that register and an debug expression.
 static void addToFwdRegWorklist(FwdRegWorklist &Worklist, unsigned Reg,
-                                ArrayRef<unsigned> ParamsToAdd) {
+                                const DIExpression *Expr,
+                                ArrayRef<FwdRegParamInfo> ParamsToAdd) {
   auto I = Worklist.insert({Reg, {}});
   auto &ParamsForFwdReg = I.first->second;
-  for (auto ParamReg : ParamsToAdd) {
-    assert(!is_contained(ParamsForFwdReg, ParamReg) &&
+  for (auto Param : ParamsToAdd) {
+    assert(none_of(ParamsForFwdReg,
+                   [Param](const FwdRegParamInfo &D) {
+                     return D.ParamReg == Param.ParamReg;
+                   }) &&
            "Same parameter described twice by forwarding reg");
-    ParamsForFwdReg.push_back(ParamReg);
+
+    // If a parameter's call site value is produced by a chain of
+    // instructions we may have already created an expression for the
+    // parameter when walking through the instructions. Append that to the
+    // new expression.
+    const DIExpression *CombinedExpr =
+        (Param.Expr->getNumElements() > 0)
+            ? DIExpression::append(Expr, Param.Expr->getElements())
+            : Expr;
+    assert(CombinedExpr->isValid() && "Combined debug expression is invalid");
+
+    ParamsForFwdReg.push_back({Param.ParamReg, CombinedExpr});
   }
 }
 
@@ -632,10 +676,14 @@ static void collectCallSiteParameters(const MachineInstr *CallMI,
   // instruction has been handled.
   FwdRegWorklist NewWorklistItems;
 
+  const DIExpression *EmptyExpr =
+      DIExpression::get(MF->getFunction().getContext(), {});
+
   // Add all the forwarding registers into the ForwardedRegWorklist.
   for (auto ArgReg : CallFwdRegsInfo->second) {
     bool InsertedReg =
-        ForwardedRegWorklist.insert({ArgReg.Reg, {{ArgReg.Reg}}}).second;
+        ForwardedRegWorklist.insert({ArgReg.Reg, {{ArgReg.Reg, EmptyExpr}}})
+            .second;
     assert(InsertedReg && "Single register used to forward two arguments?");
     (void)InsertedReg;
   }
@@ -696,30 +744,21 @@ static void collectCallSiteParameters(const MachineInstr *CallMI,
                                ForwardedRegWorklist[ParamFwdReg], Params);
         } else if (ParamValue->first.isReg()) {
           Register RegLoc = ParamValue->first.getReg();
-          // TODO: For now, there is no use of describing the value loaded into the
-          //       register that is also the source registers (e.g. $r0 = add $r0, x).
-          if (ParamFwdReg == RegLoc)
-            continue;
-
           unsigned SP = TLI->getStackPointerRegisterToSaveRestore();
           Register FP = TRI->getFrameRegister(*MF);
           bool IsSPorFP = (RegLoc == SP) || (RegLoc == FP);
           if (TRI->isCalleeSavedPhysReg(RegLoc, *MF) || IsSPorFP) {
             MachineLocation MLoc(RegLoc, /*IsIndirect=*/IsSPorFP);
             finishCallSiteParams(MLoc, ParamValue->second,
                                  ForwardedRegWorklist[ParamFwdReg], Params);
-          // TODO: Add support for entry value plus an expression.
-          } else if (ShouldTryEmitEntryVals &&
-                     ParamValue->second->getNumElements() == 0) {
-            assert(RegLoc != ParamFwdReg &&
-                   "Can't handle a register that is described by itself");
+          } else {
             // ParamFwdReg was described by the non-callee saved register
             // RegLoc. Mark that the call site values for the parameters are
             // dependent on that register instead of ParamFwdReg. Since RegLoc
             // may be a register that will be handled in this iteration, we
             // postpone adding the items to the worklist, and instead keep them
             // in a temporary container.
-            addToFwdRegWorklist(NewWorklistItems, RegLoc,
+            addToFwdRegWorklist(NewWorklistItems, RegLoc, ParamValue->second,
                                 ForwardedRegWorklist[ParamFwdReg]);
           }
         }
@@ -733,7 +772,8 @@ static void collectCallSiteParameters(const MachineInstr *CallMI,
     // Now that we are done handling this instruction, add items from the
     // temporary worklist to the real one.
     for (auto New : NewWorklistItems)
-      addToFwdRegWorklist(ForwardedRegWorklist, New.first, New.second);
+      addToFwdRegWorklist(ForwardedRegWorklist, New.first, EmptyExpr,
+                          New.second);
     NewWorklistItems.clear();
   }
 

llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-expr-chain.mir

@@ -0,0 +1,116 @@
+# RUN: llc -mtriple aarch64-linux-gnu -debug-entry-values -start-after=livedebugvalues -filetype=obj -o - %s \
+# RUN:     | llvm-dwarfdump - | FileCheck %s --implicit-check-not=DW_TAG_GNU_call_site_parameter
+#
+# Based on the following C reproducer:
+#
+#   extern void call(long, long, long);
+#   extern long global;
+#
+#   long foo() {
+#     long local = global;
+#     call(local + 123, local - 456, local + 789);
+#     return local;
+#   }
+
+--- |
+  target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+  target triple = "aarch64"
+
+  @global = external dso_local local_unnamed_addr global i64, align 8
+
+  define dso_local i64 @foo() local_unnamed_addr !dbg !12 {
+  entry:
+    %0 = load i64, i64* @global, align 8, !dbg !17
+    call void @llvm.dbg.value(metadata i64 %0, metadata !16, metadata !DIExpression()), !dbg !17
+    %add = add nsw i64 %0, 123, !dbg !17
+    %sub = add nsw i64 %0, -456, !dbg !17
+    %add1 = add nsw i64 %0, 789, !dbg !17
+    call void @call(i64 %add, i64 %sub, i64 %add1), !dbg !17
+    ret i64 %0, !dbg !17
+  }
+
+  declare !dbg !4 dso_local void @call(i64, i64, i64) local_unnamed_addr
+
+  declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!8, !9, !10}
+  !llvm.ident = !{!11}
+
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 11.0.0 ", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, splitDebugInlining: false, nameTableKind: None)
+  !1 = !DIFile(filename: "dbgcall-site-expr-chain.c", directory: "/")
+  !2 = !{}
+  !3 = !{!4}
+  !4 = !DISubprogram(name: "call", scope: !1, file: !1, line: 1, type: !5, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
+  !5 = !DISubroutineType(types: !6)
+  !6 = !{null, !7, !7, !7}
+  !7 = !DIBasicType(name: "long int", size: 64, encoding: DW_ATE_signed)
+  !8 = !{i32 7, !"Dwarf Version", i32 4}
+  !9 = !{i32 2, !"Debug Info Version", i32 3}
+  !10 = !{i32 1, !"wchar_size", i32 4}
+  !11 = !{!"clang version 11.0.0 "}
+  !12 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 5, type: !13, scopeLine: 5, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !15)
+  !13 = !DISubroutineType(types: !14)
+  !14 = !{!7}
+  !15 = !{!16}
+  !16 = !DILocalVariable(name: "local", scope: !12, file: !1, line: 6, type: !7)
+  !17 = !DILocation(line: 6, scope: !12)
+
+...
+---
+name:            foo
+tracksRegLiveness: true
+stack:
+  - { id: 0, type: spill-slot, offset: -16, size: 8, alignment: 16, callee-saved-register: '$x19' }
+  - { id: 1, type: spill-slot, offset: -24, size: 8, alignment: 8, callee-saved-register: '$lr' }
+  - { id: 2, type: spill-slot, offset: -32, size: 8, alignment: 8, callee-saved-register: '$fp' }
+callSites:
+  - { bb: 0, offset: 17, fwdArgRegs:
+      - { arg: 0, reg: '$x0' }
+      - { arg: 1, reg: '$x1' }
+      - { arg: 2, reg: '$x2' } }
+body:             |
+  bb.0.entry:
+    liveins: $lr, $x19
+
+    early-clobber $sp = frame-setup STPXpre $fp, killed $lr, $sp, -4 :: (store 8 into %stack.2), (store 8 into %stack.1)
+    frame-setup STRXui killed $x19, $sp, 2 :: (store 8 into %stack.0)
+    $fp = frame-setup ADDXri $sp, 0, 0
+    frame-setup CFI_INSTRUCTION def_cfa $w29, 32
+    frame-setup CFI_INSTRUCTION offset $w19, -16, debug-location !17
+    frame-setup CFI_INSTRUCTION offset $w30, -24, debug-location !17
+    frame-setup CFI_INSTRUCTION offset $w29, -32, debug-location !17
+    renamable $x8 = ADRP target-flags(aarch64-page) @global, debug-location !17
+    renamable $x19 = LDRXui killed renamable $x8, target-flags(aarch64-pageoff, aarch64-nc) @global, debug-location !17 :: (dereferenceable load 8 from @global)
+    DBG_VALUE $x19, $noreg, !16, !DIExpression(), debug-location !17
+    renamable $x0 = nsw ADDXri renamable $x19, 100, 0, debug-location !17
+    renamable $x8 = nsw SUBXri renamable $x19, 406, 0, debug-location !17
+    renamable $x0 = nsw ADDXri renamable $x0, 23, 0, debug-location !17
+    renamable $x1 = nsw SUBXri renamable $x8, 50, 0, debug-location !17
+    renamable $x2 = nsw ADDXri renamable $x19, 700, 0, debug-location !17
+    renamable $x2 = nsw ADDXri renamable $x2, 9, 0, debug-location !17
+    renamable $x2 = nsw ADDXri renamable $x2, 80, 0, debug-location !17
+    BL @call, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $x0, implicit killed $x1, implicit killed $x2, implicit-def $sp, debug-location !17
+    $x0 = ORRXrs $xzr, killed $x19, 0, debug-location !17
+    $x19 = frame-destroy LDRXui $sp, 2, debug-location !17 :: (load 8 from %stack.0)
+    early-clobber $sp, $fp, $lr = frame-destroy LDPXpost $sp, 4, debug-location !17 :: (load 8 from %stack.2), (load 8 from %stack.1)
+    RET undef $lr, implicit killed $x0, debug-location !17
+
+...
+
+# Verify that call site entries are emitted for all three parameters.
+#
+# The MIR has been hand-modified to build up the call site values using chains
+# of ADDXri/SUBXri instructions instead of a single instruction per parameter.
+
+# CHECK: DW_TAG_GNU_call_site_parameter
+# CHECK-NEXT: DW_AT_location (DW_OP_reg2 W2)
+# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_breg19 W19+700, DW_OP_plus_uconst 0x9, DW_OP_plus_uconst 0x50)
+
+# CHECK: DW_TAG_GNU_call_site_parameter
+# CHECK-NEXT: DW_AT_location (DW_OP_reg1 W1)
+# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_breg19 W19-406, DW_OP_constu 0x32, DW_OP_minus)
+
+# CHECK: DW_TAG_GNU_call_site_parameter
+# CHECK-NEXT: DW_AT_location (DW_OP_reg0 W0)
+# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_breg19 W19+100, DW_OP_plus_uconst 0x17)

llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-expr-entry-value.mir

@@ -0,0 +1,92 @@
+# RUN: llc -mtriple aarch64-linux-gnu -debug-entry-values -start-after=livedebugvalues -filetype=obj -o - %s \
+# RUN:     | llvm-dwarfdump - | FileCheck %s --implicit-check-not=DW_TAG_GNU_call_site_parameter
+#
+# Based on the following C reproducer:
+#
+# extern void call(long, long, long);
+#
+# void entry_value (long param) {
+#   call(param + 222, param - 444, param);
+# }
+
+--- |
+  target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+  target triple = "aarch64"
+
+  define dso_local void @entry_value(i64 %param) local_unnamed_addr !dbg !12 {
+  entry:
+    call void @llvm.dbg.value(metadata i64 %param, metadata !16, metadata !DIExpression()), !dbg !17
+    %add = add nsw i64 %param, 222, !dbg !17
+    %sub = add nsw i64 %param, -444, !dbg !17
+    call void @call(i64 %add, i64 %sub, i64 %param), !dbg !17
+    ret void, !dbg !17
+  }
+
+  declare !dbg !4 dso_local void @call(i64, i64, i64) local_unnamed_addr
+
+  declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!8, !9, !10}
+  !llvm.ident = !{!11}
+
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 11.0.0 ", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, splitDebugInlining: false, nameTableKind: None)
+  !1 = !DIFile(filename: "dbgcall-site-expr-entry-value.mir", directory: "/")
+  !2 = !{}
+  !3 = !{!4}
+  !4 = !DISubprogram(name: "call", scope: !1, file: !1, line: 1, type: !5, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
+  !5 = !DISubroutineType(types: !6)
+  !6 = !{null, !7, !7, !7}
+  !7 = !DIBasicType(name: "long int", size: 64, encoding: DW_ATE_signed)
+  !8 = !{i32 7, !"Dwarf Version", i32 4}
+  !9 = !{i32 2, !"Debug Info Version", i32 3}
+  !10 = !{i32 1, !"wchar_size", i32 4}
+  !11 = !{!"clang version 11.0.0 "}
+  !12 = distinct !DISubprogram(name: "entry_value", scope: !1, file: !1, line: 3, type: !13, scopeLine: 3, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !15)
+  !13 = !DISubroutineType(types: !14)
+  !14 = !{null, !7}
+  !15 = !{!16}
+  !16 = !DILocalVariable(name: "param", arg: 1, scope: !12, file: !1, line: 3, type: !7)
+  !17 = !DILocation(line: 4, scope: !12)
+
+...
+---
+name:            entry_value
+tracksRegLiveness: true
+stack:
+  - { id: 0, type: spill-slot, offset: -8, size: 8, alignment: 8, callee-saved-register: '$lr' }
+  - { id: 1, type: spill-slot, offset: -16, size: 8, alignment: 8, callee-saved-register: '$fp' }
+callSites:
+  - { bb: 0, offset: 10, fwdArgRegs:
+      - { arg: 0, reg: '$x0' }
+      - { arg: 1, reg: '$x1' }
+      - { arg: 2, reg: '$x2' } }
+body:             |
+  bb.0.entry:
+    liveins: $x0, $lr
+
+    DBG_VALUE $x0, $noreg, !16, !DIExpression(), debug-location !17
+    early-clobber $sp = frame-setup STPXpre $fp, killed $lr, $sp, -2 :: (store 8 into %stack.1), (store 8 into %stack.0)
+    $fp = frame-setup ADDXri $sp, 0, 0
+    frame-setup CFI_INSTRUCTION def_cfa $w29, 16
+    frame-setup CFI_INSTRUCTION offset $w30, -8
+    frame-setup CFI_INSTRUCTION offset $w29, -16
+    $x2 = ORRXrs $xzr, $x0, 0
+    DBG_VALUE $x2, $noreg, !16, !DIExpression(), debug-location !17
+    renamable $x0 = nsw ADDXri killed $x0, 222, 0, debug-location !17
+    renamable $x1 = nsw SUBXri renamable $x2, 444, 0, debug-location !17
+    BL @call, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $x0, implicit killed $x1, implicit killed $x2, implicit-def $sp, debug-location !17
+    early-clobber $sp, $fp, $lr = frame-destroy LDPXpost $sp, 2, debug-location !17 :: (load 8 from %stack.1), (load 8 from %stack.0)
+    RET undef $lr, debug-location !17
+
+...
+
+# Verify that a call site parameter is emitted for the third parameter. There
+# should also be entries for the first and second parameter, but
+# DW_OP_LLVM_entry_value operations can currently not be emitted together with
+# any other expressions. Verify that nothing is emitted rather than an assert
+# being triggered, or broken expressions being emitted.
+
+# CHECK: DW_TAG_GNU_call_site_parameter
+# CHECK-NEXT: DW_AT_location      (DW_OP_reg2 W2)
+# CHECK-NEXT: DW_AT_GNU_call_site_value   (DW_OP_GNU_entry_value(DW_OP_reg0 W0))