[DebugInfo][InstrRef] Prefer stack locations for variables over restored registers

[jmorse]

Summary

This patch adjusts what location is picked for a known variable value -- preferring to leave locations on the stack, even when a value is re-loaded into a register. The benefit is reduced location list entropy, on a clang-3.4 build I found that .debug_loclists reduces in size by 6%, from 29Mb down to 27Mb. The kind of change is demonstrated in the modifications to the spill-nospill.ll test below.

There's a slight risk from the fact that we don't terminate stack variable location at the end of the function, when the stack pointer gets modified a lot during frame destruction. The policy we follow right now [0] is that we expect consumers to know stack locations aren't valid outside the function body. Doing this more often increases the damage if a consumer doesn't have the same expectations. CC @dblaikie , who had an opinion on this last time it came up.

Testing: a few tests need the stack slot to be written to explicitly, to force LiveDebugValues into restoring the variable location to a register. I've added an explicit test for the desired behaviour in livedebugvalues_recover_clobbers.mir .

[0] https://github.com/llvm/llvm-project/blob/843d1eda18c3a7a700fe0858748e175727498d21/llvm/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp#L502

[jmorse]

(Remembering to upload the modified tests this time)

[dblaikie]

CC @dblaikie , who had an opinion on this last time it came up.

Got a link to the previous discussion/context?

[jmorse]

A sample of things are D63083, https://bugs.llvm.org/show_bug.cgi?id=40188#c8 , D61940. Although looking back, they all appear to be about not making complicated location-list entries in the prologue, rather than the epilogue.

[dblaikie]

In D120732#3353016, @jmorse wrote:

A sample of things are D63083, https://bugs.llvm.org/show_bug.cgi?id=40188#c8 , D61940. Although looking back, they all appear to be about not making complicated location-list entries in the prologue, rather than the epilogue.

Doing it in either place seems wasteful to me. The broad logic is: At -O0 we manage to succeed by using a stack-only location, which seems to imply that the consumers don't try to use that location in the prologue or epilogue (since the stack would be wrong in parts of those regions). So when we do more fine-grained locations, I think the same logic should apply and we shouldn't try to be accurate in those areas - and choose whatever representation is easy to generate and/or dense/efficient to encode. Does that seem reasonable/make sense?

[Orlando]

FWIW the change LGTM mechanically and I think it sounds reasonable (/good).

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

for (MCRegAliasIterator RAI(Reg, TRI, true); RAI.isValid(); ++RAI)
       MTracker->defReg(*RAI, CurBB, CurInst);
 
     // Now find subregisters within the destination register, and load values
     // from stack slot positions.
     auto DoTransfer = [&](Register DestReg, unsigned SpillID) {
       LocIdx SrcIdx = MTracker->getSpillMLoc(SpillID);
       auto ReadValue = MTracker->readMLoc(SrcIdx);
       MTracker->setReg(DestReg, ReadValue);
-
-      if (TTracker) {
-        LocIdx DstLoc = MTracker->getRegMLoc(DestReg);
-        TTracker->transferMlocs(SrcIdx, DstLoc, MI.getIterator());
-      }
     };
 
     for (MCSubRegIterator SRI(Reg, TRI, false); SRI.isValid(); ++SRI) {
       unsigned Subreg = TRI->getSubRegIndex(Reg, *SRI);
       unsigned SpillID = MTracker->getLocID(*Loc, Subreg);
       DoTransfer(*SRI, SpillID);
     }
 

llvm/test/DebugInfo/COFF/pieces.ll

 ; ASM:         xorl    %ecx, %ecx
 ; ASM:         callq   g
 ; ASM:         movl    %eax, [[offset_o_x:[0-9]+]](%rsp)          # 4-byte Spill
 ; ASM: [[spill_o_x_start:\.Ltmp[0-9]+]]:
 ; ASM:         #DEBUG_VALUE: bitpiece_spill:o <- [DW_OP_plus_uconst [[offset_o_x]], DW_OP_LLVM_fragment 32 32] [$rsp+0]
 ; ASM:         #APP
 ; ASM:         #NO_APP
 ; ASM:         movl    [[offset_o_x]](%rsp), %eax          # 4-byte Reload
-; ASM: [[spill_o_x_end:\.Ltmp[0-9]+]]:
-; ASM:         #DEBUG_VALUE: bitpiece_spill:o <- [DW_OP_LLVM_fragment 32 32] $eax
 ; ASM:         retq
 ; ASM: [[spill_o_x_end_func:\.Ltmp[0-9]+]]:
 ; ASM-NEXT: .Lfunc_end4:
 
 
 ; ASM-LABEL:  .short  4423                    # Record kind: S_GPROC32_ID
 ; ASM:        .asciz  "loop_csr"              # Function name
 ; ASM:        .short  4414                    # Record kind: S_LOCAL

 ; OBJ:       ProcEnd {
 ; OBJ:       }
 
 
 ; ASM-LABEL:  .short  4423                    # Record kind: S_GPROC32_ID
 ; ASM:        .asciz  "bitpiece_spill"        # Function name
 ; ASM:        .short  4414                    # Record kind: S_LOCAL
 ; ASM:        .asciz  "o"
-; ASM:        .cv_def_range    [[spill_o_x_start]] [[spill_o_x_end]], reg_rel, 335, 65, 36
-; ASM:        .cv_def_range    [[spill_o_x_end]] .Lfunc_end4, subfield_reg, 17, 4
+; ASM:        .cv_def_range    [[spill_o_x_start]] .Lfunc_end4, reg_rel, 335, 65, 36
 
 ; OBJ-LABEL: GlobalProcIdSym {
 ; OBJ:         Kind: S_GPROC32_ID (0x1147)
 ; OBJ:         DisplayName: bitpiece_spill
 ; OBJ:       }
 ; OBJ:       LocalSym {
 ; OBJ:         VarName: o
 ; OBJ:       }

llvm/test/DebugInfo/MIR/InstrRef/livedebugvalues_recover_clobbers.mir

bb.0.entry:
     $ebx = MOV32ri 0, debug-location !17
 
     ; CHECK-NEXT: $ebx = MOV32ri 2
     ; CHECK-NEXT: MOV32mr $rsp
     ; CHECK-NEXT: DBG_VALUE $ebx
     ; CHECK-NEXT: $ebx = MOV32ri
     ; CHECK-NEXT: DBG_VALUE $rsp
 
+    ;; Try re-loading the variable value from the stack; we shouldn't issue a
+    ;; DBG_VALUE to follow it, the stack locations are usually longer lived and
+    ;; this reduces location list entropy. Then, clobber the stack location,
+    ;; and check that we can relocate the variable to being in the loaded
+    ;; register.
+    $ebx = MOV32rm $rsp, 1, _, -12, _ :: (load 4 from %stack.0)
+    MOV32mr $rsp, 1, _, -12, _, killed $esi :: (store 4 into %stack.0)
+    ; CHECK:      $ebx = MOV32rm
+    ; CHECK-NEXT: MOV32mr $rsp
+    ; CHECK-NEXT: DBG_VALUE $ebx
+
     ;; Now test copies and register masks.
     $eax = COPY $ebx
     DBG_VALUE $ebx, $noreg, !16, !DIExpression(), debug-location !17
     ;; Overwrite ebx with a copy.
     $ecx = MOV32ri 1, debug-location !17
     $ebx = COPY $ecx
 
     ; CHECK:      DBG_VALUE $ebx

llvm/test/DebugInfo/MIR/X86/live-debug-values-restore.mir

 # CHECK: ![[TVAR:[0-9]+]] = !DILocalVariable(name: "t",
 
 # Ascertain that the spill has been recognized and manifested in a DBG_VALUE.
 # CHECK: MOV64mr $rsp,{{.*-8.*}}killed{{.*}}$rdi :: (store (s64) into %stack.0)
 # CHECK-NEXT: DBG_VALUE $rsp,{{.*}}![[PVAR]],{{.*}}!DIExpression(DW_OP_constu, 8, DW_OP_minus)
 
 # Check for the restore.
 # CHECK: $rdi = MOV64rm $rsp,{{.*-8.*}}:: (load (s64) from %stack.0)
+# CHECK-NEXT: $rax = MOV64ri 0
+# CHECK-NEXT: MOV64mr
 # CHECK-NEXT: DBG_VALUE $rdi,{{.*}}![[PVAR]], !DIExpression()
 
 --- |
   define dso_local i32 @f(i32* readonly %p) local_unnamed_addr !dbg !7 {
   entry:
     call void @llvm.dbg.value(metadata i32* %p, metadata !13, metadata !DIExpression()), !dbg !14
     %tobool = icmp eq i32* %p, null, !dbg !15
     br i1 %tobool, label %if.end, label %if.then, !dbg !17
bb.1.if.then:
     CFI_INSTRUCTION offset $r12, -48
     CFI_INSTRUCTION offset $r13, -40
     CFI_INSTRUCTION offset $r14, -32
     CFI_INSTRUCTION offset $r15, -24
     CFI_INSTRUCTION offset $rbp, -16
     MOV64mr $rsp, 1, $noreg, -8, $noreg, killed renamable $rdi :: (store (s64) into %stack.0)
     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 $eflags, !20, debug-location !18
     renamable $rdi = MOV64rm $rsp, 1, $noreg, -8, $noreg :: (load (s64) from %stack.0)
+    ; Clobber stack location to force variable location to move to $rdi.
+    $rax = MOV64ri 0
+    MOV64mr $rsp, 1, _, -8, _, killed renamable $rax :: (store (s64) into %stack.0)
     $rbx = frame-destroy POP64r implicit-def $rsp, implicit $rsp
     CFI_INSTRUCTION def_cfa_offset 48
     $r12 = frame-destroy POP64r implicit-def $rsp, implicit $rsp
     CFI_INSTRUCTION def_cfa_offset 40
     $r13 = frame-destroy POP64r implicit-def $rsp, implicit $rsp
     CFI_INSTRUCTION def_cfa_offset 32
     $r14 = frame-destroy POP64r implicit-def $rsp, implicit $rsp
     CFI_INSTRUCTION def_cfa_offset 24
bb.1.if.then:
     CFI_INSTRUCTION offset $r12, -48
     CFI_INSTRUCTION offset $r13, -40
     CFI_INSTRUCTION offset $r14, -32
     CFI_INSTRUCTION offset $r15, -24
     CFI_INSTRUCTION offset $rbp, -16
     MOV64mr $rsp, 1, $noreg, -8, $noreg, killed renamable $rdi :: (store (s64) into %stack.0)
     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 $eflags, !220, debug-location !218
     renamable $rdi = MOV64rm $rsp, 1, $noreg, -8, $noreg :: (load (s64) from %stack.0)
+    ; Clobber stack location to force variable location to move to $rdi.
+    $rax = MOV64ri 0
+    MOV64mr $rsp, 1, _, -8, _, killed renamable $rax :: (store (s64) into %stack.0)
     $rbx = frame-destroy POP64r implicit-def $rsp, implicit $rsp
     CFI_INSTRUCTION def_cfa_offset 48
     $r12 = frame-destroy POP64r implicit-def $rsp, implicit $rsp
     CFI_INSTRUCTION def_cfa_offset 40
     $r13 = frame-destroy POP64r implicit-def $rsp, implicit $rsp
     CFI_INSTRUCTION def_cfa_offset 32
     $r14 = frame-destroy POP64r implicit-def $rsp, implicit $rsp
     CFI_INSTRUCTION def_cfa_offset 24
bb.2.if.then:
     CFI_INSTRUCTION def_cfa_offset 16
     $rbp = frame-destroy POP64r implicit-def $rsp, implicit $rsp
     CFI_INSTRUCTION def_cfa_offset 8
   
   bb.3.if.end:
     liveins: $rbx, $r12, $r13, $r14, $r15, $rbp
   
     renamable $rdi = MOV64rm $rsp, 1, $noreg, -8, $noreg :: (load (s64) from %stack.0)
+    ; Clobber stack location to force variable location to move to $rdi.
+    $rax = MOV64ri 0
+    MOV64mr $rsp, 1, _, -8, _, killed renamable $rax :: (store (s64) into %stack.0)
     renamable $eax = MOV32rm killed renamable $rdi, 1, $noreg, 4, $noreg, debug-location !323 :: (load (s32) from %ir.add.ptr, !tbaa !24)
     RET64 $eax, debug-location !328
 
 ...
 ---
 # Test that if an unspilt DBG_VALUE starts as an indirect DBG_VALUE, then it
 # is restored as an indirect DBG_VALUE. FIXME: Note that for the intervening
 # period of being a spilt location there is still a missing layer of
bb.1.if.then:
     CFI_INSTRUCTION offset $r12, -48
     CFI_INSTRUCTION offset $r13, -40
     CFI_INSTRUCTION offset $r14, -32
     CFI_INSTRUCTION offset $r15, -24
     CFI_INSTRUCTION offset $rbp, -16
     MOV64mr $rsp, 1, $noreg, -8, $noreg, killed renamable $rdi :: (store (s64) into %stack.0)
     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 $eflags, !20, debug-location !409
     renamable $rdi = MOV64rm $rsp, 1, $noreg, -8, $noreg :: (load (s64) from %stack.0)
+    ; Clobber stack location to force variable location to move to $rdi.
+    $rax = MOV64ri 0
+    MOV64mr $rsp, 1, _, -8, _, killed renamable $rax :: (store (s64) into %stack.0)
     $rbx = frame-destroy POP64r implicit-def $rsp, implicit $rsp
     CFI_INSTRUCTION def_cfa_offset 48
     $r12 = frame-destroy POP64r implicit-def $rsp, implicit $rsp
     CFI_INSTRUCTION def_cfa_offset 40
     $r13 = frame-destroy POP64r implicit-def $rsp, implicit $rsp
     CFI_INSTRUCTION def_cfa_offset 32
     $r14 = frame-destroy POP64r implicit-def $rsp, implicit $rsp
     CFI_INSTRUCTION def_cfa_offset 24

llvm/test/DebugInfo/X86/pr34545.ll

 ; RUN: llc -O1 -filetype=asm -mtriple x86_64-unknown-linux-gnu -mcpu=x86-64 \
 ; RUN:    -o - %s -stop-after=livedebugvars \
 ; RUN:    -experimental-debug-variable-locations=false \
-; RUN: | FileCheck %s --check-prefixes=CHECK
+; RUN: | FileCheck %s --check-prefixes=CHECK,VARLOCS
 ; RUN: llc -O1 -filetype=asm -mtriple x86_64-unknown-linux-gnu -mcpu=x86-64 \
 ; RUN:    -o - %s -stop-after=livedebugvars \
 ; RUN:    -experimental-debug-variable-locations=true \
 ; RUN: | FileCheck %s --check-prefixes=CHECK,INSTRREF
 
 ; CHECK:         $eax = MOV32rm
 ; INSTRREF-SAME:      debug-instr-number 1
 ; INSTRREF:      DBG_INSTR_REF 1, 0
 ; CHECK:         DBG_VALUE $eax
 ; CHECK:         $eax = SHL32rCL killed renamable $eax,
 ; INSTRREF-SAME:      debug-instr-number 2
 ; INSTRREF:      DBG_INSTR_REF 2, 0
 ; CHECK:         DBG_VALUE $eax
 ; CHECK:         DBG_VALUE $rsp, 0, !{{[0-9]+}}, !DIExpression(DW_OP_constu, 4, DW_OP_minus)
-; CHECK:         DBG_VALUE $eax
+; VARLOCS:       DBG_VALUE $eax
 ; CHECK:         $eax = SHL32rCL killed renamable $eax,
 ; INSTRREF-SAME:      debug-instr-number 3
 ; INSTRREF:      DBG_INSTR_REF 3, 0
 ; CHECK:         DBG_VALUE $eax
 ; CHECK:         RET64 $eax
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"

llvm/test/DebugInfo/X86/spill-nospill.ll

 ; CHECK: #DEBUG_VALUE: f:y <- $esi
 ; CHECK: movl    %eax, %ecx
 ; CHECK: callq   g
 ; CHECK: movl    %[[CSR]], %ecx
 ; CHECK: callq   g
 ; CHECK: movl    %[[CSR]], %ecx
 ; CHECK: callq   g
 ; CHECK: movl    [[X_OFFS]](%rsp), %eax          # 4-byte Reload
-; CHECK: #DEBUG_VALUE: f:x <- $eax
+;; Variable value remains on stack, location left pointing there.
 ; CHECK: addl    %[[CSR]], %eax
 
 ; DWARF:      DW_TAG_variable
 ; DWARF-NEXT:   DW_AT_location        (
-; DWARF-NEXT:      [{{.*}}, {{.*}}): DW_OP_breg7 RSP+36
-; DWARF-NEXT:      [{{.*}}, {{.*}}): DW_OP_reg0 RAX
 ; DWARF-NEXT:      [{{.*}}, {{.*}}): DW_OP_breg7 RSP+36)
 ; DWARF-NEXT:   DW_AT_name    ("x")
 
 ; DWARF:      DW_TAG_variable
 ; DWARF-NEXT:   DW_AT_location        (
 ; DWARF-NEXT:      [{{.*}},  {{.*}}): DW_OP_reg4 RSI)
 ; DWARF-NEXT:   DW_AT_name    ("y")