forked from OSchip/llvm-project
[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 llvm-svn: 345996
This commit is contained in:
Jeremy Morse
parent
82536501c7
commit
d538352b3e
|
|
@ -734,12 +734,18 @@ static bool SinkingPreventsImplicitNullCheck(MachineInstr &MI,
|
|||
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(MachineBasicBlock &CurBB,
|
|||
// 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(MachineBasicBlock &CurBB,
|
|||
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(MachineBasicBlock &CurBB,
|
|||
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;
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
...
|
||||
Loading…
Reference in New Issue