[llvm] 1e40799 - [DebugInfo] Teach LDV how to handle identical variable fragments
via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 11 02:26:08 PST 2020
Author: OCHyams
Date: 2020-02-11T10:20:24Z
New Revision: 1e4079932436474d6a358637e47b8da5b73f1947
URL: https://github.com/llvm/llvm-project/commit/1e4079932436474d6a358637e47b8da5b73f1947
DIFF: https://github.com/llvm/llvm-project/commit/1e4079932436474d6a358637e47b8da5b73f1947.diff
LOG: [DebugInfo] Teach LDV how to handle identical variable fragments
LiveDebugVariables uses interval maps to explicitly represent DBG_VALUE
intervals. DBG_VALUEs are filtered into an interval map based on their {
Variable, DIExpression }. The interval map will coalesce adjacent entries that
use the same { Location }. Under this model, DBG_VALUEs which refer to the same
bits of the same variable will be filtered into different interval maps if they
have different DIExpressions which means the original intervals will not be
properly preserved.
This patch fixes the problem by using { Variable, Fragment } to filter the
DBG_VALUEs into maps, and coalesces adjacent entries iff they have the same
{ Location, DIExpression } pair.
The solution is not perfect because we see the similar issues appear when
partially overlapping fragments are encountered, but is far simpler than a
complete solution (i.e. D70121).
Fixes: pr41992, pr43957
Reviewed By: aprantl
Differential Revision: https://reviews.llvm.org/D74053
Added:
llvm/test/DebugInfo/X86/live-debug-vars-intervals.mir
Modified:
llvm/lib/CodeGen/LiveDebugVariables.cpp
llvm/test/tools/llvm-locstats/locstats.ll
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/LiveDebugVariables.cpp b/llvm/lib/CodeGen/LiveDebugVariables.cpp
index 5b20a2482b7b..8712c6edc1f0 100644
--- a/llvm/lib/CodeGen/LiveDebugVariables.cpp
+++ b/llvm/lib/CodeGen/LiveDebugVariables.cpp
@@ -96,18 +96,19 @@ LiveDebugVariables::LiveDebugVariables() : MachineFunctionPass(ID) {
enum : unsigned { UndefLocNo = ~0U };
-/// Describes a location by number along with some flags about the original
-/// usage of the location.
+/// Describes a debug variable value by location number and expression along
+/// with some flags about the original usage of the location.
class DbgValueLocation {
public:
- DbgValueLocation(unsigned LocNo, bool WasIndirect)
- : LocNo(LocNo), WasIndirect(WasIndirect) {
- static_assert(sizeof(*this) == sizeof(unsigned), "bad bitfield packing");
+ DbgValueLocation(unsigned LocNo, bool WasIndirect,
+ const DIExpression &Expression)
+ : LocNo(LocNo), WasIndirect(WasIndirect), Expression(&Expression) {
assert(locNo() == LocNo && "location truncation");
}
DbgValueLocation() : LocNo(0), WasIndirect(0) {}
+ const DIExpression *getExpression() const { return Expression; }
unsigned locNo() const {
// Fix up the undef location number, which gets truncated.
return LocNo == INT_MAX ? UndefLocNo : LocNo;
@@ -116,12 +117,13 @@ class DbgValueLocation {
bool isUndef() const { return locNo() == UndefLocNo; }
DbgValueLocation changeLocNo(unsigned NewLocNo) const {
- return DbgValueLocation(NewLocNo, WasIndirect);
+ return DbgValueLocation(NewLocNo, WasIndirect, *Expression);
}
friend inline bool operator==(const DbgValueLocation &LHS,
const DbgValueLocation &RHS) {
- return LHS.LocNo == RHS.LocNo && LHS.WasIndirect == RHS.WasIndirect;
+ return LHS.LocNo == RHS.LocNo && LHS.WasIndirect == RHS.WasIndirect &&
+ LHS.Expression == RHS.Expression;
}
friend inline bool operator!=(const DbgValueLocation &LHS,
@@ -132,6 +134,7 @@ class DbgValueLocation {
private:
unsigned LocNo : 31;
unsigned WasIndirect : 1;
+ const DIExpression *Expression = nullptr;
};
/// Map of where a user value is live, and its location.
@@ -156,6 +159,8 @@ class LDVImpl;
/// closure of that relation.
class UserValue {
const DILocalVariable *Variable; ///< The debug info variable we are part of.
+ // FIXME: This is only used to get the FragmentInfo that describes the part
+ // of the variable we are a part of. We should just store the FragmentInfo.
const DIExpression *Expression; ///< Any complex address expression.
DebugLoc dl; ///< The debug location for the variable. This is
///< used by dwarf writer to find lexical scope.
@@ -205,9 +210,19 @@ class UserValue {
/// Does this UserValue match the parameters?
bool match(const DILocalVariable *Var, const DIExpression *Expr,
const DILocation *IA) const {
- // FIXME: The fragment should be part of the equivalence class, but not
- // other things in the expression like stack values.
- return Var == Variable && Expr == Expression && dl->getInlinedAt() == IA;
+ // FIXME: Handle partially overlapping fragments.
+ // A DBG_VALUE with a fragment which overlaps a previous DBG_VALUE fragment
+ // for the same variable terminates the interval opened by the first.
+ // getUserValue() uses match() to filter DBG_VALUEs into interval maps to
+ // represent these intervals.
+ // Given two _partially_ overlapping fragments match() will always return
+ // false. The DBG_VALUEs will be filtered into separate interval maps and
+ // therefore we do not faithfully represent the original intervals.
+ // See D70121#1849741 for a more detailed explanation and further
+ // discussion.
+ return Var == Variable &&
+ Expr->getFragmentInfo() == Expression->getFragmentInfo() &&
+ dl->getInlinedAt() == IA;
}
/// Merge equivalence classes.
@@ -285,8 +300,9 @@ class UserValue {
void mapVirtRegs(LDVImpl *LDV);
/// Add a definition point to this value.
- void addDef(SlotIndex Idx, const MachineOperand &LocMO, bool IsIndirect) {
- DbgValueLocation Loc(getLocationNo(LocMO), IsIndirect);
+ void addDef(SlotIndex Idx, const MachineOperand &LocMO, bool IsIndirect,
+ const DIExpression &Expr) {
+ DbgValueLocation Loc(getLocationNo(LocMO), IsIndirect, Expr);
// Add a singular (Idx,Idx) -> Loc mapping.
LocMap::iterator I = locInts.find(Idx);
if (!I.valid() || I.start() != Idx)
@@ -320,12 +336,11 @@ class UserValue {
/// possible.
///
/// \param LI Scan for copies of the value in LI->reg.
- /// \param LocNo Location number of LI->reg.
- /// \param WasIndirect Indicates if the original use of LI->reg was indirect
+ /// \param Loc Location number of LI->reg.
/// \param Kills Points where the range of LocNo could be extended.
/// \param [in,out] NewDefs Append (Idx, LocNo) of inserted defs here.
void addDefsFromCopies(
- LiveInterval *LI, unsigned LocNo, bool WasIndirect,
+ LiveInterval *LI, DbgValueLocation Loc,
const SmallVectorImpl<SlotIndex> &Kills,
SmallVectorImpl<std::pair<SlotIndex, DbgValueLocation>> &NewDefs,
MachineRegisterInfo &MRI, LiveIntervals &LIS);
@@ -663,11 +678,11 @@ bool LDVImpl::handleDebugValue(MachineInstr &MI, SlotIndex Idx) {
UserValue *UV =
getUserValue(Var, Expr, MI.getDebugLoc());
if (!Discard)
- UV->addDef(Idx, MI.getOperand(0), IsIndirect);
+ UV->addDef(Idx, MI.getOperand(0), IsIndirect, *Expr);
else {
MachineOperand MO = MachineOperand::CreateReg(0U, false);
MO.setIsDebug();
- UV->addDef(Idx, MO, false);
+ UV->addDef(Idx, MO, false, *Expr);
}
return true;
}
@@ -775,7 +790,7 @@ void UserValue::extendDef(SlotIndex Idx, DbgValueLocation Loc, LiveRange *LR,
}
void UserValue::addDefsFromCopies(
- LiveInterval *LI, unsigned LocNo, bool WasIndirect,
+ LiveInterval *LI, DbgValueLocation Loc,
const SmallVectorImpl<SlotIndex> &Kills,
SmallVectorImpl<std::pair<SlotIndex, DbgValueLocation>> &NewDefs,
MachineRegisterInfo &MRI, LiveIntervals &LIS) {
@@ -805,7 +820,7 @@ void UserValue::addDefsFromCopies(
// it, or we are looking at a wrong value of LI.
SlotIndex Idx = LIS.getInstructionIndex(*MI);
LocMap::iterator I = locInts.find(Idx.getRegSlot(true));
- if (!I.valid() || I.value().locNo() != LocNo)
+ if (!I.valid() || I.value() != Loc)
continue;
if (!LIS.hasInterval(DstReg))
@@ -839,7 +854,7 @@ void UserValue::addDefsFromCopies(
MachineInstr *CopyMI = LIS.getInstructionFromIndex(DstVNI->def);
assert(CopyMI && CopyMI->isCopy() && "Bad copy value");
unsigned LocNo = getLocationNo(CopyMI->getOperand(0));
- DbgValueLocation NewLoc(LocNo, WasIndirect);
+ DbgValueLocation NewLoc = Loc.changeLocNo(LocNo);
I.insert(Idx, Idx.getNextSlot(), NewLoc);
NewDefs.push_back(std::make_pair(Idx, NewLoc));
break;
@@ -887,8 +902,7 @@ void UserValue::computeIntervals(MachineRegisterInfo &MRI,
// sub-register in that regclass). For now, simply skip handling copies if
// a sub-register is involved.
if (LI && !LocMO.getSubReg())
- addDefsFromCopies(LI, Loc.locNo(), Loc.wasIndirect(), Kills, Defs, MRI,
- LIS);
+ addDefsFromCopies(LI, Loc, Kills, Defs, MRI, LIS);
continue;
}
@@ -1329,7 +1343,7 @@ void UserValue::insertDebugValue(MachineBasicBlock *MBB, SlotIndex StartIdx,
// original DBG_VALUE was indirect, we need to add DW_OP_deref to indicate
// that the original virtual register was a pointer. Also, add the stack slot
// offset for the spilled register to the expression.
- const DIExpression *Expr = Expression;
+ const DIExpression *Expr = Loc.getExpression();
uint8_t DIExprFlags = DIExpression::ApplyOffset;
bool IsIndirect = Loc.wasIndirect();
if (Spilled) {
diff --git a/llvm/test/DebugInfo/X86/live-debug-vars-intervals.mir b/llvm/test/DebugInfo/X86/live-debug-vars-intervals.mir
new file mode 100644
index 000000000000..f215b50eef34
--- /dev/null
+++ b/llvm/test/DebugInfo/X86/live-debug-vars-intervals.mir
@@ -0,0 +1,154 @@
+# These tests check that DBG_VALUE intervals are correctly coalesced (or not) in
+# LiveDebugVariables.
+# NOTE: We do not currently handle partially overlapping fragments in LDV.
+#
+# The IR in this file has been modified by hand, generated from this source:
+# void escape(int *);
+# extern int global;
+# void f(int x) {
+# escape(&x);
+# x = 1;
+# global = x;
+# x = 2;
+# escape(&x);
+# }
+
+# RUN: llc %s -start-before=machine-scheduler -stop-after=virtregrewriter -o - \
+# RUN: | FileCheck %s --implicit-check-not=DBG_VALUE
+
+# Verify that DBG_VALUEs with same { Variable, Fragment } but
diff erent DIExpressions
+# are not coalesced.
+#
+# DV1 %0 "x" (deref) --+-- Do not coalesce these because DV2 refers to the same bits.
+# DV2 %0 "x" () |
+# DV3 %0 "x" (deref) --+
+#
+# CHECK: name: f1
+# CHECK: DBG_VALUE %stack.0.x.addr, $noreg, ![[VAR:[0-9]+]], !DIExpression(DW_OP_deref)
+# CHECK: DBG_VALUE %stack.0.x.addr, $noreg, ![[VAR]], !DIExpression()
+# CHECK: DBG_VALUE %stack.0.x.addr, $noreg, ![[VAR]], !DIExpression(DW_OP_deref)
+
+# Verify that DBG_VALUEs with same { Variable, Fragment } and { Location, DIExpression } are
+# coalesced if there are no intervening DBG_VALUEs with the same { Variable, Fragment }.
+#
+# DV1 %0 "x" (fragment 00 16) --+-- Coalesce these because DV2 refers to
diff erent bits.
+# DV2 %0 "x" (fragment 16 16) |
+# DV3 %0 "x" (fragment 00 16) --+
+#
+# CHECK: name: f2
+# CHECK: DBG_VALUE %stack.0.x.addr, $noreg, ![[VAR:[0-9]+]], !DIExpression(DW_OP_deref, DW_OP_LLVM_fragment, 0, 16)
+# CHECK: DBG_VALUE %stack.0.x.addr, $noreg, ![[VAR]], !DIExpression(DW_OP_LLVM_fragment, 16, 16)
+
+--- |
+ target triple = "x86_64-unknown-linux-gnu"
+
+ @global = external global i32, align 4
+ declare void @llvm.dbg.value(metadata, metadata, metadata)
+ declare void @escape(i32*)
+
+ define void @f1(i32 %x) !dbg !6 {
+ entry:
+ %x.addr = alloca i32, align 4
+ store i32 %x, i32* %x.addr, align 4
+ call void @llvm.dbg.value(metadata i32* %x.addr, metadata !11, metadata !DIExpression(DW_OP_deref)), !dbg !12
+ call void @escape(i32* %x.addr), !dbg !12
+ call void @llvm.dbg.value(metadata i32* %x.addr, metadata !11, metadata !DIExpression()), !dbg !12
+ store i32 1, i32* @global, align 4, !dbg !12
+ call void @llvm.dbg.value(metadata i32* %x.addr, metadata !11, metadata !DIExpression(DW_OP_deref)), !dbg !12
+ store i32 2, i32* %x.addr, align 4, !dbg !12
+ call void @escape(i32* %x.addr), !dbg !12
+ ret void, !dbg !12
+ }
+
+ define void @f2(i32 %x) !dbg !13 {
+ entry:
+ %x.addr = alloca i32, align 4
+ store i32 %x, i32* %x.addr, align 4
+ call void @llvm.dbg.value(metadata i32* %x.addr, metadata !14, metadata !DIExpression(DW_OP_deref, DW_OP_LLVM_fragment, 0, 16)), !dbg !15
+ call void @escape(i32* %x.addr)
+ call void @llvm.dbg.value(metadata i32* %x.addr, metadata !14, metadata !DIExpression(DW_OP_LLVM_fragment, 16, 16)), !dbg !15
+ store i32 1, i32* @global, align 4
+ call void @llvm.dbg.value(metadata i32* %x.addr, metadata !14, metadata !DIExpression(DW_OP_deref, DW_OP_LLVM_fragment, 0, 16)), !dbg !15
+ store i32 2, i32* %x.addr, align 4
+ call void @escape(i32* %x.addr)
+ ret void
+ }
+
+ !llvm.dbg.cu = !{!0}
+ !llvm.module.flags = !{!3, !4}
+ !llvm.ident = !{!5}
+
+ !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 11.0.0 ", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+ !1 = !DIFile(filename: "test.c", directory: "")
+ !2 = !{}
+ !3 = !{i32 2, !"Dwarf Version", i32 4}
+ !4 = !{i32 2, !"Debug Info Version", i32 3}
+ !5 = !{!"clang version 11.0.0 "}
+ !6 = distinct !DISubprogram(name: "f1", scope: !1, file: !1, line: 3, type: !7, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !10)
+ !7 = !DISubroutineType(types: !8)
+ !8 = !{null, !9}
+ !9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+ !10 = !{!11}
+ !11 = !DILocalVariable(name: "x", arg: 1, scope: !6, file: !1, line: 3, type: !9)
+ !12 = !DILocation(line: 3, column: 12, scope: !6)
+ !13 = distinct !DISubprogram(name: "f2", scope: !1, file: !1, line: 20, type: !7, scopeLine: 20, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !10)
+ !14 = !DILocalVariable(name: "x", arg: 1, scope: !13, file: !1, line: 21, type: !9)
+ !15 = !DILocation(line: 23, column: 12, scope: !13)
+
+...
+---
+name: f1
+tracksRegLiveness: true
+stack:
+ - { id: 0, name: x.addr, type: default, offset: 0, size: 4, alignment: 4,
+ stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+ debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+body: |
+ bb.0.entry:
+ liveins: $edi
+ %0:gr32 = COPY $edi
+ MOV32mr %stack.0.x.addr, 1, $noreg, 0, $noreg, %0 :: (store 4 into %ir.x.addr)
+ DBG_VALUE %stack.0.x.addr, $noreg, !11, !DIExpression(DW_OP_deref), debug-location !12
+ ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp, debug-location !12
+ %1:gr64 = LEA64r %stack.0.x.addr, 1, $noreg, 0, $noreg
+ $rdi = COPY %1, debug-location !12
+ CALL64pcrel32 @escape, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp, debug-location !12
+ ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp, debug-location !12
+ DBG_VALUE %stack.0.x.addr, $noreg, !11, !DIExpression(), debug-location !12
+ MOV32mi $rip, 1, $noreg, @global, $noreg, 1, debug-location !12 :: (store 4 into @global)
+ DBG_VALUE %stack.0.x.addr, $noreg, !11, !DIExpression(DW_OP_deref), debug-location !12
+ MOV32mi %stack.0.x.addr, 1, $noreg, 0, $noreg, 2, debug-location !12 :: (store 4 into %ir.x.addr)
+ ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp, debug-location !12
+ $rdi = COPY %1, debug-location !12
+ CALL64pcrel32 @escape, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp, debug-location !12
+ ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp, debug-location !12
+ RET 0, debug-location !12
+...
+---
+name: f2
+tracksRegLiveness: true
+stack:
+ - { id: 0, name: x.addr, type: default, offset: 0, size: 4, alignment: 4,
+ stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+ debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+body: |
+ bb.0.entry:
+ liveins: $edi
+ %0:gr32 = COPY $edi
+ MOV32mr %stack.0.x.addr, 1, $noreg, 0, $noreg, %0 :: (store 4 into %ir.x.addr)
+ DBG_VALUE %stack.0.x.addr, $noreg, !14, !DIExpression(DW_OP_deref, DW_OP_LLVM_fragment, 0, 16), debug-location !15
+ ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+ %1:gr64 = LEA64r %stack.0.x.addr, 1, $noreg, 0, $noreg
+ $rdi = COPY %1
+ CALL64pcrel32 @escape, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp
+ ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+ DBG_VALUE %stack.0.x.addr, $noreg, !14, !DIExpression(DW_OP_LLVM_fragment, 16, 16), debug-location !15
+ MOV32mi $rip, 1, $noreg, @global, $noreg, 1 :: (store 4 into @global)
+ DBG_VALUE %stack.0.x.addr, $noreg, !14, !DIExpression(DW_OP_deref, DW_OP_LLVM_fragment, 0, 16), debug-location !15
+ MOV32mi %stack.0.x.addr, 1, $noreg, 0, $noreg, 2 :: (store 4 into %ir.x.addr)
+ ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+ $rdi = COPY %1
+ CALL64pcrel32 @escape, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp
+ ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+ RET 0
+...
diff --git a/llvm/test/tools/llvm-locstats/locstats.ll b/llvm/test/tools/llvm-locstats/locstats.ll
index e267b7eeb1f9..f16635d2e8e4 100644
--- a/llvm/test/tools/llvm-locstats/locstats.ll
+++ b/llvm/test/tools/llvm-locstats/locstats.ll
@@ -11,9 +11,9 @@
; LOCSTATS: [30%,40%) 0 0%
; LOCSTATS: [40%,50%) 1 11%
; LOCSTATS: [50%,60%) 1 11%
-; LOCSTATS: [60%,70%) 0 0%
+; LOCSTATS: [60%,70%) 1 11%
; LOCSTATS: [70%,80%) 0 0%
-; LOCSTATS: [80%,90%) 3 33%
+; LOCSTATS: [80%,90%) 2 22%
; LOCSTATS: [90%,100%) 1 11%
; LOCSTATS: 100% 2 22%
;
More information about the llvm-commits
mailing list