From 534e6904ea2a183f936f3cf1e28d4648e701593f Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Thu, 13 Jun 2019 10:03:17 +0000 Subject: [PATCH] [DebugInfo] Use FrameDestroy to extend stack locations to end-of-function We aim to ignore changes in variable locations during the prologue and epilogue of functions, to avoid using space documenting location changes that aren't visible. However in D61940 / r362951 this got ripped out as the previous implementation was unsound. Instead, use the FrameDestroy flag to identify when we're in the epilogue of a function, and ignore variable location changes accordingly. This fits in with existing code that examines the FrameSetup flag. Some variable locations get shuffled in modified tests as they now cover greater ranges, which is what would be expected. Some additional single-location variables are generated too. Two tests are un-xfailed, they were only xfailed due to r362951 deleting functionality they depended on. Apparently some out-of-tree backends don't accurately maintain FrameDestroy flags -- if you're an out-of-tree maintainer and see changes in variable locations disappear due to a faulty FrameDestroy flag, it's safe to back this change out. The impact is just slightly more debug info than necessary. Differential Revision: https://reviews.llvm.org/D62314 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@363245 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp | 7 +++++-- test/DebugInfo/COFF/pieces.ll | 5 +++-- test/DebugInfo/X86/dbg-addr.ll | 6 +----- test/DebugInfo/X86/debug-loc-offset.mir | 6 ------ test/DebugInfo/X86/pr19307.mir | 6 ------ 5 files changed, 9 insertions(+), 21 deletions(-) diff --git a/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp b/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp index d87aac94c04..ee375529cf9 100644 --- a/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp +++ b/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp @@ -275,9 +275,12 @@ void llvm::calculateDbgEntityHistory(const MachineFunction *MF, clobberRegisterUses(RegVars, MO.getReg(), DbgValues, LiveEntries, MI); // If this is a register def operand, it may end a debug value - // range. Ignore defs of the frame register in the prologue. + // range. Ignore frame-register defs in the epilogue and prologue, + // we expect debuggers to understand that stack-locations are + // invalid outside of the function body. else if (MO.getReg() != FrameReg || - !MI.getFlag(MachineInstr::FrameSetup)) { + (!MI.getFlag(MachineInstr::FrameDestroy) && + !MI.getFlag(MachineInstr::FrameSetup))) { for (MCRegAliasIterator AI(MO.getReg(), TRI, true); AI.isValid(); ++AI) clobberRegisterUses(RegVars, *AI, DbgValues, LiveEntries, MI); diff --git a/test/DebugInfo/COFF/pieces.ll b/test/DebugInfo/COFF/pieces.ll index 8fbdce3cbeb..2ca07aa4f5a 100644 --- a/test/DebugInfo/COFF/pieces.ll +++ b/test/DebugInfo/COFF/pieces.ll @@ -104,8 +104,9 @@ ; ASM: #APP ; ASM: #NO_APP ; ASM: movl [[offset_o_x]](%rsp), %eax # 4-byte Reload -; ASM: [[spill_o_x_end:\.Ltmp[0-9]+]]: ; ASM: retq +; ASM-NEXT: [[spill_o_x_end:\.Ltmp[0-9]+]]: +; ASM-NEXT: .Lfunc_end4: ; ASM-LABEL: .short 4423 # Record kind: S_GPROC32_ID @@ -231,7 +232,7 @@ ; 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]], "E\021O\001A\000$\000\000\000" +; ASM: .cv_def_range [[spill_o_x_start]] .Lfunc_end4, "E\021O\001A\000$\000\000\000" ; OBJ-LABEL: GlobalProcIdSym { ; OBJ: Kind: S_GPROC32_ID (0x1147) diff --git a/test/DebugInfo/X86/dbg-addr.ll b/test/DebugInfo/X86/dbg-addr.ll index 7bf83442b4c..4e27cf5754c 100644 --- a/test/DebugInfo/X86/dbg-addr.ll +++ b/test/DebugInfo/X86/dbg-addr.ll @@ -9,12 +9,8 @@ ; CHECK-LABEL: use_dbg_addr: ; CHECK: #DEBUG_VALUE: use_dbg_addr:o <- [$rsp+0] -; FIXME: Avoid the use of a single-location location list and use -; DW_AT_start_offset instead. - ; DWARF: DW_TAG_variable -; DWARF-NEXT: DW_AT_location (0x00000000 -; DWARF-NEXT: [0x{{.*}}, 0x{{.*}}): DW_OP_breg7 RSP+0) +; DWARF-NEXT: DW_AT_location (DW_OP_fbreg +0) ; DWARF-NEXT: DW_AT_name ("o") diff --git a/test/DebugInfo/X86/debug-loc-offset.mir b/test/DebugInfo/X86/debug-loc-offset.mir index 89641addc2e..be4213c8298 100644 --- a/test/DebugInfo/X86/debug-loc-offset.mir +++ b/test/DebugInfo/X86/debug-loc-offset.mir @@ -1,11 +1,5 @@ # RUN: llc -o - %s -start-after=patchable-function -filetype=obj -O0 -mtriple=i386-unknown-linux-gnu -dwarf-version=4 | llvm-dwarfdump -v - | FileCheck %s -# XFAIL: * -# Marked XFail due to the removal of "ChangingRegs" from -# DbgEntityHistoryCalculator, shortening the checked range to not reach the -# end of the function. Fixed by an about-to-land patch using the FrameDestroy -# flag to identify the end of functions. - # From the code: # # debug-loc-offset1.cc diff --git a/test/DebugInfo/X86/pr19307.mir b/test/DebugInfo/X86/pr19307.mir index a688d4b9f3e..06b95bc928a 100644 --- a/test/DebugInfo/X86/pr19307.mir +++ b/test/DebugInfo/X86/pr19307.mir @@ -1,11 +1,5 @@ # RUN: llc -o - %s -start-after=patchable-function -O0 | FileCheck %s -# XFAIL: * -# Marked XFail due to the removal of "ChangingRegs" from -# DbgEntityHistoryCalculator, shortening the checked range to not reach the -# end of the function. Fixed by an about-to-land patch using the FrameDestroy -# flag to identify the end of functions. - # Generated from the source file pr19307.cc: # #include # void parse_range(unsigned long long &offset, unsigned long long &limit, -- 2.11.0