[DebugInfo][X86] Describe call site values for zero-valued imms

Summary:
Add zero-materializing XORs to X86's describeLoadedValue() hook in order
to produce call site values.

I have had to change the defs logic in collectCallSiteParameters() a bit
to be able to describe the XORs. The XORs implicitly define $eflags,
which would cause them to never be considered, due to a guard condition
that I->getNumDefs() is one. I have changed that condition so that we
now only consider instructions where a forwarded register overlaps with
the instruction's single explicit define. We still need to collect the implicit
defines of other forwarded registers to remove them from the work list.
I'm not sure how to move towards supporting instructions with multiple
explicit defines, cases where forwarded register are implicitly defined,
and/or cases where an instruction produces values for multiple forwarded
registers. Perhaps the describeLoadedValue() hook should take a register
argument, and we then leave it up to the hook to describe the loaded
value in that register? I have not yet encountered a situation where
that would be necessary though.

Reviewers: aprantl, vsk, djtodoro, NikolaPrica

Reviewed By: vsk

Subscribers: ychen, hiraditya, llvm-commits

Tags: #debug-info, #llvm

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

llvm-svn: 371333

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

@@ -596,12 +596,13 @@ static void collectCallSiteParameters(const MachineInstr *CallMI,
   bool ShouldTryEmitEntryVals = MBB->getIterator() == MF->begin();
   DenseMap<unsigned, unsigned> RegsForEntryValues;
 
-  // If the MI is an instruction defining a parameter's forwarding register,
-  // add it into the Defs. If the MI clobbers more then one register, we use
-  // the Defs in order to remove all the registers from
-  // the ForwardedRegWorklist, since we do not support such situations now.
+  // If the MI is an instruction defining one or more parameters' forwarding
+  // registers, add those defines. We can currently only describe forwarded
+  // registers that are explicitly defined, but keep track of implicit defines
+  // also to remove those registers from the work list.
   auto getForwardingRegsDefinedByMI = [&](const MachineInstr &MI,
-                                         SmallVectorImpl<unsigned> &Defs) {
+                                          SmallVectorImpl<unsigned> &Explicit,
+                                          SmallVectorImpl<unsigned> &Implicit) {
     if (MI.isDebugInstr())
       return;
 
@@ -610,7 +611,10 @@ static void collectCallSiteParameters(const MachineInstr *CallMI,
           Register::isPhysicalRegister(MO.getReg())) {
         for (auto FwdReg : ForwardedRegWorklist) {
           if (TRI->regsOverlap(FwdReg, MO.getReg())) {
-            Defs.push_back(FwdReg);
+            if (MO.isImplicit())
+              Implicit.push_back(FwdReg);
+            else
+              Explicit.push_back(FwdReg);
             break;
           }
         }
@@ -641,18 +645,24 @@ static void collectCallSiteParameters(const MachineInstr *CallMI,
     if (ForwardedRegWorklist.empty())
       return;
 
-    SmallVector<unsigned, 4> Defs;
-    getForwardingRegsDefinedByMI(*I, Defs);
-    if (Defs.empty())
+    SmallVector<unsigned, 4> ExplicitFwdRegDefs;
+    SmallVector<unsigned, 4> ImplicitFwdRegDefs;
+    getForwardingRegsDefinedByMI(*I, ExplicitFwdRegDefs, ImplicitFwdRegDefs);
+    if (ExplicitFwdRegDefs.empty() && ImplicitFwdRegDefs.empty())
       continue;
 
     // If the MI clobbers more then one forwarding register we must remove
     // all of them from the working list.
-    for (auto Reg : Defs)
+    for (auto Reg : concat<unsigned>(ExplicitFwdRegDefs, ImplicitFwdRegDefs))
       ForwardedRegWorklist.erase(Reg);
-    if (I->getNumDefs() != 1)
+
+    // The describeLoadedValue() hook currently does not have any information
+    // about which register it should describe in case of multiple defines, so
+    // for now we only handle instructions where a forwarded register is (at
+    // least partially) defined by the instruction's single explicit define.
+    if (I->getNumExplicitDefs() != 1 || ExplicitFwdRegDefs.empty())
       continue;
-    unsigned Reg = Defs[0];
+    unsigned Reg = ExplicitFwdRegDefs[0];
 
     if (auto ParamValue = TII->describeLoadedValue(*I)) {
       if (ParamValue->first.isImm()) {

llvm/lib/Target/X86/X86InstrInfo.cpp

@@ -7547,6 +7547,11 @@ X86InstrInfo::describeLoadedValue(const MachineInstr &MI) const {
 
     return ParamLoadedValue(*Op, Expr);;
   }
+  case X86::XOR32rr: {
+    if (MI.getOperand(1).getReg() == MI.getOperand(2).getReg())
+      return ParamLoadedValue(MachineOperand::CreateImm(0), Expr);
+    return None;
+  }
   default:
     return TargetInstrInfo::describeLoadedValue(MI);
   }

llvm/test/DebugInfo/X86/dbgcall-site-zero-valued-imms.ll

@@ -0,0 +1,68 @@
+; RUN: llc -O3 -debug-entry-values -filetype=obj -o - %s | llvm-dwarfdump - | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Verify that we produce call site entries for the zero-valued parameters.
+;
+; Based on the following reproducer:
+;
+; #include <stdint.h>
+; extern void callee(uint8_t, uint16_t, uint32_t, void *);
+; int caller() {
+;   callee(0, 0, 0, (void *)0);
+;   return 1;
+; }
+
+; CHECK: DW_TAG_GNU_call_site_parameter
+; CHECK-NEXT: DW_AT_location	(DW_OP_reg2 RCX)
+; CHECK-NEXT: DW_AT_GNU_call_site_value	(DW_OP_lit0)
+
+; CHECK: DW_TAG_GNU_call_site_parameter
+; CHECK-NEXT: DW_AT_location	(DW_OP_reg1 RDX)
+; CHECK-NEXT: DW_AT_GNU_call_site_value	(DW_OP_lit0)
+
+; CHECK: DW_TAG_GNU_call_site_parameter
+; CHECK-NEXT: DW_AT_location	(DW_OP_reg4 RSI)
+; CHECK-NEXT: DW_AT_GNU_call_site_value	(DW_OP_lit0)
+
+; CHECK: DW_TAG_GNU_call_site_parameter
+; CHECK-NEXT: DW_AT_location	(DW_OP_reg5 RDI)
+; CHECK-NEXT: DW_AT_GNU_call_site_value	(DW_OP_lit0)
+
+; Function Attrs: nounwind uwtable
+define i32 @caller() #0 !dbg !15 {
+entry:
+  tail call void @callee(i8 zeroext 0, i16 zeroext 0, i32 0, i8* null), !dbg !19
+  ret i32 1, !dbg !20
+}
+
+declare !dbg !5 void @callee(i8 zeroext, i16 zeroext, i32, i8*)
+
+attributes #0 = { nounwind uwtable }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!11, !12, !13}
+!llvm.ident = !{!14}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 10.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, nameTableKind: None)
+!1 = !DIFile(filename: "zero.c", directory: "/")
+!2 = !{}
+!3 = !{!4, !5}
+!4 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: null, size: 64)
+!5 = !DISubprogram(name: "callee", scope: !1, file: !1, line: 2, type: !6, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
+!6 = !DISubroutineType(types: !7)
+!7 = !{null, !8, !9, !10, !4}
+!8 = !DIBasicType(name: "unsigned char", size: 8, encoding: DW_ATE_unsigned_char)
+!9 = !DIBasicType(name: "unsigned short", size: 16, encoding: DW_ATE_unsigned)
+!10 = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
+!11 = !{i32 2, !"Dwarf Version", i32 4}
+!12 = !{i32 2, !"Debug Info Version", i32 3}
+!13 = !{i32 1, !"wchar_size", i32 4}
+!14 = !{!"clang version 10.0.0"}
+!15 = distinct !DISubprogram(name: "caller", scope: !1, file: !1, line: 3, type: !16, scopeLine: 3, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!16 = !DISubroutineType(types: !17)
+!17 = !{!18}
+!18 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!19 = !DILocation(line: 4, scope: !15)
+!20 = !DILocation(line: 5, scope: !15)