[llvm] r369092 - [DebugInfo] Handle complex expressions with spills in LiveDebugValues

Hans Wennborg via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 20 02:39:53 PDT 2019


Wrong thread.. it was r369026 that got merged to release_90. Sorry for
the noise.


On Tue, Aug 20, 2019 at 11:34 AM Hans Wennborg <hans at chromium.org> wrote:
>
> Merged to release_90 in r369354. Thanks!
>
> On Fri, Aug 16, 2019 at 12:03 PM Jeremy Morse via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> >
> > Author: jmorse
> > Date: Fri Aug 16 03:04:17 2019
> > New Revision: 369092
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=369092&view=rev
> > Log:
> > [DebugInfo] Handle complex expressions with spills in LiveDebugValues
> >
> > In r369026 we disabled spill-recognition in LiveDebugValues for anything
> > that has a complex expression. This is because it's hard to recover the
> > complex expression once the spill location is baked into it.
> >
> > This patch re-enables spill-recognition and slightly adjusts the DBG_VALUE
> > insts that LiveDebugValues tracks: instead of tracking the last DBG_VALUE
> > for a variable, it tracks the last _unspilt_ DBG_VALUE. The spill-restore
> > code is then able to access and copy the original complex expression; but
> > the rest of LiveDebugValues has to be aware of the slight semantic shift,
> > and produce a new spilt location if a spilt location is propagated between
> > blocks.
> >
> > The test added produces an incorrect variable location (see FIXME), which
> > will be the subject of future work.
> >
> > Differential Revision: https://reviews.llvm.org/D65368
> >
> > Modified:
> >     llvm/trunk/lib/CodeGen/LiveDebugValues.cpp
> >     llvm/trunk/test/DebugInfo/MIR/X86/live-debug-values-restore.mir
> >
> > Modified: llvm/trunk/lib/CodeGen/LiveDebugValues.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LiveDebugValues.cpp?rev=369092&r1=369091&r2=369092&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/CodeGen/LiveDebugValues.cpp (original)
> > +++ llvm/trunk/lib/CodeGen/LiveDebugValues.cpp Fri Aug 16 03:04:17 2019
> > @@ -227,8 +227,8 @@ private:
> >
> >      /// The constructor for spill locations.
> >      VarLoc(const MachineInstr &MI, unsigned SpillBase, int SpillOffset,
> > -           LexicalScopes &LS)
> > -        : Var(MI), MI(MI), UVS(MI.getDebugLoc(), LS) {
> > +           LexicalScopes &LS, const MachineInstr &OrigMI)
> > +        : Var(MI), MI(OrigMI), UVS(MI.getDebugLoc(), LS) {
> >        assert(MI.isDebugValue() && "not a DBG_VALUE");
> >        assert(MI.getNumOperands() == 4 && "malformed DBG_VALUE");
> >        Kind = SpillLocKind;
> > @@ -567,11 +567,7 @@ void LiveDebugValues::transferDebugValue
> >      ID = VarLocIDs.insert(VL);
> >      OpenRanges.insert(ID, VL.Var);
> >    } else if (MI.hasOneMemOperand()) {
> > -    // It's a stack spill -- fetch spill base and offset.
> > -    VarLoc::SpillLoc SpillLocation = extractSpillBaseRegAndOffset(MI);
> > -    VarLoc VL(MI, SpillLocation.SpillBase, SpillLocation.SpillOffset, LS);
> > -    ID = VarLocIDs.insert(VL);
> > -    OpenRanges.insert(ID, VL.Var);
> > +    llvm_unreachable("DBG_VALUE with mem operand encountered after regalloc?");
> >    } else {
> >      // This must be an undefined location. We should leave OpenRanges closed.
> >      assert(MI.getOperand(0).isReg() && MI.getOperand(0).getReg() == 0 &&
> > @@ -678,7 +674,7 @@ void LiveDebugValues::insertTransferDebu
> >          *MF, DebugInstr->getDebugLoc(), DebugInstr->getDesc(), true,
> >          SpillLocation.SpillBase, DebugInstr->getDebugVariable(), SpillExpr);
> >      VarLoc VL(*NewDebugInstr, SpillLocation.SpillBase,
> > -              SpillLocation.SpillOffset, LS);
> > +              SpillLocation.SpillOffset, LS, *DebugInstr);
> >      ProcessVarLoc(VL, NewDebugInstr);
> >      LLVM_DEBUG(dbgs() << "Creating DBG_VALUE inst for spill: ";
> >                 NewDebugInstr->print(dbgs(), /*IsStandalone*/false,
> > @@ -691,17 +687,11 @@ void LiveDebugValues::insertTransferDebu
> >             "No register supplied when handling a restore of a debug value");
> >      MachineFunction *MF = MI.getMF();
> >      DIBuilder DIB(*const_cast<Function &>(MF->getFunction()).getParent());
> > -
> > -    const DIExpression *NewExpr;
> > -    if (auto Fragment = DebugInstr->getDebugExpression()->getFragmentInfo())
> > -      NewExpr = *DIExpression::createFragmentExpression(DIB.createExpression(),
> > -        Fragment->OffsetInBits, Fragment->SizeInBits);
> > -    else
> > -      NewExpr = DIB.createExpression();
> > -
> > -    NewDebugInstr =
> > -        BuildMI(*MF, DebugInstr->getDebugLoc(), DebugInstr->getDesc(), false,
> > -                NewReg, DebugInstr->getDebugVariable(), NewExpr);
> > +    // DebugInstr refers to the pre-spill location, therefore we can reuse
> > +    // its expression.
> > +    NewDebugInstr = BuildMI(
> > +        *MF, DebugInstr->getDebugLoc(), DebugInstr->getDesc(), false, NewReg,
> > +        DebugInstr->getDebugVariable(), DebugInstr->getDebugExpression());
> >      VarLoc VL(*NewDebugInstr, LS);
> >      ProcessVarLoc(VL, NewDebugInstr);
> >      LLVM_DEBUG(dbgs() << "Creating DBG_VALUE inst for register restore: ";
> > @@ -856,14 +846,9 @@ void LiveDebugValues::transferSpillOrRes
> >                        << "\n");
> >    }
> >    // Check if the register or spill location is the location of a debug value.
> > -  // FIXME: Don't create a spill transfer if there is a complex expression,
> > -  // because we currently cannot recover the original expression on restore.
> >    for (unsigned ID : OpenRanges.getVarLocs()) {
> > -    const MachineInstr *DebugInstr = &VarLocIDs[ID].MI;
> > -
> >      if (TKind == TransferKind::TransferSpill &&
> > -        VarLocIDs[ID].isDescribedByReg() == Reg &&
> > -        !DebugInstr->getDebugExpression()->isComplex()) {
> > +        VarLocIDs[ID].isDescribedByReg() == Reg) {
> >        LLVM_DEBUG(dbgs() << "Spilling Register " << printReg(Reg, TRI) << '('
> >                          << VarLocIDs[ID].Var.getVar()->getName() << ")\n");
> >      } else if (TKind == TransferKind::TransferRestore &&
> > @@ -1121,13 +1106,24 @@ bool LiveDebugValues::join(
> >                     DebugInstr->getDebugVariable(),
> >                     DebugInstr->getDebugExpression());
> >      } else {
> > +      auto *DebugExpr = DebugInstr->getDebugExpression();
> > +      Register Reg = DebugInstr->getOperand(0).getReg();
> > +      bool IsIndirect = DebugInstr->isIndirectDebugValue();
> > +
> > +      if (DiffIt.Kind == VarLoc::SpillLocKind) {
> > +        // This is is a spilt location; DebugInstr refers to the unspilt
> > +        // location. We need to rebuild the spilt location expression and
> > +        // point the DBG_VALUE at the frame register.
> > +        DebugExpr = DIExpression::prepend(DebugInstr->getDebugExpression(),
> > +                                          DIExpression::ApplyOffset,
> > +                                          DiffIt.Loc.SpillLocation.SpillOffset);
> > +        Reg = TRI->getFrameRegister(*DebugInstr->getMF());
> > +        IsIndirect = true;
> > +      }
> > +
> >        MI = BuildMI(MBB, MBB.instr_begin(), DebugInstr->getDebugLoc(),
> > -                   DebugInstr->getDesc(), DebugInstr->isIndirectDebugValue(),
> > -                   DebugInstr->getOperand(0).getReg(),
> > -                   DebugInstr->getDebugVariable(),
> > -                   DebugInstr->getDebugExpression());
> > -      if (DebugInstr->isIndirectDebugValue())
> > -        MI->getOperand(1).setImm(DebugInstr->getOperand(1).getImm());
> > +                   DebugInstr->getDesc(), IsIndirect, Reg,
> > +                   DebugInstr->getDebugVariable(), DebugExpr);
> >      }
> >      LLVM_DEBUG(dbgs() << "Inserted: "; MI->dump(););
> >      ILS.set(ID);
> >
> > Modified: llvm/trunk/test/DebugInfo/MIR/X86/live-debug-values-restore.mir
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/MIR/X86/live-debug-values-restore.mir?rev=369092&r1=369091&r2=369092&view=diff
> > ==============================================================================
> > --- llvm/trunk/test/DebugInfo/MIR/X86/live-debug-values-restore.mir (original)
> > +++ llvm/trunk/test/DebugInfo/MIR/X86/live-debug-values-restore.mir Fri Aug 16 03:04:17 2019
> > @@ -2,6 +2,8 @@
> >
> >  # Generated from the following source with:
> >  # clang -g -mllvm -stop-before=livedebugvalues -S -O2 test.c -o test.mir
> > +# Then more functions added to test for extra behaviours with complex
> > +# expressions.
> >
> >  # #define FORCE_SPILL() \
> >  #   __asm volatile("" : : : \
> > @@ -14,9 +16,10 @@
> >  #   return *(p + 1);
> >  # }
> >
> > -# Pick out DILocalVariable numbers for "p" and "q"
> > +# Pick out DILocalVariable numbers for "p", "q" and "r"
> >  # CHECK: ![[PVAR:[0-9]+]] = !DILocalVariable(name: "p",
> >  # CHECK: ![[QVAR:[0-9]+]] = !DILocalVariable(name: "q",
> > +# CHECK: ![[RVAR:[0-9]+]] = !DILocalVariable(name: "r",
> >
> >  # Ascertain that the spill has been recognized and manifested in a DBG_VALUE.
> >  # CHECK: MOV64mr $rsp,{{.*-8.*}}killed{{.*}}$rdi :: (store 8 into %stack.0)
> > @@ -59,6 +62,23 @@
> >      ret i32 %0, !dbg !128
> >    }
> >
> > +  define dso_local i32 @h(i32* readonly %p) local_unnamed_addr !dbg !207 {
> > +  entry:
> > +    call void @llvm.dbg.value(metadata i32* %p, metadata !213, metadata !DIExpression()), !dbg !214
> > +    %tobool = icmp eq i32* %p, null, !dbg !215
> > +    br i1 %tobool, label %if.end, label %if.then, !dbg !217
> > +
> > +  if.then:                                          ; preds = %entry
> > +    tail call void asm sideeffect "", "~{rax},~{rbx},~{rcx},~{rdx},~{rsi},~{rdi},~{rbp},~{r8},~{r9},~{r10},~{r11},~{r12},~{r13},~{r14},~{r15},~{dirflag},~{fpsr},~{flags}"(), !dbg !218, !srcloc !220
> > +    br label %if.end, !dbg !221
> > +
> > +  if.end:                                           ; preds = %entry, %if.then
> > +    %add.ptr = getelementptr inbounds i32, i32* %p, i64 1, !dbg !222
> > +    %0 = load i32, i32* %add.ptr, align 4, !dbg !223, !tbaa !24
> > +    ret i32 %0, !dbg !228
> > +  }
> > +
> > +
> >    declare void @llvm.dbg.value(metadata, metadata, metadata)
> >
> >    !llvm.dbg.cu = !{!0}
> > @@ -95,7 +115,7 @@
> >    !27 = !{!"Simple C/C++ TBAA"}
> >    !28 = !DILocation(line: 9, column: 3, scope: !7)
> >    !101 = !DIBasicType(name: "looong int", size: 64, encoding: DW_ATE_signed)
> > -  !107 = distinct !DISubprogram(name: "g", scope: !1, file: !1, line: 105, type: !8, scopeLine: 105, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !112)
> > +  !107 = distinct !DISubprogram(name: "g", scope: !0, file: !1, line: 105, type: !8, scopeLine: 105, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !112)
> >    !112 = !{!113}
> >    !113 = !DILocalVariable(name: "q", arg: 1, scope: !107, file: !1, line: 105, type: !101)
> >    !114 = !DILocation(line: 105, column: 12, scope: !107)
> > @@ -109,8 +129,21 @@
> >    !122 = !DILocation(line: 109, column: 14, scope: !107)
> >    !123 = !DILocation(line: 109, column: 10, scope: !107)
> >    !128 = !DILocation(line: 109, column: 3, scope: !107)
> > -
> > -
> > +  !201 = !DIBasicType(name: "looong int", size: 64, encoding: DW_ATE_signed)
> > +  !207 = distinct !DISubprogram(name: "g", scope: !0, file: !1, line: 105, type: !8, scopeLine: 105, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !212)
> > +  !212 = !{!213}
> > +  !213 = !DILocalVariable(name: "r", arg: 1, scope: !207, file: !1, line: 105, type: !201)
> > +  !214 = !DILocation(line: 105, column: 12, scope: !207)
> > +  !215 = !DILocation(line: 106, column: 7, scope: !216)
> > +  !216 = distinct !DILexicalBlock(scope: !207, file: !1, line: 106, column: 7)
> > +  !217 = !DILocation(line: 106, column: 7, scope: !207)
> > +  !218 = !DILocation(line: 107, column: 5, scope: !219)
> > +  !219 = distinct !DILexicalBlock(scope: !216, file: !1, line: 106, column: 10)
> > +  !220 = !{i32 -2147471544}
> > +  !221 = !DILocation(line: 108, column: 3, scope: !219)
> > +  !222 = !DILocation(line: 109, column: 14, scope: !207)
> > +  !223 = !DILocation(line: 109, column: 10, scope: !207)
> > +  !228 = !DILocation(line: 109, column: 3, scope: !207)
> >  ...
> >  ---
> >  name:            f
> > @@ -298,3 +331,111 @@ body:             |
> >      RETQ $eax, debug-location !128
> >
> >  ...
> > +---
> > +# 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 $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)
> > +
> > +name:            h
> > +alignment:       4
> > +tracksRegLiveness: true
> > +liveins:
> > +  - { reg: '$rdi', virtual-reg: '' }
> > +frameInfo:
> > +  stackSize:       48
> > +  offsetAdjustment: -48
> > +  maxAlignment:    8
> > +  cvBytesOfCalleeSavedRegisters: 48
> > +  localFrameSize:  0
> > +fixedStack:
> > +  - { id: 0, type: spill-slot, offset: -56, size: 8, alignment: 8, stack-id: default,
> > +      callee-saved-register: '$rbx', callee-saved-restored: true, debug-info-variable: '',
> > +      debug-info-expression: '', debug-info-location: '' }
> > +  - { id: 1, type: spill-slot, offset: -48, size: 8, alignment: 16, stack-id: default,
> > +      callee-saved-register: '$r12', callee-saved-restored: true, debug-info-variable: '',
> > +      debug-info-expression: '', debug-info-location: '' }
> > +  - { id: 2, type: spill-slot, offset: -40, size: 8, alignment: 8, stack-id: default,
> > +      callee-saved-register: '$r13', callee-saved-restored: true, debug-info-variable: '',
> > +      debug-info-expression: '', debug-info-location: '' }
> > +  - { id: 3, type: spill-slot, offset: -32, size: 8, alignment: 16, stack-id: default,
> > +      callee-saved-register: '$r14', callee-saved-restored: true, debug-info-variable: '',
> > +      debug-info-expression: '', debug-info-location: '' }
> > +  - { id: 4, type: spill-slot, offset: -24, size: 8, alignment: 8, stack-id: default,
> > +      callee-saved-register: '$r15', callee-saved-restored: true, debug-info-variable: '',
> > +      debug-info-expression: '', debug-info-location: '' }
> > +  - { id: 5, type: spill-slot, offset: -16, size: 8, alignment: 16, stack-id: default,
> > +      callee-saved-register: '$rbp', callee-saved-restored: true, debug-info-variable: '',
> > +      debug-info-expression: '', debug-info-location: '' }
> > +stack:
> > +  - { id: 0, name: '', type: spill-slot, offset: -64, size: 8, alignment: 8,
> > +      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
> > +      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
> > +constants:       []
> > +body:             |
> > +  bb.0.entry:
> > +    successors: %bb.2(0x30000000), %bb.1(0x50000000)
> > +    liveins: $rdi, $rbx, $r12, $r13, $r14, $r15, $rbp
> > +
> > +    DBG_VALUE $rdi, $noreg, !213, !DIExpression(DW_OP_plus_uconst, 1), debug-location !214
> > +    TEST64rr renamable $rdi, renamable $rdi, implicit-def $eflags, debug-location !215
> > +    JCC_1 %bb.2, 4, implicit $eflags, debug-location !217
> > +
> > +  bb.1.if.then:
> > +    successors: %bb.2(0x80000000)
> > +    liveins: $rdi, $rbp, $r15, $r14, $r13, $r12, $rbx
> > +
> > +    frame-setup PUSH64r killed $rbp, implicit-def $rsp, implicit $rsp
> > +    CFI_INSTRUCTION def_cfa_offset 16
> > +    frame-setup PUSH64r killed $r15, implicit-def $rsp, implicit $rsp
> > +    CFI_INSTRUCTION def_cfa_offset 24
> > +    frame-setup PUSH64r killed $r14, implicit-def $rsp, implicit $rsp
> > +    CFI_INSTRUCTION def_cfa_offset 32
> > +    frame-setup PUSH64r killed $r13, implicit-def $rsp, implicit $rsp
> > +    CFI_INSTRUCTION def_cfa_offset 40
> > +    frame-setup PUSH64r killed $r12, implicit-def $rsp, implicit $rsp
> > +    CFI_INSTRUCTION def_cfa_offset 48
> > +    frame-setup PUSH64r killed $rbx, implicit-def $rsp, implicit $rsp
> > +    CFI_INSTRUCTION def_cfa_offset 56
> > +    CFI_INSTRUCTION offset $rbx, -56
> > +    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 8 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 8 from %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
> > +    $r15 = frame-destroy POP64r implicit-def $rsp, implicit $rsp
> > +    CFI_INSTRUCTION def_cfa_offset 16
> > +    $rbp = frame-destroy POP64r implicit-def $rsp, implicit $rsp
> > +    CFI_INSTRUCTION def_cfa_offset 8
> > +
> > +  bb.2.if.end:
> > +    liveins: $rdi, $rbx, $r12, $r13, $r14, $r15, $rbp
> > +
> > +    renamable $eax = MOV32rm killed renamable $rdi, 1, $noreg, 4, $noreg, debug-location !223 :: (load 4 from %ir.add.ptr, !tbaa !24)
> > +    RETQ $eax, debug-location !228
> > +
> > +
> > +
> > +...
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list