Index: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h =================================================================== --- llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h +++ llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h @@ -629,6 +629,19 @@ /// Return true if Idx is a spill machine location. bool isSpill(LocIdx Idx) const { return LocIdxToLocID[Idx] >= NumRegs; } + /// How large is this location (aka, how wide is a value defined there?). + unsigned getLocSizeInBits(LocIdx L) const { + unsigned ID = LocIdxToLocID[L]; + if (!isSpill(L)) { + return TRI.getRegSizeInBits(Register(ID), MF.getRegInfo()); + } else { + // The slot location on the stack is uninteresting, we care about the + // position of the value within the slot (which comes with a size). + StackSlotPos Pos = locIDToSpillIdx(ID); + return Pos.first; + } + } + MLocIterator begin() { return MLocIterator(LocIdxToIDNum, 0); } MLocIterator end() { Index: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp =================================================================== --- llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp +++ llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp @@ -854,6 +854,16 @@ StackSlotPos StackIdx = locIDToSpillIdx(LocID); unsigned short Offset = StackIdx.second; + // The value in this stack slot might not be the same size as the variable + // being accessed -- this needs to be encoded in the expression. + Optional VariableSize = None; + if (Var.getFragment()) { + VariableSize = Var.getFragment()->SizeInBits; + } else if (auto Size = Var.getVariable()->getSizeInBits()) { + VariableSize = *Size; + } + unsigned ValueSize = getLocSizeInBits(*MLoc); + // TODO: support variables that are located in spill slots, with non-zero // offsets from the start of the spill slot. It would require some more // complex DIExpression calculations. This doesn't seem to be produced by @@ -863,19 +873,52 @@ // the variable is. if (Offset == 0) { const SpillLoc &Spill = SpillLocs[SpillID.id()]; - Expr = TRI.prependOffsetExpression(Expr, DIExpression::ApplyOffset, - Spill.SpillOffset); unsigned Base = Spill.SpillBase; MIB.addReg(Base); - MIB.addImm(0); - // Being on the stack makes this location indirect; if it was _already_ - // indirect though, we need to add extra indirection. See this test for - // a scenario where this happens: - // llvm/test/DebugInfo/X86/spill-nontrivial-param.ll - if (Properties.Indirect) { - std::vector Elts = {dwarf::DW_OP_deref}; - Expr = DIExpression::append(Expr, Elts); + // There are several ways we can dereference things, and several inputs + // to consider: + // * NRVO variables will appear with IsIndirect set, but should have + // nothing else in their DIExpressions, + // * Variables with DW_OP_stack_value in their expr already need an + // explicit dereference of the stack location, + // * Values that don't match the variable size need DW_OP_deref_size, + // * Everything else can just become a simple location expression. + + if (Properties.Indirect) { + // This is something like an NRVO variable, where the pointer has been + // spilt to the stack. It should end up being a memory location, with + // the pointer to the variable loaded off the stack with a deref: + assert(!Expr->isComplex()); + Expr = TRI.prependOffsetExpression(Expr, DIExpression::ApplyOffset | + DIExpression::DerefAfter, + Spill.SpillOffset); + MIB.addImm(0); + } else if (VariableSize && *VariableSize != ValueSize) { + // We're loading a value off the stack that's not the same size as the + // variable. Add / subtract stack offset, explicitly deref with a size, + // and add DW_OP_stack_value if not already present. + assert((ValueSize % 8) == 0); + SmallVector Ops = {dwarf::DW_OP_deref_size, ValueSize / 8}; + Expr = DIExpression::prependOpcodes(Expr, Ops, true); + unsigned Flags = DIExpression::StackValue | DIExpression::ApplyOffset; + Expr = TRI.prependOffsetExpression(Expr, Flags, Spill.SpillOffset); + MIB.addReg(0); + } else if (Expr->isComplex()) { + // A variable with no size ambiguity, but with extra elements in it's + // expression. Manually dereference the stack location. + assert(Expr->isComplex()); + Expr = TRI.prependOffsetExpression(Expr, DIExpression::ApplyOffset | + DIExpression::DerefAfter, + Spill.SpillOffset); + MIB.addReg(0); + } else { + // A plain value that has been spilt to the stack, with no further + // context. Request a location expression, marking the DBG_VALUE as + // IsIndirect. + Expr = TRI.prependOffsetExpression(Expr, DIExpression::ApplyOffset, + Spill.SpillOffset); + MIB.addImm(0); } } else { // This is a stack location with a weird subregister offset: emit an undef Index: llvm/test/DebugInfo/MIR/InstrRef/deref-spills-with-size.mir =================================================================== --- /dev/null +++ llvm/test/DebugInfo/MIR/InstrRef/deref-spills-with-size.mir @@ -0,0 +1,197 @@ +# RUN: llc %s -o - -experimental-debug-variable-locations=true \ +# RUN: -run-pass=livedebugvalues \ +# RUN: | FileCheck %s --implicit-check-not=DBG_VALUE +# RUN: llc %s -o - -experimental-debug-variable-locations=true \ +# RUN: -start-before=livedebugvalues -filetype=obj \ +# RUN: | llvm-dwarfdump - | FileCheck %s --check-prefix=DWARF +# +# LLVM can produce DIExpressions that convert from one value of arbitrary size +# to another. This is normally fine, however that means the value for a +# variable tracked in instruction referencing might not be the same size as the +# variable itself. +# +# This becomes a problem when values move onto the stack and we emit +# DW_OP_deref: there is no information about how large a value the consumer +# should load from the stack. The convention today appears to be the size of +# the variable, and as a result if you have a value that's smaller than the +# variable size, extra information is loaded onto the DWARF expression stack. +# This then appears as a false variable value to the consumer. +# +# In this test, take a 32 bit variable and point it at an 8 bit register, which +# is then spilt. Ensure that DW_OP_deref_size is used, rather than a plain +# DW_OP_deref which is open to interpretation. +# +## Capture variable num, +# CHECK: ![[VARNUM:[0-9]+]] = !DILocalVariable + +# Checks for DWARF location list up front: +# DWARF: DW_TAG_variable +# DWARF-NEXT: DW_AT_location +# DWARF-NEXT: DW_OP_breg0 RAX+0, DW_OP_constu 0xff, DW_OP_and, +# DWARF-SAME: DW_OP_convert (0x00000026) "DW_ATE_signed_8", +# DWARF-SAME: DW_OP_convert (0x0000002a) "DW_ATE_signed_32", DW_OP_stack_value +# DWARF-NEXT: DW_OP_breg7 RSP-8, DW_OP_deref_size 0x1, +# DWARF-SAME: DW_OP_convert (0x00000026) "DW_ATE_signed_8", +# DWARF-SAME: DW_OP_convert (0x0000002a) "DW_ATE_signed_32", DW_OP_stack_value +## First define-on-stack location. +# DWARF-NEXT: DW_OP_breg7 RSP-8, DW_OP_deref_size 0x1, +# DWARF-SAME: DW_OP_convert (0x00000026) "DW_ATE_signed_8", +# DWARF-SAME: DW_OP_convert (0x0000002a) "DW_ATE_signed_32", DW_OP_stack_value +## Tests with meaningless converts, on 32 bit values. +# DWARF-NEXT: DW_OP_breg0 RAX+0, DW_OP_constu 0xffffffff, DW_OP_and, +# DWARF-SAME: DW_OP_convert (0x0000002a) "DW_ATE_signed_32", +# DWARF-SAME: DW_OP_convert (0x0000002a) "DW_ATE_signed_32", DW_OP_stack_value +# DWARF-NEXT: DW_OP_breg7 RSP-8, DW_OP_deref, +# DWARF-SAME: DW_OP_convert (0x0000002a) "DW_ATE_signed_32", +# DWARF-SAME: DW_OP_convert (0x0000002a) "DW_ATE_signed_32", DW_OP_stack_value +# DWARF-NEXT: DW_OP_breg7 RSP-8, DW_OP_deref, +# DWARF-SAME: DW_OP_convert (0x0000002a) "DW_ATE_signed_32", +# DWARF-SAME: DW_OP_convert (0x0000002a) "DW_ATE_signed_32", DW_OP_stack_value +## Tests with no DIExpression. +# DWARF-NEXT: DW_OP_reg0 RAX +# DWARF-NEXT: DW_OP_breg7 RSP-8, DW_OP_deref_size 0x1, DW_OP_stack_value +# DWARF-NEXT: DW_OP_breg7 RSP-8, DW_OP_deref_size 0x1, DW_OP_stack_value) + +--- | + ; ModuleID = 'missingvar.ll' + source_filename = "a" + target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" + target triple = "x86_64-unknown-linux-gnu" + + define linkonce_odr void @_ZNSt5dequeIPN4llvm4LoopESaIS2_EE13_M_insert_auxESt15_Deque_iteratorIS2_RS2_PS2_EmRKS2_() local_unnamed_addr align 2 !dbg !3 { + entry: + call void @llvm.dbg.value(metadata i32 0, metadata !8, metadata !DIExpression()), !dbg !7 + ret void + } + + declare void @llvm.dbg.value(metadata, metadata, metadata) + + !llvm.module.flags = !{!0, !9} + !llvm.dbg.cu = !{!1} + + !0 = !{i32 2, !"Debug Info Version", i32 3} + !1 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !2, producer: "beards", isOptimized: true, runtimeVersion: 4, emissionKind: FullDebug) + !2 = !DIFile(filename: "bees.cpp", directory: "") + !3 = distinct !DISubprogram(name: "nope", scope: !2, file: !2, line: 1, type: !4, spFlags: DISPFlagDefinition, unit: !1) + !4 = !DISubroutineType(types: !5) + !5 = !{!6} + !6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) + !7 = !DILocation(line: 1, scope: !3) + !8 = !DILocalVariable(name: "flannel", scope: !3, type: !6) + !9 = !{i32 2, !"Dwarf Version", i32 5} + +... +--- +name: _ZNSt5dequeIPN4llvm4LoopESaIS2_EE13_M_insert_auxESt15_Deque_iteratorIS2_RS2_PS2_EmRKS2_ +alignment: 16 +tracksRegLiveness: true +liveins: + - { reg: '$rdi' } + - { reg: '$rsi' } + - { reg: '$rdx' } +frameInfo: + stackSize: 48 + offsetAdjustment: -48 + maxAlignment: 8 + maxCallFrameSize: 0 + cvBytesOfCalleeSavedRegisters: 48 +fixedStack: + - { id: 0, type: spill-slot, offset: -56, size: 8, alignment: 8, callee-saved-register: '$rbx' } + - { id: 1, type: spill-slot, offset: -48, size: 8, alignment: 16, callee-saved-register: '$r12' } + - { id: 2, type: spill-slot, offset: -40, size: 8, alignment: 8, callee-saved-register: '$r13' } + - { id: 3, type: spill-slot, offset: -32, size: 8, alignment: 16, callee-saved-register: '$r14' } + - { id: 4, type: spill-slot, offset: -24, size: 8, alignment: 8, callee-saved-register: '$r15' } + - { id: 5, type: spill-slot, offset: -16, size: 8, alignment: 16, callee-saved-register: '$rbp' } +stack: + - { id: 0, type: spill-slot, offset: -64, size: 8, alignment: 8 } +machineFunctionInfo: {} +body: | + bb.0.entry: + liveins: $rdi, $rdx, $rsi, $rbp, $r15, $r14, $r13, $r12, $rbx + + ; CHECK-LABEL: bb.0.entry: + + $al = MOV8ri 0, debug-instr-number 1, debug-location !7 + DBG_INSTR_REF 1, 0, !8, !DIExpression(DW_OP_LLVM_convert, 8, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value), debug-location !7 + ; CHECK: DBG_VALUE $al, $noreg, ![[VARNUM]], + ; CHECK-SAME: !DIExpression(DW_OP_LLVM_convert, 8, DW_ATE_signed, + ; CHECK-SAME : DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value) + + MOV8mr $rsp, 1, $noreg, -8, $noreg, renamable $al :: (store 1 into %stack.0) + ; Clobber to force variable location onto stack. + $al = MOV8ri 0, debug-location !7 + ; CHECK: DBG_VALUE $rsp, $noreg, ![[VARNUM]], + ; CHECK-SAME: !DIExpression(DW_OP_constu, 8, DW_OP_minus, + ; CHECK-SAME: DW_OP_deref_size, 1, DW_OP_LLVM_convert, 8, DW_ATE_signed, + ; CHECK-SAME: DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value), + + ; Terminate the current variable location, + DBG_VALUE $noreg, $noreg, !8, !DIExpression(), debug-location !7 + ; CHECK: DBG_VALUE $noreg, $noreg, ![[VARNUM]], !DIExpression() + + ; Try again, but with the value originating on the stack, to ensure that + ; we can find its size. + INC8m $rsp, 1, $noreg, 4, $noreg, implicit-def dead $eflags, debug-instr-number 2, debug-location !7 :: (store (s8) into %stack.0) + DBG_INSTR_REF 2, 1000000, !8, !DIExpression(DW_OP_LLVM_convert, 8, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value), debug-location !7 + ; CHECK: DBG_VALUE $rsp, $noreg, ![[VARNUM]], + ; CHECK-SAME: !DIExpression(DW_OP_constu, 8, DW_OP_minus, + ; CHECK-SAME: DW_OP_deref_size, 1, DW_OP_LLVM_convert, 8, DW_ATE_signed, + ; CHECK-SAME: DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value), + + ; Just for certainty, let's do it all again but with the correct + ; sizes, and ensure they get the right expressions. With a meaningless + ; convert from 32 bits to 32 bits baked in. + + $eax = MOV32ri 0, debug-instr-number 3, debug-location !7 + DBG_INSTR_REF 3, 0, !8, !DIExpression(DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value), debug-location !7 + ; CHECK: DBG_VALUE $eax, $noreg, ![[VARNUM]], + ; CHECK-SAME: !DIExpression(DW_OP_LLVM_convert, 32, DW_ATE_signed, + ; CHECK-SAME: DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value), + + MOV32mr $rsp, 1, $noreg, -8, $noreg, renamable $eax :: (store 4 into %stack.0) + ; Clobber to force variable location onto stack. + $eax = MOV32ri 0, debug-location !7 + ; CHECK: DBG_VALUE $rsp, $noreg, ![[VARNUM]], + ; CHECK-SAME: !DIExpression(DW_OP_constu, 8, DW_OP_minus, + ; CHECK-SAME: DW_OP_deref, DW_OP_LLVM_convert, 32, DW_ATE_signed, + ; CHECK-SAME: DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value), + + DBG_VALUE $noreg, $noreg, !8, !DIExpression(), debug-location !7 + ; CHECK: DBG_VALUE $noreg, $noreg, ![[VARNUM]], !DIExpression(), + + INC32m $rsp, 1, $noreg, 4, $noreg, implicit-def dead $eflags, debug-instr-number 4, debug-location !7 :: (store (s32) into %stack.0) + DBG_INSTR_REF 4, 1000000, !8, !DIExpression(DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value), debug-location !7 + ; CHECK: DBG_VALUE $rsp, $noreg, ![[VARNUM]], + ; CHECK-SAME: !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_deref, + ; CHECK-SAME: DW_OP_LLVM_convert, 32, DW_ATE_signed, + ; CHECK-SAME: DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value), + + ; Repeat again -- this time with no contents in the DIExpression. This test + ; that when we switch between a plain register location and a DWARF location + ; expression, we add a DW_OP_stack_value. We can't use a non-stack-value + ; location expression, because the difference between the variable size and + ; the value size will not be described. + + $al = MOV8ri 0, debug-instr-number 5, debug-location !7 + DBG_INSTR_REF 5, 0, !8, !DIExpression(), debug-location !7 + ; CHECK: DBG_VALUE $al, $noreg, ![[VARNUM]], !DIExpression() + + MOV32mr $rsp, 1, $noreg, -8, $noreg, renamable $eax :: (store 4 into %stack.0) + ; Clobber to force variable location onto stack. + $eax = MOV32ri 0, debug-location !7 + ; CHECK: DBG_VALUE $rsp, $noreg, ![[VARNUM]], + ; CHECK-SAME: !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_deref_size, 1, DW_OP_stack_value) + + DBG_VALUE $noreg, $noreg, !8, !DIExpression(), debug-location !7 + ; CHECK: DBG_VALUE $noreg, $noreg, ![[VARNUM]], !DIExpression(), + + ; We can tell from the memory operand that a single-byte value is defined, + ; which should be recognised as "variable size doesn't match value size". + INC8m $rsp, 1, $noreg, 4, $noreg, implicit-def dead $eflags, debug-instr-number 6, debug-location !7 :: (store (s8) into %stack.0) + DBG_INSTR_REF 6, 1000000, !8, !DIExpression(), debug-location !7 + ; CHECK: DBG_VALUE $rsp, $noreg, ![[VARNUM]], + ; CHECK-SAME: !DIExpression(DW_OP_constu, 8, DW_OP_minus, + ; CHECK-SAME: DW_OP_deref_size, 1, DW_OP_stack_value), + + RET64 +... Index: llvm/test/DebugInfo/MIR/X86/live-debug-values-restore.mir =================================================================== --- llvm/test/DebugInfo/MIR/X86/live-debug-values-restore.mir +++ llvm/test/DebugInfo/MIR/X86/live-debug-values-restore.mir @@ -409,15 +409,12 @@ # This third function tests that complex expressions are spilt, and restored # correctly within a basic block. -# FIXME: the spilt location below is wrong, there should be a deref between -# the spill-offset and the DW_OP_plus_uconst. - # CHECK-LABEL: name: h # CHECK-LABEL: bb.0.entry: # CHECK: DBG_VALUE $rdi, $noreg, ![[RVAR]], !DIExpression(DW_OP_plus_uconst, 1) # CHECK-LABEL: bb.1.if.then: # CHECK: DBG_VALUE $rdi, $noreg, ![[RVAR]], !DIExpression(DW_OP_plus_uconst, 1) -# CHECK: DBG_VALUE $rsp, 0, ![[RVAR]], !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_plus_uconst, 1) +# CHECK: DBG_VALUE $rsp, $noreg, ![[RVAR]], !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_deref, DW_OP_plus_uconst, 1) # CHECK: DBG_VALUE $rdi, $noreg, ![[RVAR]], !DIExpression(DW_OP_plus_uconst, 1) # CHECK-LABEL: bb.2.if.end: # CHECK: DBG_VALUE $rdi, $noreg, ![[RVAR]], !DIExpression(DW_OP_plus_uconst, 1) @@ -519,8 +516,6 @@ # bb 2 and 3, neither of which modifies the stack loc. The exit block (3) # should still be tracking the spill, and restore it on stack load. -# FIXME: this test too contains a broken spill location. - # Summary: loc is in $rdi in bb0, spills to stack in bb1, remains in stack # in bb2, starts in stack then loaded in bb3. @@ -529,11 +524,11 @@ # CHECK: DBG_VALUE $rdi, $noreg, ![[SVAR]], !DIExpression(DW_OP_plus_uconst, 1) # CHECK-LABEL: bb.1.foo: # CHECK: DBG_VALUE $rdi, $noreg, ![[SVAR]], !DIExpression(DW_OP_plus_uconst, 1) -# CHECK: DBG_VALUE $rsp, 0, ![[SVAR]], !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_plus_uconst, 1) +# CHECK: DBG_VALUE $rsp, $noreg, ![[SVAR]], !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_deref, DW_OP_plus_uconst, 1) # CHECK-LABEL: bb.2.if.then: -# CHECK: DBG_VALUE $rsp, 0, ![[SVAR]], !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_plus_uconst, 1) +# CHECK: DBG_VALUE $rsp, $noreg, ![[SVAR]], !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_deref, DW_OP_plus_uconst, 1) # CHECK-LABEL: bb.3.if.end -# CHECK: DBG_VALUE $rsp, 0, ![[SVAR]], !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_plus_uconst, 1) +# CHECK: DBG_VALUE $rsp, $noreg, ![[SVAR]], !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_deref, DW_OP_plus_uconst, 1) # CHECK: DBG_VALUE $rdi, $noreg, ![[SVAR]], !DIExpression(DW_OP_plus_uconst, 1) name: i alignment: 16 Index: llvm/test/DebugInfo/MIR/X86/livedebugvalues_load_in_loop.mir =================================================================== --- llvm/test/DebugInfo/MIR/X86/livedebugvalues_load_in_loop.mir +++ llvm/test/DebugInfo/MIR/X86/livedebugvalues_load_in_loop.mir @@ -52,7 +52,7 @@ !3 = !{!4} !4 = !DIGlobalVariableExpression(var: !5, expr: !DIExpression()) !5 = distinct !DIGlobalVariable(name: "start", scope: !0, file: !1, line: 4, type: !6, isLocal: false, isDefinition: true) - !6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) + !6 = !DIBasicType(name: "int", size: 64, encoding: DW_ATE_signed) !7 = !{i32 2, !"Dwarf Version", i32 4} !8 = !{i32 2, !"Debug Info Version", i32 3} !9 = !{i32 1, !"wchar_size", i32 2}