[DebugInfo] Find call site values for params in preserved registers #172339
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For example, given code like this:
int local = value();
callee(local);
return local;
resulting in assembly like:
movl %eax, %ebx // %eax = local
movl %eax, %edi
callq callee@PLT
the call site value generation did not understand that the value of
local is stored in the callee-saved %ebx during the call, and did not
emit any call site value. This patch addresses that, resulting in:
DW_TAG_call_site_parameter
DW_AT_location (DW_OP_reg5 RDI)
DW_AT_call_value (DW_OP_breg3 RBX+0)
This code does not keep track if any succeeding instructions save
registers, meaning that it fails to emit a call site value for a case
like this:
movq %rax, %rdi
movq %rax, %rbx
callq callee@PLT
The test case dbgcall-site-preserved-clobbered.mir has been added for
that. This should be rather easy to address, but can be done in a
follow-up patch.
Building a clang-21 RelWithDebInfo binary for x86-64 without/with this
patch results in a ~1.8% increase of the number of call site parameter
entries with a location (from 1792876 to 1825718). This also reduces the
number of call site parameter entries using DW_OP_entry_value locations,
which are not guaranteed to be printable as they require the frame above
to provide a call site value for that parameter, from 57718 to 34871.
Fixes llvm#43464.
Member
|
@llvm/pr-subscribers-debuginfo Author: David Stenberg (dstenb) ChangesFor example, given code like this: resulting in assembly like: the call site value generation did not understand that the value of local is stored in the callee-saved %ebx during the call, and did not emit any call site value. This patch addresses that, resulting in: This code does not keep track if any succeeding instructions save registers, meaning that it fails to emit a call site value for a case like this: The test case dbgcall-site-preserved-clobbered.mir has been added for that. This should be rather easy to address, but can be done in a follow-up patch. Building a clang-21 RelWithDebInfo binary for x86-64 without/with this patch results in a ~1.8% increase of the number of call site parameter entries with a location (from 1792876 to 1825718). This also reduces the number of call site parameter entries using DW_OP_entry_value locations, which are not guaranteed to be printable as they require the frame above to provide a call site value for that parameter, from 57718 to 34871. Fixes #43464. Patch is 23.31 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/172339.diff 7 Files Affected:
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 08444fc680df6..a30f5ffec93d0 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -672,6 +672,58 @@ static void interpretValues(const MachineInstr *CurMI,
const auto &TII = *MF->getSubtarget().getInstrInfo();
const auto &TLI = *MF->getSubtarget().getTargetLowering();
+ // It's possible that we find a copy from a non-volatile register to the param
+ // register, which is clobbered in the meantime. Test for clobbered reg unit
+ // overlaps before completing.
+ auto IsRegClobberedInMeantime = [&](Register Reg) -> bool {
+ for (auto &RegUnit : ClobberedRegUnits)
+ if (TRI.hasRegUnit(Reg, RegUnit))
+ return true;
+ return false;
+ };
+
+ auto DescribeFwdRegsByCalleeSavedCopy = [&](DestSourcePair CopyInst) {
+ Register CopyDestReg = CopyInst.Destination->getReg();
+ Register CopySrcReg = CopyInst.Source->getReg();
+ if (IsRegClobberedInMeantime(CopyDestReg))
+ return;
+ // FIXME: This may be incorrect in cases where the caller and callee use
+ // different calling conventions.
+ if (!TRI.isCalleeSavedPhysReg(CopyDestReg, *MF))
+ return;
+ // Describe any forward registers matching the source register. If the
+ // forward register is a sub-register of the source, we describe it using
+ // the corresponding sub-register in the destination, if such a
+ // sub-register exists. The end iterator in the MapVector is invalidated at
+ // erase(), so it needs to be evaluated at each iteration.
+ for (auto FwdRegIt = ForwardedRegWorklist.begin();
+ FwdRegIt != ForwardedRegWorklist.end();) {
+ Register CalleeSavedReg = MCRegister::NoRegister;
+ if (FwdRegIt->first == CopySrcReg)
+ CalleeSavedReg = CopyDestReg;
+ else if (unsigned SubRegIdx =
+ TRI.getSubRegIndex(CopySrcReg, FwdRegIt->first))
+ if (Register CopyDestSubReg = TRI.getSubReg(CopyDestReg, SubRegIdx))
+ CalleeSavedReg = CopyDestSubReg;
+
+ if (CalleeSavedReg == MCRegister::NoRegister) {
+ ++FwdRegIt;
+ continue;
+ }
+
+ MachineLocation MLoc(CalleeSavedReg, /*Indirect=*/false);
+ finishCallSiteParams(MLoc, EmptyExpr, FwdRegIt->second, Params);
+ FwdRegIt = ForwardedRegWorklist.erase(FwdRegIt);
+ }
+ };
+
+ // Detect if this is a copy instruction. If this saves any of the forward
+ // registers in callee-saved registers, we can finalize those parameters
+ // directly.
+ // TODO: Can we do something similar for stack saves?
+ if (auto CopyInst = TII.isCopyInstr(*CurMI))
+ DescribeFwdRegsByCalleeSavedCopy(*CopyInst);
+
// 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
@@ -721,16 +773,6 @@ static void interpretValues(const MachineInstr *CurMI,
return;
}
- // It's possible that we find a copy from a non-volatile register to the param
- // register, which is clobbered in the meantime. Test for clobbered reg unit
- // overlaps before completing.
- auto IsRegClobberedInMeantime = [&](Register Reg) -> bool {
- for (auto &RegUnit : ClobberedRegUnits)
- if (TRI.hasRegUnit(Reg, RegUnit))
- return true;
- return false;
- };
-
for (auto ParamFwdReg : FwdRegDefs) {
if (auto ParamValue = TII.describeLoadedValue(*CurMI, ParamFwdReg)) {
if (ParamValue->first.isImm()) {
@@ -742,6 +784,8 @@ static void interpretValues(const MachineInstr *CurMI,
Register SP = TLI.getStackPointerRegisterToSaveRestore();
Register FP = TRI.getFrameRegister(*MF);
bool IsSPorFP = (RegLoc == SP) || (RegLoc == FP);
+ // FIXME: This may be incorrect in cases where the caller and callee use
+ // different calling conventions.
if (!IsRegClobberedInMeantime(RegLoc) &&
(TRI.isCalleeSavedPhysReg(RegLoc, *MF) || IsSPorFP)) {
MachineLocation MLoc(RegLoc, /*Indirect=*/IsSPorFP);
diff --git a/llvm/test/DebugInfo/AArch64/dbgcall-site-preserved-subreg.ll b/llvm/test/DebugInfo/AArch64/dbgcall-site-preserved-subreg.ll
new file mode 100644
index 0000000000000..378c20b2baf80
--- /dev/null
+++ b/llvm/test/DebugInfo/AArch64/dbgcall-site-preserved-subreg.ll
@@ -0,0 +1,56 @@
+; RUN: llc -mtriple aarch64-linux-gnu -emit-call-site-info -filetype=obj -o - %s | llvm-dwarfdump - | FileCheck %s
+
+; Based on the following C reproducer:
+;
+; void bar(int);
+; long foo(long p) {
+; bar(p);
+; return p;
+; }
+;
+; Verify that we are able to emit a call site value for the 32-bit W0 parameter
+; using a subreg of the preserved register storing the 64-bit value of p.
+
+; CHECK: DW_TAG_call_site_parameter
+; CHECK-NEXT: DW_AT_location (DW_OP_reg0 W0)
+; CHECK-NEXT: DW_AT_call_value (DW_OP_breg19 W19+0)
+
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128-Fn32"
+target triple = "aarch64"
+
+define i64 @foo(i64 noundef returned %p) !dbg !8 {
+entry:
+ %conv = trunc i64 %p to i32, !dbg !16
+ tail call void @bar(i32 noundef %conv), !dbg !16
+ ret i64 %p, !dbg !17
+}
+
+declare !dbg !18 void @bar(i32 noundef)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang version 22.0.0git.prerel", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "c1.i", directory: "/")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 7, !"frame-pointer", i32 4}
+!6 = !{i32 7, !"debug-info-assignment-tracking", i1 true}
+!7 = !{!"clang version 22.0.0git.prerel"}
+!8 = distinct !DISubprogram(name: "foo", scope: !9, file: !9, line: 2, type: !10, scopeLine: 2, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !13, keyInstructions: true)
+!9 = !DIFile(filename: "c1.i", directory: "/", checksumkind: CSK_MD5, checksum: "d6df8546b35604f19079ca89d49a7275")
+!10 = !DISubroutineType(types: !11)
+!11 = !{!12, !12}
+!12 = !DIBasicType(name: "long", size: 64, encoding: DW_ATE_signed)
+!13 = !{!14}
+!14 = !DILocalVariable(name: "p", arg: 1, scope: !8, file: !9, line: 2, type: !12)
+!15 = !DILocation(line: 0, scope: !8)
+!16 = !DILocation(line: 3, scope: !8)
+!17 = !DILocation(line: 4, scope: !8, atomGroup: 1, atomRank: 1)
+!18 = !DISubprogram(name: "bar", scope: !9, file: !9, line: 1, type: !19, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized)
+!19 = !DISubroutineType(types: !20)
+!20 = !{null, !21}
+!21 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
diff --git a/llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-interpretation.mir b/llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-interpretation.mir
index 9df0044d39711..84e180f4df773 100644
--- a/llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-interpretation.mir
+++ b/llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-interpretation.mir
@@ -13,13 +13,17 @@
# CHECK: DW_TAG_GNU_call_site
# CHECK-NEXT: DW_AT_abstract_origin {{.*}} "func2"
# CHECK: DW_TAG_GNU_call_site_parameter
-# CHECK-NOT: DW_AT_location (DW_OP_reg2 W0)
# CHECK-NEXT: DW_AT_location (DW_OP_reg2 W2)
# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_fbreg +28)
# CHECK-EMPTY:
# CHECK-NEXT: 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-4)
+# CHECK-EMPTY:
+# CHECK-NEXT: 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+2)
+# CHECK-NOT: DW_TAG_GNU_call_site_parameter
# CHECK: DW_TAG_GNU_call_site
# CHECK-NEXT: DW_AT_abstract_origin {{.*}} "func2")
# CHECK-NEXT: DW_AT_low_pc
@@ -29,10 +33,10 @@
# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_lit13)
# CHECK-EMPTY:
# CHECK-NEXT: DW_TAG_GNU_call_site_parameter
-# W2 loads memory location. We can't rely that memory location won't be changed.
-# CHECK-NOT: DW_AT_location (DW_OP_reg2 W2)
# CHECK-NEXT: DW_AT_location (DW_OP_reg1 W1)
# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_constu 0xc0)
+# W2 loads memory location. We can't rely that memory location won't be changed.
+# CHECK-NOT: DW_TAG_GNU_call_site_parameter
--- |
; ModuleID = 'describe-call-pram-load-instruction.c'
source_filename = "describe-call-pram-load-instruction.c"
diff --git a/llvm/test/DebugInfo/MIR/ARM/dbgcall-site-interpretation.mir b/llvm/test/DebugInfo/MIR/ARM/dbgcall-site-interpretation.mir
index ce9fc094fa297..a285877c9c92b 100644
--- a/llvm/test/DebugInfo/MIR/ARM/dbgcall-site-interpretation.mir
+++ b/llvm/test/DebugInfo/MIR/ARM/dbgcall-site-interpretation.mir
@@ -20,15 +20,20 @@
# CHECK-NEXT: DW_TAG_GNU_call_site_parameter
# CHECK-NEXT: DW_AT_location (DW_OP_reg1 R1)
# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_breg4 R4-4)
+# CHECK-EMPTY:
+# CHECK-NEXT: DW_TAG_GNU_call_site_parameter
+# CHECK-NEXT: DW_AT_location (DW_OP_reg0 R0)
+# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_breg4 R4+2)
+# CHECK-NOT: DW_TAG_GNU_call_site_parameter
# CHECK: DW_TAG_GNU_call_site
# CHECK-NEXT: DW_AT_abstract_origin {{.*}}"func2")
# CHECK-NEXT: DW_AT_low_pc
# CHECK-EMPTY:
# CHECK-NEXT: DW_TAG_GNU_call_site_parameter
-# R2 loads memory location. We can't rely that memory location won't be changed.
-# CHECK-NOT: DW_AT_location (DW_OP_reg2 R2)
# CHECK-NEXT: DW_AT_location (DW_OP_reg1 R1)
# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_breg4 R4+8)
+# R2 loads memory location. We can't rely that memory location won't be changed.
+# CHECK-NOT: DW_TAG_GNU_call_site_parameter
--- |
; ModuleID = 'dbgcall-site-interpretation.c'
source_filename = "dbgcall-site-interpretation.c"
diff --git a/llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir b/llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir
index 9f18dd04c4674..fa959b8a5e32f 100644
--- a/llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir
+++ b/llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir
@@ -14,22 +14,21 @@
# 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-NEXT: DW_AT_GNU_call_site_value (DW_OP_breg15 R15+0)
# 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-NEXT: DW_AT_GNU_call_site_value (DW_OP_breg15 R15+0)
# CHECK-EMPTY:
# CHECK: DW_TAG_GNU_call_site
# CHECK-NEXT: DW_AT_abstract_origin {{.*}}"foo"
# CHECK-NEXT: DW_AT_low_pc
# CHECK-EMPTY:
# CHECK-NEXT: DW_TAG_GNU_call_site_parameter
-# RCX loads memory location. We can't rely that memory location won't be changed.
-# CHECK-NOT: DW_AT_location (DW_OP_reg2 RCX)
# CHECK-NEXT: DW_AT_location (DW_OP_reg4 RSI)
# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_lit4)
# CHECK-EMPTY:
+# RCX loads memory location. We can't rely that memory location won't be changed.
# CHECK-NOT: DW_TAG_GNU_call_site_parameter
#
# Check that call site interpretation analysis can interpret instructions such
diff --git a/llvm/test/DebugInfo/MIR/X86/dbgcall-site-preserved-after.mir b/llvm/test/DebugInfo/MIR/X86/dbgcall-site-preserved-after.mir
new file mode 100644
index 0000000000000..4f22ddb7b5d79
--- /dev/null
+++ b/llvm/test/DebugInfo/MIR/X86/dbgcall-site-preserved-after.mir
@@ -0,0 +1,126 @@
+# RUN: llc %s -o - -filetype=obj -start-after=livedebugvalues \
+# RUN: -emit-call-site-info | \
+# RUN: llvm-dwarfdump - | FileCheck %s
+
+--- |
+ ; ModuleID = 'pr43464.c'
+ source_filename = "pr43464.c"
+ target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+ target triple = "x86_64-unknown-linux-gnu"
+
+ ; Function Attrs: nounwind uwtable
+ define dso_local noundef i32 @caller() local_unnamed_addr #0 !dbg !10 {
+ entry:
+ %call = tail call i64 @value() #2, !dbg !17
+ #dbg_value(i64 %call, !15, !DIExpression(), !18)
+ tail call void @callee(i64 noundef %call) #2, !dbg !19
+ %conv = trunc i64 %call to i32, !dbg !20
+ ret i32 %conv, !dbg !21
+ }
+
+ declare !dbg !22 i64 @value() local_unnamed_addr #1
+
+ declare !dbg !25 void @callee(i64 noundef) local_unnamed_addr #1
+
+ attributes #0 = { nounwind uwtable "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+ attributes #1 = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+ attributes #2 = { nounwind }
+
+ !llvm.dbg.cu = !{!0}
+ !llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
+ !llvm.ident = !{!9}
+
+ !0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang version 22.0.0git.prerel", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+ !1 = !DIFile(filename: "pr43464.c", directory: "/repo/edasten/llvm-project/llvm", checksumkind: CSK_MD5, checksum: "e63080e8df26470df7e757aa6a4ccecd")
+ !2 = !{i32 7, !"Dwarf Version", i32 5}
+ !3 = !{i32 2, !"Debug Info Version", i32 3}
+ !4 = !{i32 1, !"wchar_size", i32 4}
+ !5 = !{i32 8, !"PIC Level", i32 2}
+ !6 = !{i32 7, !"PIE Level", i32 2}
+ !7 = !{i32 7, !"uwtable", i32 2}
+ !8 = !{i32 7, !"debug-info-assignment-tracking", i1 true}
+ !9 = !{!"clang version 22.0.0git.prerel"}
+ !10 = distinct !DISubprogram(name: "caller", scope: !1, file: !1, line: 4, type: !11, scopeLine: 4, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !14, keyInstructions: true)
+ !11 = !DISubroutineType(types: !12)
+ !12 = !{!13}
+ !13 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+ !14 = !{!15}
+ !15 = !DILocalVariable(name: "local", scope: !10, file: !1, line: 5, type: !16)
+ !16 = !DIBasicType(name: "long", size: 64, encoding: DW_ATE_signed)
+ !17 = !DILocation(line: 5, scope: !10, atomGroup: 1, atomRank: 2)
+ !18 = !DILocation(line: 0, scope: !10)
+ !19 = !DILocation(line: 6, scope: !10)
+ !20 = !DILocation(line: 7, scope: !10, atomGroup: 3, atomRank: 2)
+ !21 = !DILocation(line: 7, scope: !10, atomGroup: 3, atomRank: 1)
+ !22 = !DISubprogram(name: "value", scope: !1, file: !1, line: 2, type: !23, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized)
+ !23 = !DISubroutineType(types: !24)
+ !24 = !{!16}
+ !25 = !DISubprogram(name: "callee", scope: !1, file: !1, line: 1, type: !26, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized)
+ !26 = !DISubroutineType(types: !27)
+ !27 = !{null, !16}
+...
+---
+name: caller
+alignment: 16
+tracksRegLiveness: true
+noPhis: true
+isSSA: false
+noVRegs: true
+hasFakeUses: false
+debugInstrRef: true
+tracksDebugUserValues: true
+frameInfo:
+ stackSize: 8
+ offsetAdjustment: -8
+ maxAlignment: 1
+ adjustsStack: true
+ hasCalls: true
+ maxCallFrameSize: 0
+ cvBytesOfCalleeSavedRegisters: 8
+ isCalleeSavedInfoValid: true
+fixedStack:
+ - { id: 0, type: spill-slot, offset: -16, size: 8, alignment: 16, callee-saved-register: '$rbx' }
+callSites:
+ - { bb: 0, offset: 3 }
+ - { bb: 0, offset: 8, fwdArgRegs:
+ - { arg: 0, reg: '$rdi' } }
+machineFunctionInfo:
+ amxProgModel: None
+body: |
+ bb.0.entry:
+ liveins: $rbx
+
+ frame-setup PUSH64r killed $rbx, implicit-def $rsp, implicit $rsp, debug-location !17
+ frame-setup CFI_INSTRUCTION def_cfa_offset 16
+ CFI_INSTRUCTION offset $rbx, -16
+ CALL64pcrel32 target-flags(x86-plt) @value, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax, debug-instr-number 1, debug-location !17
+ DBG_INSTR_REF !15, !DIExpression(DW_OP_LLVM_arg, 0), dbg-instr-ref(1, 6), debug-location !18
+ DBG_VALUE_LIST !15, !DIExpression(DW_OP_LLVM_arg, 0), $rbx, debug-location !18
+ $rdi = MOV64rr $rax, debug-location !19
+ $rbx = MOV64rr $rax, debug-location !17
+ CALL64pcrel32 target-flags(x86-plt) @callee, csr_64, implicit $rsp, implicit $ssp, implicit killed $rdi, implicit-def $rsp, implicit-def $ssp, debug-location !19
+ $eax = MOV32rr $ebx, implicit killed $rbx, debug-location !21
+ $rbx = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !21
+ frame-destroy CFI_INSTRUCTION def_cfa_offset 8, debug-location !21
+ RET64 $eax, debug-location !21
+...
+
+# Generated from the following C reproducer:
+#
+# extern void callee(long);
+# extern long value(void);
+#
+# int caller() {
+# long local = value();
+# callee(local);
+# return local;
+# }
+#
+# but with the order of the instructions saving the value of `local` in $rbx
+# with the copy of $rax to $rdi switched.
+#
+# FIXME: The call site value interpreting code iterates
+# instruction-by-instruction upwards, and does not keep track of which register
+# values are saved during the call by succeeding instructions, so it does not
+# understand that the value of $rdi is available in $rbx.
+# CHECK-NOT: call_site_parameter
diff --git a/llvm/test/DebugInfo/MIR/X86/dbgcall-site-preserved-clobbered.mir b/llvm/test/DebugInfo/MIR/X86/dbgcall-site-preserved-clobbered.mir
new file mode 100644
index 0000000000000..7e911c0ec8d18
--- /dev/null
+++ b/llvm/test/DebugInfo/MIR/X86/dbgcall-site-preserved-clobbered.mir
@@ -0,0 +1,124 @@
+# RUN: llc %s -o - -filetype=obj -start-after=livedebugvalues \
+# RUN: -emit-call-site-info | \
+# RUN: llvm-dwarfdump - | FileCheck %s
+
+--- |
+ ; ModuleID = 'pr43464.c'
+ source_filename = "pr43464.c"
+ target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+ target triple = "x86_64-unknown-linux-gnu"
+
+ ; Function Attrs: nounwind uwtable
+ define dso_local noundef i32 @caller() local_unnamed_addr #0 !dbg !10 {
+ entry:
+ %call = tail call i64 @value() #2, !dbg !17
+ #dbg_value(i64 %call, !15, !DIExpression(), !18)
+ tail call void @callee(i64 noundef %call) #2, !dbg !19
+ %conv = trunc i64 %call to i32, !dbg !20
+ ret i32 %conv, !dbg !21
+ }
+
+ declare !dbg !22 i64 @value() local_unnamed_addr #1
+
+ declare !dbg !25 void @callee(i64 noundef) local_unnamed_addr #1
+
+ attributes #0 = { nounwind uwtable "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+ attributes #1 = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+ attributes #2 = { nounwind }
+
+ !llvm.dbg.cu = !{!0}
+ !llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
+ !llvm.ident = !{!9}
+
+ !0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang version 22.0.0git.prerel", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+ !1 = !DIFile(filename: "pr43464.c", directory: "/repo/edasten/llvm-project/llvm", checksumkind: CSK_MD5, checksum: "e63080e8df26470df7e757aa6a4ccecd")
+ !2 = !{i32 7, !"Dwarf Version", i32 5}
+ !3 = !{i32 2, !"Debug Info Version", i32 3}
+ !4 = !{i32 1, !"wchar_size", i32 4}
+ !5 = !{i32 8, !"PIC Level", i32 2}
+ !6 = !{i32 7, !"PIE Level", i32 2}
+ !7 = !{i32 7, !"uwtable", i32 2}
+ !8 = !{i32 7, !"debug-info-assignment-tracking", i1 true}
+ !9 = !{!"clang version 22.0.0git.prerel"}
+ !10 = distinct !DISubprogram(name: "caller", scope: !1, file: !1, line: 4, type: !11, scopeLine: 4, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinit...
[truncated]
|
OCHyams
reviewed
Dec 15, 2025
Contributor
OCHyams
left a comment
OCHyams
left a comment
There was a problem hiding this comment.
Overall SGTM, it's nice to see some improvements here.
It's a bit fiddly so I'll give others a chance to take a look before approving.
Test changes all LGTM.
| @@ -0,0 +1,56 @@ | |||
| ; RUN: llc -mtriple aarch64-linux-gnu -emit-call-site-info -filetype=obj -o - %s | llvm-dwarfdump - | FileCheck %s | |||
Contributor
There was a problem hiding this comment.
Could we narrow down the tested pipeline a bit? e.g. using -stop-after=livedebugvalues -simplify-mir and testing with -start-after?
Commenting (or testing) the disassembly could ease understanding of the test too, but I'll leave that up to you
| # RCX loads memory location. We can't rely that memory location won't be changed. | ||
| # CHECK-NOT: DW_TAG_GNU_call_site_parameter | ||
| # | ||
| # Check that call site interpretation analysis can interpret instructions such |
Contributor
There was a problem hiding this comment.
I know it's not your test but could you add a note saying the MIR is hand-modified? (I believe the other tests already state this)
| DBG_INSTR_REF !15, !DIExpression(DW_OP_LLVM_arg, 0), dbg-instr-ref(1, 6), debug-location !18 | ||
| DBG_VALUE_LIST !15, !DIExpression(DW_OP_LLVM_arg, 0), $rbx, debug-location !18 | ||
| $rdi = MOV64rr $rax, debug-location !19 | ||
| ; Manually inserted instruction partially convering $rbx. |
Contributor
There was a problem hiding this comment.
Suggested change
| ; Manually inserted instruction partially convering $rbx. | |
| ; Manually inserted instruction partially clobbering $rbx. |
Comment on lines
+25
to
+27
| attributes #0 = { nounwind uwtable "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" } | ||
| attributes #1 = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" } | ||
| attributes #2 = { nounwind } |
Contributor
There was a problem hiding this comment.
does the test still work if you delete the stringy attributes (or all of them)?
djtodoro
reviewed
Dec 16, 2025
Comment on lines
+63
to
+82
| name: caller | ||
| alignment: 16 | ||
| tracksRegLiveness: true | ||
| noPhis: true | ||
| isSSA: false | ||
| noVRegs: true | ||
| hasFakeUses: false | ||
| debugInstrRef: true | ||
| tracksDebugUserValues: true | ||
| frameInfo: | ||
| stackSize: 8 | ||
| offsetAdjustment: -8 | ||
| maxAlignment: 1 | ||
| adjustsStack: true | ||
| hasCalls: true | ||
| maxCallFrameSize: 0 | ||
| cvBytesOfCalleeSavedRegisters: 8 | ||
| isCalleeSavedInfoValid: true | ||
| fixedStack: | ||
| - { id: 0, type: spill-slot, offset: -16, size: 8, alignment: 16, callee-saved-register: '$rbx' } |
Collaborator
There was a problem hiding this comment.
I guess we can remove some of these, and test would still pass
djtodoro
reviewed
Dec 16, 2025
Comment on lines
+63
to
+82
| name: caller | ||
| alignment: 16 | ||
| tracksRegLiveness: true | ||
| noPhis: true | ||
| isSSA: false | ||
| noVRegs: true | ||
| hasFakeUses: false | ||
| debugInstrRef: true | ||
| tracksDebugUserValues: true | ||
| frameInfo: | ||
| stackSize: 8 | ||
| offsetAdjustment: -8 | ||
| maxAlignment: 1 | ||
| adjustsStack: true | ||
| hasCalls: true | ||
| maxCallFrameSize: 0 | ||
| cvBytesOfCalleeSavedRegisters: 8 | ||
| isCalleeSavedInfoValid: true | ||
| fixedStack: | ||
| - { id: 0, type: spill-slot, offset: -16, size: 8, alignment: 16, callee-saved-register: '$rbx' } |
Collaborator
There was a problem hiding this comment.
(same as bellow) we can delete some of these
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For example, given code like this:
resulting in assembly like:
the call site value generation did not understand that the value of local is stored in the callee-saved %ebx during the call, and did not emit any call site value. This patch addresses that, resulting in:
This code does not keep track if any succeeding instructions save registers, meaning that it fails to emit a call site value for a case like this:
The test case dbgcall-site-preserved-clobbered.mir has been added for that. This should be rather easy to address, but can be done in a follow-up patch.
Building a clang-21 RelWithDebInfo binary for x86-64 without/with this patch results in a ~1.8% increase of the number of call site parameter entries with a location (from 1792876 to 1825718). This also reduces the number of call site parameter entries using DW_OP_entry_value locations, which are not guaranteed to be printable as they require the frame above to provide a call site value for that parameter, from 57718 to 34871.
Fixes #43464.