Skip to content

Commit 5d9cd3b

Browse files
jmorsejmorse
committedSep 6, 2019
[DebugInfo] LiveDebugValues: explicitly terminate overwritten stack locations
If a stack spill location is overwritten by another spill instruction, any variable locations pointing at that slot should be terminated. We cannot rely on spills always being restored to registers or variable locations being moved by a DBG_VALUE: the register allocator is entitled to spill a value and then forget about it when it goes out of liveness. To address this, scan for memory writes to spill locations, even those we don't consider to be normal "spills". isSpillInstruction and isLocationSpill distinguish the two now. After identifying spill overwrites, terminate the open range, and insert a $noreg DBG_VALUE for that variable. Differential Revision: https://reviews.llvm.org/D66941 llvm-svn: 371193
·
llvmorg-21-initllvmorg-10.0.0-rc1
1 parent 6c0204c commit 5d9cd3b

File tree

2 files changed

+254
-12
lines changed

2 files changed

+254
-12
lines changed
 

‎llvm/lib/CodeGen/LiveDebugValues.cpp

+54-12
Original file line numberDiff line numberDiff line change
@@ -361,8 +361,18 @@ class LiveDebugValues : public MachineFunctionPass {
361361
}
362362
};
363363

364-
bool isSpillInstruction(const MachineInstr &MI, MachineFunction *MF,
365-
unsigned &Reg);
364+
/// Tests whether this instruction is a spill to a stack location.
365+
bool isSpillInstruction(const MachineInstr &MI, MachineFunction *MF);
366+
367+
/// Decide if @MI is a spill instruction and return true if it is. We use 2
368+
/// criteria to make this decision:
369+
/// - Is this instruction a store to a spill slot?
370+
/// - Is there a register operand that is both used and killed?
371+
/// TODO: Store optimization can fold spills into other stores (including
372+
/// other spills). We do not handle this yet (more than one memory operand).
373+
bool isLocationSpill(const MachineInstr &MI, MachineFunction *MF,
374+
unsigned &Reg);
375+
366376
/// If a given instruction is identified as a spill, return the spill location
367377
/// and set \p Reg to the spilled register.
368378
Optional<VarLoc::SpillLoc> isRestoreInstruction(const MachineInstr &MI,
@@ -779,16 +789,8 @@ void LiveDebugValues::transferRegisterDef(
779789
}
780790
}
781791

782-
/// Decide if @MI is a spill instruction and return true if it is. We use 2
783-
/// criteria to make this decision:
784-
/// - Is this instruction a store to a spill slot?
785-
/// - Is there a register operand that is both used and killed?
786-
/// TODO: Store optimization can fold spills into other stores (including
787-
/// other spills). We do not handle this yet (more than one memory operand).
788792
bool LiveDebugValues::isSpillInstruction(const MachineInstr &MI,
789-
MachineFunction *MF, unsigned &Reg) {
790-
SmallVector<const MachineMemOperand*, 1> Accesses;
791-
793+
MachineFunction *MF) {
792794
// TODO: Handle multiple stores folded into one.
793795
if (!MI.hasOneMemOperand())
794796
return false;
@@ -797,6 +799,14 @@ bool LiveDebugValues::isSpillInstruction(const MachineInstr &MI,
797799
return false; // This is not a spill instruction, since no valid size was
798800
// returned from either function.
799801

802+
return true;
803+
}
804+
805+
bool LiveDebugValues::isLocationSpill(const MachineInstr &MI,
806+
MachineFunction *MF, unsigned &Reg) {
807+
if (!isSpillInstruction(MI, MF))
808+
return false;
809+
800810
auto isKilledReg = [&](const MachineOperand MO, unsigned &Reg) {
801811
if (!MO.isReg() || !MO.isUse()) {
802812
Reg = 0;
@@ -865,7 +875,39 @@ void LiveDebugValues::transferSpillOrRestoreInst(MachineInstr &MI,
865875

866876
LLVM_DEBUG(dbgs() << "Examining instruction: "; MI.dump(););
867877

868-
if (isSpillInstruction(MI, MF, Reg)) {
878+
// First, if there are any DBG_VALUEs pointing at a spill slot that is
879+
// written to, then close the variable location. The value in memory
880+
// will have changed.
881+
VarLocSet KillSet;
882+
if (isSpillInstruction(MI, MF)) {
883+
Loc = extractSpillBaseRegAndOffset(MI);
884+
for (unsigned ID : OpenRanges.getVarLocs()) {
885+
const VarLoc &VL = VarLocIDs[ID];
886+
if (VL.Kind == VarLoc::SpillLocKind && VL.Loc.SpillLocation == *Loc) {
887+
// This location is overwritten by the current instruction -- terminate
888+
// the open range, and insert an explicit DBG_VALUE $noreg.
889+
//
890+
// Doing this at a later stage would require re-interpreting all
891+
// DBG_VALUes and DIExpressions to identify whether they point at
892+
// memory, and then analysing all memory writes to see if they
893+
// overwrite that memory, which is expensive.
894+
//
895+
// At this stage, we already know which DBG_VALUEs are for spills and
896+
// where they are located; it's best to fix handle overwrites now.
897+
KillSet.set(ID);
898+
MachineInstr *NewDebugInstr =
899+
BuildMI(*MF, VL.MI.getDebugLoc(), VL.MI.getDesc(),
900+
VL.MI.isIndirectDebugValue(), 0, // $noreg
901+
VL.MI.getDebugVariable(), VL.MI.getDebugExpression());
902+
Transfers.push_back({&MI, NewDebugInstr});
903+
}
904+
}
905+
OpenRanges.erase(KillSet, VarLocIDs);
906+
}
907+
908+
// Try to recognise spill and restore instructions that may create a new
909+
// variable location.
910+
if (isLocationSpill(MI, MF, Reg)) {
869911
TKind = TransferKind::TransferSpill;
870912
LLVM_DEBUG(dbgs() << "Recognized as spill: "; MI.dump(););
871913
LLVM_DEBUG(dbgs() << "Register: " << Reg << " " << printReg(Reg, TRI)

‎llvm/test/DebugInfo/MIR/X86/live-debug-values-stack-clobber.mir

+200
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
# RUN: llc -mtriple=x86_64-unknown-unknown %s -o - -run-pass=livedebugvalues | FileCheck %s
2+
#
3+
# Fix some of PR42772. Consider the code below: the arguments are forced onto
4+
# the stack by the FORCE_SPILL macro, and go out of liveness if the
5+
# "bees == 2" conditional is not taken. A spill slot is then re-used to
6+
# preserve quux over the second FORCE_SPILL, over-writing the value of either
7+
# "a" or "b". LiveDebugValues should detect when this happens, and terminate
8+
# stack-spill locations when they get overwritten by a new value.
9+
#
10+
# --------8<--------
11+
# #define FORCE_SPILL() \
12+
# __asm volatile("" : : : \
13+
# "rax", "rbx", "rcx", "rdx", "rsi", "rdi", "rbp", "r8", \
14+
# "r9", "r10", "r11", "r12", "r13", "r14", "r15")
15+
#
16+
# volatile int bees = 0;
17+
#
18+
# long int f(long int a, long int b) {
19+
# if (bees == 12)
20+
# return 3;
21+
#
22+
# FORCE_SPILL();
23+
# if (bees == 2)
24+
# return a - b;
25+
#
26+
# int quux = sum(1, 2);
27+
# FORCE_SPILL();
28+
# bees++;
29+
# return quux;
30+
# }
31+
# -------->8--------
32+
#
33+
# CHECK: ![[ANUM:[0-9]+]] = !DILocalVariable(name: "a"
34+
# CHECK: ![[BNUM:[0-9]+]] = !DILocalVariable(name: "b"
35+
#
36+
# These variables should be spilt,
37+
# CHECK-LABEL: bb.1.if.end:
38+
# CHECK: MOV64mr $rsp, 1, $noreg, 16, $noreg, killed renamable $rsi
39+
# CHECK-NEXT: DBG_VALUE $rsp, 0, ![[BNUM]], !DIExpression(
40+
# CHECK-NEXT: MOV64mr $rsp, 1, $noreg, 8, $noreg, killed renamable $rdi
41+
# CHECK-NEXT: DBG_VALUE $rsp, 0, ![[ANUM]], !DIExpression(
42+
# CHECK-NEXT: INLINEASM
43+
#
44+
# Then the location of "a" should be terminated when overwritten
45+
# CHECK-LABEL: bb.3.if.end3:
46+
# CHECK: CALL64pcrel32 @sum
47+
# CHECK-NEXT: MOV64mr $rsp, 1, $noreg, 8, $noreg, $rax
48+
# CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[ANUM]], !DIExpression()
49+
# CHECK-NEXT: INLINEASM
50+
51+
--- |
52+
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
53+
target triple = "x86_64-unknown-linux-gnu"
54+
55+
@bees = external global i32, !dbg !0
56+
57+
; Function Attrs: noinline norecurse nounwind readnone uwtable
58+
declare i64 @sum(i64, i64)
59+
60+
; Function Attrs: noinline nounwind uwtable
61+
define i64 @f(i64 %a, i64 %b) !dbg !12 {
62+
entry:
63+
br label %if.end
64+
if.end:
65+
br label %if.then2
66+
if.then2:
67+
br label %if.end3
68+
if.end3:
69+
br label %return
70+
return:
71+
ret i64 0
72+
}
73+
74+
!llvm.dbg.cu = !{!2}
75+
!llvm.module.flags = !{!8, !9, !10}
76+
77+
!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
78+
!1 = distinct !DIGlobalVariable(name: "bees", scope: !2, file: !3, line: 6, type: !6, isLocal: false, isDefinition: true)
79+
!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, nameTableKind: None)
80+
!3 = !DIFile(filename: "pr42772.c", directory: ".")
81+
!4 = !{}
82+
!5 = !{!0}
83+
!6 = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: !7)
84+
!7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
85+
!8 = !{i32 2, !"Dwarf Version", i32 4}
86+
!9 = !{i32 2, !"Debug Info Version", i32 3}
87+
!10 = !{i32 1, !"wchar_size", i32 4}
88+
!12 = distinct !DISubprogram(name: "f", scope: !3, file: !3, line: 15, type: !13, scopeLine: 15, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !16)
89+
!13 = !DISubroutineType(types: !14)
90+
!14 = !{!15, !15, !15}
91+
!15 = !DIBasicType(name: "long int", size: 64, encoding: DW_ATE_signed)
92+
!16 = !{!17, !18, !19}
93+
!17 = !DILocalVariable(name: "a", arg: 1, scope: !12, file: !3, line: 15, type: !15)
94+
!18 = !DILocalVariable(name: "b", arg: 2, scope: !12, file: !3, line: 15, type: !15)
95+
!19 = !DILocalVariable(name: "quux", scope: !12, file: !3, line: 23, type: !7)
96+
!28 = !DILocation(line: 1, column: 1, scope: !12)
97+
98+
...
99+
---
100+
name: f
101+
tracksRegLiveness: true
102+
liveins:
103+
- { reg: '$rdi', virtual-reg: '' }
104+
- { reg: '$rsi', virtual-reg: '' }
105+
frameInfo:
106+
stackSize: 72
107+
offsetAdjustment: -72
108+
maxAlignment: 8
109+
adjustsStack: true
110+
hasCalls: true
111+
cvBytesOfCalleeSavedRegisters: 48
112+
fixedStack:
113+
- { id: 0, type: spill-slot, offset: -56, size: 8, alignment: 8, stack-id: default,
114+
callee-saved-register: '$rbx', callee-saved-restored: true, debug-info-variable: '',
115+
debug-info-expression: '', debug-info-location: '' }
116+
- { id: 1, type: spill-slot, offset: -48, size: 8, alignment: 16, stack-id: default,
117+
callee-saved-register: '$r12', callee-saved-restored: true, debug-info-variable: '',
118+
debug-info-expression: '', debug-info-location: '' }
119+
- { id: 2, type: spill-slot, offset: -40, size: 8, alignment: 8, stack-id: default,
120+
callee-saved-register: '$r13', callee-saved-restored: true, debug-info-variable: '',
121+
debug-info-expression: '', debug-info-location: '' }
122+
- { id: 3, type: spill-slot, offset: -32, size: 8, alignment: 16, stack-id: default,
123+
callee-saved-register: '$r14', callee-saved-restored: true, debug-info-variable: '',
124+
debug-info-expression: '', debug-info-location: '' }
125+
- { id: 4, type: spill-slot, offset: -24, size: 8, alignment: 8, stack-id: default,
126+
callee-saved-register: '$r15', callee-saved-restored: true, debug-info-variable: '',
127+
debug-info-expression: '', debug-info-location: '' }
128+
- { id: 5, type: spill-slot, offset: -16, size: 8, alignment: 16, stack-id: default,
129+
callee-saved-register: '$rbp', callee-saved-restored: true, debug-info-variable: '',
130+
debug-info-expression: '', debug-info-location: '' }
131+
stack:
132+
- { id: 0, name: '', type: spill-slot, offset: -72, size: 8, alignment: 8,
133+
stack-id: default, callee-saved-register: '', callee-saved-restored: true,
134+
debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
135+
- { id: 1, name: '', type: spill-slot, offset: -64, size: 8, alignment: 8,
136+
stack-id: default, callee-saved-register: '', callee-saved-restored: true,
137+
debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
138+
body: |
139+
bb.0.entry:
140+
successors: %bb.4, %bb.1
141+
liveins: $rdi, $rsi, $rbp, $r15, $r14, $r13, $r12, $rbx
142+
143+
DBG_VALUE $rdi, $noreg, !17, !DIExpression(), debug-location !28
144+
DBG_VALUE $rdi, $noreg, !17, !DIExpression(), debug-location !28
145+
DBG_VALUE $rsi, $noreg, !18, !DIExpression(), debug-location !28
146+
DBG_VALUE $rsi, $noreg, !18, !DIExpression(), debug-location !28
147+
frame-setup PUSH64r killed $rbp, implicit-def $rsp, implicit $rsp, debug-location !28
148+
frame-setup PUSH64r killed $r15, implicit-def $rsp, implicit $rsp, debug-location !28
149+
frame-setup PUSH64r killed $r14, implicit-def $rsp, implicit $rsp, debug-location !28
150+
frame-setup PUSH64r killed $r13, implicit-def $rsp, implicit $rsp, debug-location !28
151+
frame-setup PUSH64r killed $r12, implicit-def $rsp, implicit $rsp, debug-location !28
152+
frame-setup PUSH64r killed $rbx, implicit-def $rsp, implicit $rsp, debug-location !28
153+
$rsp = frame-setup SUB64ri8 $rsp, 24, implicit-def dead $eflags
154+
renamable $ecx = MOV32rm $rip, 1, $noreg, @bees, $noreg, debug-location !28 :: (volatile dereferenceable load 4 from @bees)
155+
$eax = MOV32ri 3, implicit-def $rax
156+
CMP32ri8 killed renamable $ecx, 12, implicit-def $eflags, debug-location !28
157+
JCC_1 %bb.4, 4, implicit $eflags, debug-location !28
158+
159+
bb.1.if.end:
160+
successors: %bb.2, %bb.3
161+
liveins: $rdi, $rsi
162+
163+
MOV64mr $rsp, 1, $noreg, 16, $noreg, killed renamable $rsi :: (store 8 into %stack.1)
164+
MOV64mr $rsp, 1, $noreg, 8, $noreg, killed renamable $rdi :: (store 8 into %stack.0)
165+
INLINEASM &"", 1, 12, implicit-def dead early-clobber $rax, 12, implicit-def dead early-clobber $rbx, 12, implicit-def dead early-clobber $rcx, 12, implicit-def dead early-clobber $rdx, 12, implicit-def dead early-clobber $rsi, 12, implicit-def dead early-clobber $rdi, 12, implicit-def dead early-clobber $rbp, 12, implicit-def dead early-clobber $r8, 12, implicit-def dead early-clobber $r9, 12, implicit-def dead early-clobber $r10, 12, implicit-def dead early-clobber $r11, 12, implicit-def dead early-clobber $r12, 12, implicit-def dead early-clobber $r13, 12, implicit-def dead early-clobber $r14, 12, implicit-def dead early-clobber $r15, 12, implicit-def dead early-clobber $df, 12, implicit-def dead early-clobber $fpsw, 12, implicit-def dead early-clobber $eflags, debug-location !28
166+
renamable $eax = MOV32rm $rip, 1, $noreg, @bees, $noreg, debug-location !28 :: (volatile dereferenceable load 4 from @bees)
167+
CMP32ri8 killed renamable $eax, 2, implicit-def $eflags, debug-location !28
168+
JCC_1 %bb.3, 5, implicit killed $eflags, debug-location !28
169+
170+
bb.2.if.then2:
171+
successors: %bb.4
172+
173+
renamable $rax = MOV64rm $rsp, 1, $noreg, 8, $noreg :: (load 8 from %stack.0)
174+
renamable $rax = SUB64rm killed renamable $rax, $rsp, 1, $noreg, 16, $noreg, implicit-def dead $eflags, debug-location !28 :: (load 8 from %stack.1)
175+
JMP_1 %bb.4
176+
177+
bb.3.if.end3:
178+
successors: %bb.4
179+
180+
$edi = MOV32ri 1, implicit-def $rdi, debug-location !28
181+
$esi = MOV32ri 2, implicit-def $rsi, debug-location !28
182+
CALL64pcrel32 @sum, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit $rsi, implicit-def $rsp, implicit-def $ssp, implicit-def $rax, debug-location !28
183+
MOV64mr $rsp, 1, $noreg, 8, $noreg, $rax :: (store 8 into %stack.0)
184+
INLINEASM &"", 1, 12, implicit-def dead early-clobber $rax, 12, implicit-def dead early-clobber $rbx, 12, implicit-def dead early-clobber $rcx, 12, implicit-def dead early-clobber $rdx, 12, implicit-def dead early-clobber $rsi, 12, implicit-def dead early-clobber $rdi, 12, implicit-def dead early-clobber $rbp, 12, implicit-def dead early-clobber $r8, 12, implicit-def dead early-clobber $r9, 12, implicit-def dead early-clobber $r10, 12, implicit-def dead early-clobber $r11, 12, implicit-def dead early-clobber $r12, 12, implicit-def dead early-clobber $r13, 12, implicit-def dead early-clobber $r14, 12, implicit-def dead early-clobber $r15, 12, implicit-def dead early-clobber $df, 12, implicit-def dead early-clobber $fpsw, 12, implicit-def dead early-clobber $eflags, debug-location !28
185+
ADD32mi8 $rip, 1, $noreg, @bees, $noreg, 1, implicit-def dead $eflags, debug-location !28 :: (volatile store 4 into @bees), (volatile dereferenceable load 4 from @bees)
186+
renamable $rax = MOVSX64rm32 $rsp, 1, $noreg, 8, $noreg, debug-location !28 :: (load 4 from %stack.0, align 8)
187+
188+
bb.4.return:
189+
liveins: $rax
190+
191+
$rsp = frame-destroy ADD64ri8 $rsp, 24, implicit-def dead $eflags, debug-location !28
192+
$rbx = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !28
193+
$r12 = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !28
194+
$r13 = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !28
195+
$r14 = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !28
196+
$r15 = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !28
197+
$rbp = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !28
198+
RETQ $rax, debug-location !28
199+
200+
...

0 commit comments

Comments
 (0)
Please sign in to comment.