diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp --- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp @@ -560,10 +560,38 @@ // Skip the call instruction. auto I = std::next(CallMI->getReverseIterator()); - DenseSet ForwardedRegWorklist; + // Register worklist. Each register has an associated list of parameter + // registers whose call site values potentially can be described using that + // register. + using FwdRegWorklist = MapVector>; + + FwdRegWorklist ForwardedRegWorklist; + + // If an instruction defines more than one item in the worklist, we may run + // into situations where a worklist register's value is (potentially) + // described by the previous value of another register that is also defined + // by that instruction. + // + // This can for example occur in cases like this: + // + // $r1 = mov 123 + // $r0, $r1 = mvrr $r1, 456 + // call @foo, $r0, $r1 + // + // When describing $r1's value for the mvrr instruction, we need to make sure + // that we don't finalize an entry value for $r0, as that is dependent on the + // previous value of $r1 (123 rather than 456). + // + // In order to not have to distinguish between those cases when finalizing + // entry values, we simply postpone adding new parameter registers to the + // worklist, by first keeping them in this temporary container until the + // instruction has been handled. + FwdRegWorklist NewWorklistItems; + // Add all the forwarding registers into the ForwardedRegWorklist. for (auto ArgReg : CallFwdRegsInfo->second) { - bool InsertedReg = ForwardedRegWorklist.insert(ArgReg.Reg).second; + bool InsertedReg = + ForwardedRegWorklist.insert({ArgReg.Reg, {ArgReg.Reg}}).second; assert(InsertedReg && "Single register used to forward two arguments?"); (void)InsertedReg; } @@ -574,12 +602,9 @@ // list, for which we do not describe a loaded value by // the describeLoadedValue(), we try to generate an entry value expression // for their call site value description, if the call is within the entry MBB. - // The RegsForEntryValues maps a forwarding register into the register holding - // the entry value. // TODO: Handle situations when call site parameter value can be described // as the entry value within basic blocks other than the first one. bool ShouldTryEmitEntryVals = MBB->getIterator() == MF->begin(); - DenseMap RegsForEntryValues; // If the MI is an instruction defining one or more parameters' forwarding // registers, add those defines. @@ -592,23 +617,32 @@ if (MO.isReg() && MO.isDef() && Register::isPhysicalRegister(MO.getReg())) { for (auto FwdReg : ForwardedRegWorklist) - if (TRI->regsOverlap(FwdReg, MO.getReg())) - Defs.insert(FwdReg); + if (TRI->regsOverlap(FwdReg.first, MO.getReg())) + Defs.insert(FwdReg.first); } } }; - auto finishCallSiteParam = [&](DbgValueLoc DbgLocVal, unsigned Reg) { - unsigned FwdReg = Reg; - if (ShouldTryEmitEntryVals) { - auto EntryValReg = RegsForEntryValues.find(Reg); - if (EntryValReg != RegsForEntryValues.end()) - FwdReg = EntryValReg->second; + auto finishCallSiteParams = [&](DbgValueLoc DbgLocVal, + ArrayRef ParamRegs) { + for (auto FwdReg : ParamRegs) { + DbgCallSiteParam CSParm(FwdReg, DbgLocVal); + Params.push_back(CSParm); + ++NumCSParams; } + }; - DbgCallSiteParam CSParm(FwdReg, DbgLocVal); - Params.push_back(CSParm); - ++NumCSParams; + // Add 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. + auto addToWorklist = [](FwdRegWorklist &Worklist, unsigned Reg, + ArrayRef ParamRegs) { + auto I = Worklist.insert({Reg, {}}); + for (auto ParamReg : ParamRegs) { + assert(!is_contained(I.first->second, ParamReg) && + "Same parameter described twice by forwarding reg"); + I.first->second.push_back(ParamReg); + } }; // Search for a loading value in forwarding registers. @@ -632,17 +666,12 @@ if (FwdRegDefs.empty()) continue; - // If the MI clobbers more then one forwarding register we must remove - // all of them from the working list. - for (auto Reg : FwdRegDefs) - ForwardedRegWorklist.erase(Reg); - for (auto ParamFwdReg : FwdRegDefs) { if (auto ParamValue = TII->describeLoadedValue(*I, ParamFwdReg)) { if (ParamValue->first.isImm()) { int64_t Val = ParamValue->first.getImm(); DbgValueLoc DbgLocVal(ParamValue->second, Val); - finishCallSiteParam(DbgLocVal, ParamFwdReg); + finishCallSiteParams(DbgLocVal, ForwardedRegWorklist[ParamFwdReg]); } else if (ParamValue->first.isReg()) { Register RegLoc = ParamValue->first.getReg(); // TODO: For now, there is no use of describing the value loaded into the @@ -657,16 +686,34 @@ DbgValueLoc DbgLocVal(ParamValue->second, MachineLocation(RegLoc, /*IsIndirect=*/IsSPorFP)); - finishCallSiteParam(DbgLocVal, ParamFwdReg); + finishCallSiteParams(DbgLocVal, ForwardedRegWorklist[ParamFwdReg]); // TODO: Add support for entry value plus an expression. } else if (ShouldTryEmitEntryVals && ParamValue->second->getNumElements() == 0) { - ForwardedRegWorklist.insert(RegLoc); - RegsForEntryValues[RegLoc] = ParamFwdReg; + assert(RegLoc != ParamFwdReg && + "Can't handle a register that is described by itself"); + // 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. + addToWorklist(NewWorklistItems, RegLoc, + ForwardedRegWorklist[ParamFwdReg]); } } } } + + // Remove all registers that this instruction defines from the worklist. + for (auto ParamFwdReg : FwdRegDefs) + ForwardedRegWorklist.erase(ParamFwdReg); + + // Now that we are done handling this instruction, add items from the + // temporary worklist to the real one. + for (auto New : NewWorklistItems) + addToWorklist(ForwardedRegWorklist, New.first, New.second); + NewWorklistItems.clear(); } // Emit the call site parameter's value as an entry value. @@ -675,15 +722,8 @@ DIExpression *EntryExpr = DIExpression::get( MF->getFunction().getContext(), {dwarf::DW_OP_LLVM_entry_value, 1}); for (auto RegEntry : ForwardedRegWorklist) { - unsigned FwdReg = RegEntry; - auto EntryValReg = RegsForEntryValues.find(RegEntry); - if (EntryValReg != RegsForEntryValues.end()) - FwdReg = EntryValReg->second; - - DbgValueLoc DbgLocVal(EntryExpr, MachineLocation(RegEntry)); - DbgCallSiteParam CSParm(FwdReg, DbgLocVal); - Params.push_back(CSParm); - ++NumCSParams; + DbgValueLoc DbgLocVal(EntryExpr, MachineLocation(RegEntry.first)); + finishCallSiteParams(DbgLocVal, RegEntry.second); } } } diff --git a/llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-orr-moves.mir b/llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-orr-moves.mir --- a/llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-orr-moves.mir +++ b/llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-orr-moves.mir @@ -268,6 +268,5 @@ # CHECK-NEXT: DW_AT_abstract_origin (0x00000052 "call_int_int") # # CHECK: DW_TAG_GNU_call_site_parameter -# FIXME: The DW_AT_location attribute should actually refer to W0! See PR44118. -# CHECK-NEXT: DW_AT_location (DW_OP_reg8 W8) +# CHECK-NEXT: DW_AT_location (DW_OP_reg0 W0) # CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_GNU_entry_value(DW_OP_reg1 W1)) diff --git a/llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir b/llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir --- a/llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir +++ b/llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir @@ -13,6 +13,10 @@ # CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_fbreg +8) # CHECK-EMPTY: # CHECK-NEXT: 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_GNU_entry_value(DW_OP_reg4 RSI)) +# CHECK-EMPTY: +# CHECK-NEXT: 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_GNU_entry_value(DW_OP_reg4 RSI)) # CHECK-EMPTY: diff --git a/llvm/test/DebugInfo/MIR/X86/dbgcall-site-partial-describe.mir b/llvm/test/DebugInfo/MIR/X86/dbgcall-site-partial-describe.mir new file mode 100644 --- /dev/null +++ b/llvm/test/DebugInfo/MIR/X86/dbgcall-site-partial-describe.mir @@ -0,0 +1,82 @@ +# RUN: llc -debug-entry-values -start-before=livedebugvalues -filetype=obj -o - %s \ +# RUN: | llvm-dwarfdump - | FileCheck %s --implicit-check-not=DW_TAG_GNU_call_site_parameter + +--- | + target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" + target triple = "x86_64-unknown-linux-gnu" + + ; Function Attrs: nounwind uwtable + define void @foo() #0 !dbg !12 { + entry: + call void @call(i32 123, i32 undef), !dbg !15 + ret void, !dbg !16 + } + + declare !dbg !4 void @call(i32, i32) + + attributes #0 = { nounwind uwtable } + + !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: "clobber.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 = !DIBasicType(name: "int", size: 32, 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: 3, type: !13, scopeLine: 3, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2) + !13 = !DISubroutineType(types: !14) + !14 = !{null} + !15 = !DILocation(line: 5, scope: !12) + !16 = !DILocation(line: 6, scope: !12) + +... +--- +name: foo +callSites: + - { bb: 0, offset: 4, fwdArgRegs: + - { arg: 0, reg: '$edi' } + - { arg: 1, reg: '$esi' } } +body: | + bb.0.entry: + frame-setup PUSH64r undef $rax, implicit-def $rsp, implicit $rsp + CFI_INSTRUCTION def_cfa_offset 16 + $esi = MOV32ri 123, debug-location !15 + $edi = MOV32rr $esi, implicit-def $esi, debug-location !15 + CALL64pcrel32 @call, csr_64, implicit $rsp, implicit $ssp, implicit killed $edi, implicit undef $esi, implicit-def $rsp, implicit-def $ssp, debug-location !15 + $rax = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !16 + CFI_INSTRUCTION def_cfa_offset 8, debug-location !16 + RETQ debug-location !16 + +... + +# In this test an implicit-def has been added to the MOV32rr instruction to +# simulate a situation where one instruction defines two call site worklist +# registers, and where we end up with one of the registers being described by +# the previous value of the other register that is clobbered by this +# instruction. +# +# In this reproducer we should end up with only one call site entry, with that +# being for $rdi. +# +# This test uses an implicit CHECK-NOT to verify that only one call site +# parameter entry is emitted. +# +# A somewhat more realistic scenario would for example be the following in a +# made-up ISA: +# +# $r1 = mv 123 +# $r0, $r1 = mvri $r1, +# call @foo, $r0, $r1 + +# 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_constu 0x7b) diff --git a/llvm/test/DebugInfo/MIR/X86/dbgcall-site-reg-shuffle.mir b/llvm/test/DebugInfo/MIR/X86/dbgcall-site-reg-shuffle.mir new file mode 100644 --- /dev/null +++ b/llvm/test/DebugInfo/MIR/X86/dbgcall-site-reg-shuffle.mir @@ -0,0 +1,93 @@ +# RUN: llc -debug-entry-values -start-before=livedebugvalues -filetype=obj -o - %s \ +# RUN: | llvm-dwarfdump - | FileCheck %s --implicit-check-not=DW_TAG_GNU_call_site_parameter + +--- | + target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" + target triple = "x86_64-unknown-linux-gnu" + + ; Function Attrs: nounwind uwtable + define void @move_around_args(i32 %a) #0 !dbg !12 { + entry: + call void @call2(i32 123, i32 %a), !dbg !15 + ret void, !dbg !16 + } + + declare !dbg !4 dso_local void @call2(i32, i32) + + attributes #0 = { nounwind uwtable } + + !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: "worklist.c", directory: "/") + !2 = !{} + !3 = !{!4} + !4 = !DISubprogram(name: "call2", scope: !1, file: !1, line: 1, type: !5, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2) + !5 = !DISubroutineType(types: !6) + !6 = !{null, !7, !7} + !7 = !DIBasicType(name: "int", size: 32, 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: "move_around_args", scope: !1, file: !1, line: 3, type: !13, scopeLine: 3, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2) + !13 = !DISubroutineType(types: !14) + !14 = !{null, !7} + !15 = !DILocation(line: 4, scope: !12) + !16 = !DILocation(line: 5, scope: !12) + +... +--- +name: move_around_args +liveins: + - { reg: '$edi' } +callSites: + - { bb: 0, offset: 12, fwdArgRegs: + - { arg: 0, reg: '$edi' } + - { arg: 1, reg: '$esi' } } +body: | + bb.0.entry: + liveins: $edi + + frame-setup PUSH64r undef $rax, implicit-def $rsp, implicit $rsp + CFI_INSTRUCTION def_cfa_offset 16 + $esi = MOV32ri 123 + + ; Move the values around between different registers. + + $edx = MOV32rr $edi + + $edi = MOV32rr $esi + $esi = MOV32rr $edx + + $edx = MOV32rr $edi + $eax = MOV32rr $esi + + $esi = MOV32rr $edx + $edx = MOV32rr $eax + + $edi = MOV32rr $esi + $esi = MOV32rr $edx + + CALL64pcrel32 @call2, csr_64, implicit $rsp, implicit $ssp, implicit $edi, implicit $esi, implicit-def $rsp, implicit-def $ssp, debug-location !15 + $rax = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !16 + CFI_INSTRUCTION def_cfa_offset 8, debug-location !16 + RETQ debug-location !16 + +... + +# Verify that we emit correct call site parameter entries even after moving +# around the call site values between different registers. +# +# This test uses an implicit CHECK-NOT to verify that only two call site +# parameter entries are emitted. + +# 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_constu 0x7b) + +# 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_GNU_entry_value(DW_OP_reg5 RDI)) diff --git a/llvm/test/DebugInfo/MIR/X86/dbgcall-site-two-fwd-reg-defs.mir b/llvm/test/DebugInfo/MIR/X86/dbgcall-site-two-fwd-reg-defs.mir --- a/llvm/test/DebugInfo/MIR/X86/dbgcall-site-two-fwd-reg-defs.mir +++ b/llvm/test/DebugInfo/MIR/X86/dbgcall-site-two-fwd-reg-defs.mir @@ -133,9 +133,10 @@ # sign-extended to 64 bits. # CHECK: DW_TAG_call_site_parameter +# CHECK-NEXT: DW_AT_location (DW_OP_reg5 RDI) +# CHECK-NEXT: DW_AT_call_value (DW_OP_breg3 RBX+0, DW_OP_constu 0xffffffff, DW_OP_and, DW_OP_convert ({{.*}}) "DW_ATE_signed_32", DW_OP_convert ({{.*}}) "DW_ATE_signed_64") +# +# CHECK: DW_TAG_call_site_parameter # CHECK-NEXT: DW_AT_location (DW_OP_reg4 RSI) # CHECK-NEXT: DW_AT_call_value (DW_OP_breg3 RBX+0) -# CHECK: DW_TAG_call_site_parameter -# CHECK-NEXT: DW_AT_location (DW_OP_reg5 RDI) -# CHECK-NEXT: DW_AT_call_value (DW_OP_breg3 RBX+0, DW_OP_constu 0xffffffff, DW_OP_and, DW_OP_convert ({{.*}}) "DW_ATE_signed_32", DW_OP_convert ({{.*}}) "DW_ATE_signed_64")