[llvm] f9ac161 - [DebugInfo][InstrRef] Fix error in copy handling in InstrRefLDV
Stephen Tozer via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 11 05:39:34 PDT 2022
Author: Stephen Tozer
Date: 2022-07-11T13:38:23+01:00
New Revision: f9ac161af9d9c3af03cda25497a7280f51fd92ac
URL: https://github.com/llvm/llvm-project/commit/f9ac161af9d9c3af03cda25497a7280f51fd92ac
DIFF: https://github.com/llvm/llvm-project/commit/f9ac161af9d9c3af03cda25497a7280f51fd92ac.diff
LOG: [DebugInfo][InstrRef] Fix error in copy handling in InstrRefLDV
Currently, an error exists when InstrRefBasedLDV observes transfers of
variables across copies, which causes it to lose track of variables
under certain circumstances, resulting in shorter lifetimes for those
variables as LDV gives up searching for live locations for them. This
patch fixes this issue by storing the currently tracked values in
the destination first, then updating them manually later without
clobbering or assigning them the wrong value.
Differential Revision: https://reviews.llvm.org/D128101
Added:
llvm/test/DebugInfo/X86/instr-ref-track-clobbers.mir
Modified:
llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
index 30ca8bd871e8..43c12c67939e 100644
--- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
+++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
@@ -536,6 +536,17 @@ class TransferTracker {
// What was the old variable value?
ValueIDNum OldValue = VarLocs[MLoc.asU64()];
+ clobberMloc(MLoc, OldValue, Pos, MakeUndef);
+ }
+ /// Overload that takes an explicit value \p OldValue for when the value in
+ /// \p MLoc has changed and the TransferTracker's locations have not been
+ /// updated yet.
+ void clobberMloc(LocIdx MLoc, ValueIDNum OldValue,
+ MachineBasicBlock::iterator Pos, bool MakeUndef = true) {
+ auto ActiveMLocIt = ActiveMLocs.find(MLoc);
+ if (ActiveMLocIt == ActiveMLocs.end())
+ return;
+
VarLocs[MLoc.asU64()] = ValueIDNum::EmptyValue;
// Examine the remaining variable locations: if we can find the same value
@@ -1730,9 +1741,35 @@ bool InstrRefBasedLDV::transferRegisterCopy(MachineInstr &MI) {
if (EmulateOldLDV && !SrcRegOp->isKill())
return false;
+ // Before we update MTracker, remember which values were present in each of
+ // the locations about to be overwritten, so that we can recover any
+ // potentially clobbered variables.
+ DenseMap<LocIdx, ValueIDNum> ClobberedLocs;
+ if (TTracker) {
+ for (MCRegAliasIterator RAI(DestReg, TRI, true); RAI.isValid(); ++RAI) {
+ LocIdx ClobberedLoc = MTracker->getRegMLoc(*RAI);
+ auto MLocIt = TTracker->ActiveMLocs.find(ClobberedLoc);
+ // If ActiveMLocs isn't tracking this location or there are no variables
+ // using it, don't bother remembering.
+ if (MLocIt == TTracker->ActiveMLocs.end() || MLocIt->second.empty())
+ continue;
+ ValueIDNum Value = MTracker->readReg(*RAI);
+ ClobberedLocs[ClobberedLoc] = Value;
+ }
+ }
+
// Copy MTracker info, including subregs if available.
InstrRefBasedLDV::performCopy(SrcReg, DestReg);
+ // The copy might have clobbered variables based on the destination register.
+ // Tell TTracker about it, passing the old ValueIDNum to search for
+ // alternative locations (or else terminating those variables).
+ if (TTracker) {
+ for (auto LocVal : ClobberedLocs) {
+ TTracker->clobberMloc(LocVal.first, LocVal.second, MI.getIterator(), false);
+ }
+ }
+
// Only produce a transfer of DBG_VALUE within a block where old LDV
// would have. We might make use of the additional value tracking in some
// other way, later.
@@ -1744,15 +1781,6 @@ bool InstrRefBasedLDV::transferRegisterCopy(MachineInstr &MI) {
if (EmulateOldLDV && SrcReg != DestReg)
MTracker->defReg(SrcReg, CurBB, CurInst);
- // Finally, the copy might have clobbered variables based on the destination
- // register. Tell TTracker about it, in case a backup location exists.
- if (TTracker) {
- for (MCRegAliasIterator RAI(DestReg, TRI, true); RAI.isValid(); ++RAI) {
- LocIdx ClobberedLoc = MTracker->getRegMLoc(*RAI);
- TTracker->clobberMloc(ClobberedLoc, MI.getIterator(), false);
- }
- }
-
return true;
}
diff --git a/llvm/test/DebugInfo/X86/instr-ref-track-clobbers.mir b/llvm/test/DebugInfo/X86/instr-ref-track-clobbers.mir
new file mode 100644
index 000000000000..8496f17b0f02
--- /dev/null
+++ b/llvm/test/DebugInfo/X86/instr-ref-track-clobbers.mir
@@ -0,0 +1,140 @@
+# RUN: llc --run-pass=livedebugvalues %s -o - -experimental-debug-variable-locations | FileCheck %s --check-prefixes=CHECK
+
+# Test that we accurately track variables through transfers and clobbers.
+
+# In this test case, the value of "b" is found in $rax. From there it is moved
+# to $r15, $rax is killed, the value is moved back from $r15 into $rax again,
+# and then $r15 is killed. Throughout this, we should track "b" and end up with
+# DBG_VALUEs pointing to $rax, then $r15, then $rax again.
+
+# CHECK-LABEL: bb.0.entry:
+# CHECK: DBG_VALUE $rax,
+# CHECK: $r15 = MOV64rr
+
+## Technically it's correct to insert the DBG_VALUE for $r15 either immediately
+## after the move to $r15 or immediately after $rax is clobbered, but no later.
+# CHECK: DBG_VALUE $r15,
+# CHECK: $rax = LEA64r
+# CHECK: $rax = MOV64rr
+
+## Again, the DBG_VALUE can come either after the copy to $rax or the kill of
+## $r15, but no later.
+# CHECK: $r15 = LEA64r
+# CHECK: DBG_VALUE $rax,
+# CHECK: RET64
+
+--- |
+ ; ModuleID = 'test.cpp'
+ source_filename = "test.cpp"
+ 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: mustprogress nofree norecurse nosync nounwind readnone willreturn uwtable
+ define dso_local noundef i64 @_Z3fooxx(i64 noundef %a, i64 noundef %b) local_unnamed_addr !dbg !7 {
+ entry:
+ call void @llvm.dbg.value(metadata i64 %a, metadata !12, metadata !DIExpression()), !dbg !16
+ call void @llvm.dbg.value(metadata i64 %b, metadata !13, metadata !DIExpression()), !dbg !16
+ %add = add nsw i64 %b, %a, !dbg !17
+ call void @llvm.dbg.value(metadata i64 %add, metadata !14, metadata !DIExpression()), !dbg !16
+ %mul17 = sub i64 %a, %b, !dbg !18
+ %sub = mul i64 %add, %mul17, !dbg !18
+ call void @llvm.dbg.value(metadata i64 %sub, metadata !15, metadata !DIExpression()), !dbg !16
+ %add2 = add nsw i64 %sub, %add, !dbg !19
+ ret i64 %add2, !dbg !20
+ }
+
+ ; Function Attrs: nocallback nofree nosync nounwind readnone speculatable willreturn
+ declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+ !llvm.dbg.cu = !{!0}
+ !llvm.module.flags = !{!2, !3, !4, !5}
+ !llvm.ident = !{!6}
+
+ !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 15.0.0 (https://github.com/llvm/llvm-project.git 44673278e029d7a6f56c8a3177247026b831720f)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+ !1 = !DIFile(filename: "test.cpp", directory: "/home/stozer/dev/llvm-project-build", checksumkind: CSK_MD5, checksum: "1e1c19354b0e967af705e204e8740d56")
+ !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, !"uwtable", i32 2}
+ !6 = !{!"clang version 15.0.0 (https://github.com/llvm/llvm-project.git 44673278e029d7a6f56c8a3177247026b831720f)"}
+ !7 = distinct !DISubprogram(name: "foo", linkageName: "_Z3fooxx", scope: !1, file: !1, line: 1, type: !8, scopeLine: 1, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !11)
+ !8 = !DISubroutineType(types: !9)
+ !9 = !{!10, !10, !10}
+ !10 = !DIBasicType(name: "long long", size: 64, encoding: DW_ATE_signed)
+ !11 = !{!12, !13, !14, !15}
+ !12 = !DILocalVariable(name: "a", arg: 1, scope: !7, file: !1, line: 1, type: !10)
+ !13 = !DILocalVariable(name: "b", arg: 2, scope: !7, file: !1, line: 1, type: !10)
+ !14 = !DILocalVariable(name: "c", scope: !7, file: !1, line: 2, type: !10)
+ !15 = !DILocalVariable(name: "d", scope: !7, file: !1, line: 3, type: !10)
+ !16 = !DILocation(line: 0, scope: !7)
+ !17 = !DILocation(line: 2, column: 19, scope: !7)
+ !18 = !DILocation(line: 3, column: 23, scope: !7)
+ !19 = !DILocation(line: 4, column: 12, scope: !7)
+ !20 = !DILocation(line: 4, column: 3, scope: !7)
+
+...
+---
+name: _Z3fooxx
+alignment: 16
+exposesReturnsTwice: false
+legalized: false
+regBankSelected: false
+selected: false
+failedISel: false
+tracksRegLiveness: true
+hasWinCFI: false
+callsEHReturn: false
+callsUnwindInit: false
+hasEHCatchret: false
+hasEHScopes: false
+hasEHFunclets: false
+failsVerification: false
+tracksDebugUserValues: true
+registers: []
+liveins:
+ - { reg: '$rdi', virtual-reg: '' }
+ - { reg: '$rsi', virtual-reg: '' }
+frameInfo:
+ isFrameAddressTaken: false
+ isReturnAddressTaken: false
+ hasStackMap: false
+ hasPatchPoint: false
+ stackSize: 0
+ offsetAdjustment: 0
+ maxAlignment: 1
+ adjustsStack: false
+ hasCalls: false
+ stackProtector: ''
+ functionContext: ''
+ maxCallFrameSize: 0
+ cvBytesOfCalleeSavedRegisters: 0
+ hasOpaqueSPAdjustment: false
+ hasVAStart: false
+ hasMustTailInVarArgFunc: false
+ hasTailCall: false
+ localFrameSize: 0
+ savePoint: ''
+ restorePoint: ''
+fixedStack:
+- { id: 0, type: spill-slot, offset: -16, size: 8, alignment: 16, stack-id: default,
+ callee-saved-register: '$r15', callee-saved-restored: true, debug-info-variable: '',
+ debug-info-expression: '', debug-info-location: '' }
+stack: []
+callSites: []
+debugValueSubstitutions:
+ - { srcinst: 1, srcop: 0, dstinst: 3, dstop: 0, subreg: 0 }
+constants: []
+machineFunctionInfo: {}
+body: |
+ bb.0.entry:
+ liveins: $rdi, $rsi
+
+ renamable $rax = LEA64r renamable $rsi, 1, renamable $rdi, 0, $noreg, debug-instr-number 3, debug-location !17
+ DBG_VALUE $rax, $noreg, !13, !DIExpression(), debug-location !17
+ $r15 = MOV64rr killed $rax, debug-location !16
+ renamable $rax = LEA64r renamable $rdi, 1, $noreg, 12, $noreg, debug-location !16
+ $rax = MOV64rr killed $r15, debug-location !16
+ renamable $r15 = LEA64r renamable $rdi, 1, $noreg, 12, $noreg, debug-location !16
+ RET64 $r15, debug-location !20
+
+...
More information about the llvm-commits
mailing list