[llvm] r345996 - [MachineSink][DebugInfo] Correctly sink DBG_VALUEs
Jeremy Morse via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 2 09:52:49 PDT 2018
Author: jmorse
Date: Fri Nov 2 09:52:48 2018
New Revision: 345996
URL: http://llvm.org/viewvc/llvm-project?rev=345996&view=rev
Log:
[MachineSink][DebugInfo] Correctly sink DBG_VALUEs
As reported in PR38952, postra-machine-sink relies on DBG_VALUE insns being
adjacent to the def of the register that they reference. This is not always
true, leading to register copies being sunk but not the associated DBG_VALUEs,
which gives the debugger a bad variable location.
This patch collects DBG_VALUEs as we walk through a BB looking for copies to
sink, then passes them down to performSink. Compile-time impact should be
negligable.
Differential Revision: https://reviews.llvm.org/D53992
Added:
llvm/trunk/test/CodeGen/X86/pr38952.mir
Modified:
llvm/trunk/lib/CodeGen/MachineSink.cpp
Modified: llvm/trunk/lib/CodeGen/MachineSink.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineSink.cpp?rev=345996&r1=345995&r2=345996&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/MachineSink.cpp (original)
+++ llvm/trunk/lib/CodeGen/MachineSink.cpp Fri Nov 2 09:52:48 2018
@@ -734,12 +734,18 @@ static bool SinkingPreventsImplicitNullC
MBP.LHS.getReg() == BaseReg;
}
-/// Sink an instruction and its associated debug instructions.
+/// Sink an instruction and its associated debug instructions. If the debug
+/// instructions to be sunk are already known, they can be provided in DbgVals.
static void performSink(MachineInstr &MI, MachineBasicBlock &SuccToSinkTo,
- MachineBasicBlock::iterator InsertPos) {
- // Collect matching debug values.
+ MachineBasicBlock::iterator InsertPos,
+ SmallVectorImpl<MachineInstr *> *DbgVals = nullptr) {
+ // If debug values are provided use those, otherwise call collectDebugValues.
SmallVector<MachineInstr *, 2> DbgValuesToSink;
- MI.collectDebugValues(DbgValuesToSink);
+ if (DbgVals)
+ DbgValuesToSink.insert(DbgValuesToSink.begin(),
+ DbgVals->begin(), DbgVals->end());
+ else
+ MI.collectDebugValues(DbgValuesToSink);
// If we cannot find a location to use (merge with), then we erase the debug
// location to prevent debug-info driven tools from potentially reporting
@@ -951,6 +957,9 @@ private:
/// Track which register units have been modified and used.
LiveRegUnits ModifiedRegUnits, UsedRegUnits;
+ /// Track DBG_VALUEs of (unmodified) register units.
+ DenseMap<unsigned, TinyPtrVector<MachineInstr*>> SeenDbgInstrs;
+
/// Sink Copy instructions unused in the same block close to their uses in
/// successors.
bool tryToSinkCopy(MachineBasicBlock &BB, MachineFunction &MF,
@@ -1105,11 +1114,34 @@ bool PostRAMachineSinking::tryToSinkCopy
// block and the current instruction.
ModifiedRegUnits.clear();
UsedRegUnits.clear();
+ SeenDbgInstrs.clear();
for (auto I = CurBB.rbegin(), E = CurBB.rend(); I != E;) {
MachineInstr *MI = &*I;
++I;
+ // Track the operand index for use in Copy.
+ SmallVector<unsigned, 2> UsedOpsInCopy;
+ // Track the register number defed in Copy.
+ SmallVector<unsigned, 2> DefedRegsInCopy;
+
+ // We must sink this DBG_VALUE if its operand is sunk. To avoid searching
+ // for DBG_VALUEs later, record them when they're encountered.
+ if (MI->isDebugValue()) {
+ auto &MO = MI->getOperand(0);
+ if (MO.isReg() && TRI->isPhysicalRegister(MO.getReg())) {
+ // Bail if we can already tell the sink would be rejected, rather
+ // than needlessly accumulating lots of DBG_VALUEs.
+ if (hasRegisterDependency(MI, UsedOpsInCopy, DefedRegsInCopy,
+ ModifiedRegUnits, UsedRegUnits))
+ continue;
+
+ // Record debug use of this register.
+ SeenDbgInstrs[MO.getReg()].push_back(MI);
+ }
+ continue;
+ }
+
if (MI->isDebugInstr())
continue;
@@ -1123,11 +1155,6 @@ bool PostRAMachineSinking::tryToSinkCopy
continue;
}
- // Track the operand index for use in Copy.
- SmallVector<unsigned, 2> UsedOpsInCopy;
- // Track the register number defed in Copy.
- SmallVector<unsigned, 2> DefedRegsInCopy;
-
// Don't sink the COPY if it would violate a register dependency.
if (hasRegisterDependency(MI, UsedOpsInCopy, DefedRegsInCopy,
ModifiedRegUnits, UsedRegUnits)) {
@@ -1149,11 +1176,21 @@ bool PostRAMachineSinking::tryToSinkCopy
assert((SuccBB->pred_size() == 1 && *SuccBB->pred_begin() == &CurBB) &&
"Unexpected predecessor");
+ // Collect DBG_VALUEs that must sink with this copy.
+ SmallVector<MachineInstr *, 4> DbgValsToSink;
+ for (auto &MO : MI->operands()) {
+ if (!MO.isReg() || !MO.isDef())
+ continue;
+ unsigned reg = MO.getReg();
+ for (auto *MI : SeenDbgInstrs.lookup(reg))
+ DbgValsToSink.push_back(MI);
+ }
+
// Clear the kill flag if SrcReg is killed between MI and the end of the
// block.
clearKillFlags(MI, CurBB, UsedOpsInCopy, UsedRegUnits, TRI);
MachineBasicBlock::iterator InsertPos = SuccBB->getFirstNonPHI();
- performSink(*MI, *SuccBB, InsertPos);
+ performSink(*MI, *SuccBB, InsertPos, &DbgValsToSink);
updateLiveIn(MI, SuccBB, UsedOpsInCopy, DefedRegsInCopy);
Changed = true;
Added: llvm/trunk/test/CodeGen/X86/pr38952.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/pr38952.mir?rev=345996&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/X86/pr38952.mir (added)
+++ llvm/trunk/test/CodeGen/X86/pr38952.mir Fri Nov 2 09:52:48 2018
@@ -0,0 +1,103 @@
+# RUN: llc %s -run-pass=postra-machine-sink -o - | FileCheck %s
+--- |
+ ; Module stripped of everything, MIR below is what's interesting
+ ; ModuleID = '<stdin>'
+ source_filename = "justacall.cpp"
+ target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+ target triple = "x86_64-unknown-linux-gnu"
+
+ ; Function Attrs: noinline norecurse nounwind uwtable
+ define dso_local i32 @main(i32 %argc, i8** nocapture readnone %argv) local_unnamed_addr #0 {
+ entry:
+ br label %if.end
+ if.end:
+ br label %return
+ return:
+ ret i32 0
+ }
+
+ !0 = !{!"dummy metadata"}
+ !2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, nameTableKind: None)
+ !3 = !DIFile(filename: "justacall.cpp", directory: "/tmp")
+ !4 = !{}
+ !5 = !{!0}
+ !7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+ !14 = distinct !DISubprogram(name: "main", scope: !3, file: !3, line: 7, type: !15, isLocal: false, isDefinition: true, scopeLine: 8, flags: DIFlagPrototyped, isOptimized: true, unit: !2, retainedNodes: !20)
+ !15 = !DISubroutineType(types: !16)
+ !16 = !{!7, !7}
+ !20 = !{!21}
+ !21 = !DILocalVariable(name: "argc", arg: 1, scope: !14, file: !3, line: 7, type: !7)
+
+...
+---
+name: main
+alignment: 4
+exposesReturnsTwice: false
+legalized: false
+regBankSelected: false
+selected: false
+failedISel: false
+tracksRegLiveness: true
+hasWinCFI: false
+registers:
+liveins:
+ - { reg: '$edi', virtual-reg: '' }
+frameInfo:
+ isFrameAddressTaken: false
+ isReturnAddressTaken: false
+ hasStackMap: false
+ hasPatchPoint: false
+ stackSize: 0
+ offsetAdjustment: 0
+ maxAlignment: 0
+ adjustsStack: false
+ hasCalls: true
+ stackProtector: ''
+ maxCallFrameSize: 4294967295
+ cvBytesOfCalleeSavedRegisters: 0
+ hasOpaqueSPAdjustment: false
+ hasVAStart: false
+ hasMustTailInVarArgFunc: false
+ localFrameSize: 0
+ savePoint: ''
+ restorePoint: ''
+fixedStack:
+stack:
+constants:
+body: |
+ bb.0.entry:
+ successors: %bb.2(0x40000000), %bb.1(0x40000000)
+ liveins: $edi
+
+ ; Test that the DBG_VALUE on ebx below is sunk with the def of ebx, despite
+ ; not being adjacent to the def, see PR38952
+
+ DBG_VALUE $edi, $noreg, !21, !DIExpression()
+ renamable $ebx = COPY $edi
+ renamable $eax = MOV32r0 implicit-def dead $eflags
+ DBG_VALUE $ebx, $noreg, !21, !DIExpression()
+ CMP32ri $edi, 255, implicit-def $eflags
+ JG_1 %bb.2, implicit killed $eflags
+ JMP_1 %bb.1
+
+ bb.1.if.end:
+ ; CHECK-LABEL: bb.1.if.end
+ successors: %bb.2(0x80000000)
+ liveins: $ebx
+
+ ; CHECK: $ebx = COPY $edi
+ ; CHECK-NEXT: DBG_VALUE $ebx
+ renamable $rdx = MOVSX64rr32 renamable $ebx
+ renamable $rdx = nsw SHL64ri killed renamable $rdx, 2, implicit-def dead $eflags
+ ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+ $rdi = MOV32ri64 0
+ $esi = MOV32r0 implicit-def dead $eflags
+ CALL64pcrel32 &memset, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit killed $esi, implicit $rdx, implicit-def $rsp, implicit-def $ssp, implicit-def dead $rax
+ ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+
+ bb.2.return:
+ liveins: $eax
+
+ RET 0, $eax
+
+...
More information about the llvm-commits
mailing list