Index: lib/CodeGen/LiveDebugValues.cpp =================================================================== --- lib/CodeGen/LiveDebugValues.cpp +++ lib/CodeGen/LiveDebugValues.cpp @@ -7,14 +7,23 @@ //===----------------------------------------------------------------------===// /// /// This pass implements a data flow analysis that propagates debug location -/// information by inserting additional DBG_VALUE instructions into the machine -/// instruction stream. The pass internally builds debug location liveness -/// ranges to determine the points where additional DBG_VALUEs need to be -/// inserted. +/// information by inserting additional DBG_VALUE insts into the machine +/// instruction stream. Before running, each DBG_VALUE inst corresponds to a +/// source assignment of a variable. Afterwards, a DBG_VALUE inst specifies a +/// variable location for the current basic block (see SourceLevelDebugging.rst). /// /// This is a separate pass from DbgValueHistoryCalculator to facilitate /// testing and improve modularity. /// +/// Each variable location is represented by a VarLoc object that identifies the +/// source variable, its current machine-location, and the DBG_VALUE inst that +/// specifies the location. Each VarLoc is indexed in the (function-scope) +/// VarLocMap, giving each VarLoc a unique index. Rather than operate directly +/// on machine locations, the dataflow analysis in this pass identifies +/// locations by their index in the VarLocMap, meaning all the variable +/// locations in a block can be described by a sparse vector of VarLocMap +/// indexes. +/// //===----------------------------------------------------------------------===// #include "llvm/ADT/DenseMap.h" @@ -320,6 +329,14 @@ Vars.insert({Var, VarLocID}); } + /// Insert a set of ranges. + void insertFromLocSet(const VarLocSet &ToLoad, const VarLocMap &Map) { + for (unsigned Id : ToLoad) { + const VarLoc &Var = Map[Id]; + insert(Id, Var.Var); + } + } + /// Empty the set. void clear() { VarLocs.clear(); @@ -361,8 +378,8 @@ void transferRegisterDef(MachineInstr &MI, OpenRangesSet &OpenRanges, VarLocMap &VarLocIDs, TransferMap &Transfers, DebugParamMap &DebugEntryVals); - bool transferTerminatorInst(MachineInstr &MI, OpenRangesSet &OpenRanges, - VarLocInMBB &OutLocs, const VarLocMap &VarLocIDs); + bool transferTerminator(MachineBasicBlock *MBB, OpenRangesSet &OpenRanges, + VarLocInMBB &OutLocs, const VarLocMap &VarLocIDs); bool process(MachineInstr &MI, OpenRangesSet &OpenRanges, VarLocInMBB &OutLocs, VarLocMap &VarLocIDs, @@ -376,7 +393,12 @@ bool join(MachineBasicBlock &MBB, VarLocInMBB &OutLocs, VarLocInMBB &InLocs, const VarLocMap &VarLocIDs, SmallPtrSet &Visited, - SmallPtrSetImpl &ArtificialBlocks); + SmallPtrSetImpl &ArtificialBlocks, + VarLocInMBB &PendingInLocs); + + /// Create DBG_VALUE insts for inlocs that have been propagated but + /// had their instruction creation deferred. + void flushPendingLocs(VarLocInMBB &PendingInLocs, VarLocMap &VarLocIDs); bool ExtendRanges(MachineFunction &MF); @@ -904,14 +926,11 @@ } /// Terminate all open ranges at the end of the current basic block. -bool LiveDebugValues::transferTerminatorInst(MachineInstr &MI, - OpenRangesSet &OpenRanges, - VarLocInMBB &OutLocs, - const VarLocMap &VarLocIDs) { +bool LiveDebugValues::transferTerminator(MachineBasicBlock *CurMBB, + OpenRangesSet &OpenRanges, + VarLocInMBB &OutLocs, + const VarLocMap &VarLocIDs) { bool Changed = false; - const MachineBasicBlock *CurMBB = MI.getParent(); - if (!(MI.isTerminator() || (&MI == &CurMBB->back()))) - return false; if (OpenRanges.empty()) return false; @@ -1011,7 +1030,6 @@ if (MI.isDebugValue()) accumulateFragmentMap(MI, SeenFragments, OverlapFragments); } - Changed = transferTerminatorInst(MI, OpenRanges, OutLocs, VarLocIDs); return Changed; } @@ -1022,7 +1040,8 @@ MachineBasicBlock &MBB, VarLocInMBB &OutLocs, VarLocInMBB &InLocs, const VarLocMap &VarLocIDs, SmallPtrSet &Visited, - SmallPtrSetImpl &ArtificialBlocks) { + SmallPtrSetImpl &ArtificialBlocks, + VarLocInMBB &PendingInLocs) { LLVM_DEBUG(dbgs() << "join MBB: " << MBB.getNumber() << "\n"); bool Changed = false; @@ -1088,44 +1107,15 @@ return false; VarLocSet &ILS = InLocs[&MBB]; + VarLocSet &Pending = PendingInLocs[&MBB]; - // Insert DBG_VALUE instructions, if not already inserted. + // New locations will have DBG_VALUE insts inserted at the start of the + // block, after location propagation has finished. Record the insertions + // that we need to perform in the Pending set. VarLocSet Diff = InLocsT; Diff.intersectWithComplement(ILS); for (auto ID : Diff) { - // This VarLoc is not found in InLocs i.e. it is not yet inserted. So, a - // new range is started for the var from the mbb's beginning by inserting - // a new DBG_VALUE. process() will end this range however appropriate. - const VarLoc &DiffIt = VarLocIDs[ID]; - const MachineInstr *DebugInstr = &DiffIt.MI; - MachineInstr *MI = nullptr; - if (DiffIt.isConstant()) { - MachineOperand MO(DebugInstr->getOperand(0)); - MI = BuildMI(MBB, MBB.instr_begin(), DebugInstr->getDebugLoc(), - DebugInstr->getDesc(), false, MO, - 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(), IsIndirect, Reg, - DebugInstr->getDebugVariable(), DebugExpr); - } - LLVM_DEBUG(dbgs() << "Inserted: "; MI->dump();); + Pending.set(ID); ILS.set(ID); ++NumInserted; Changed = true; @@ -1133,6 +1123,53 @@ return Changed; } +void LiveDebugValues::flushPendingLocs(VarLocInMBB &PendingInLocs, + VarLocMap &VarLocIDs) { + // PendingInLocs records all locations propagated into blocks, which have + // not had DBG_VALUE insts created. Go through and create those insts now. + for (auto &Iter : PendingInLocs) { + // Map is keyed on a constant pointer, unwrap it so we can insert insts. + auto &MBB = const_cast(*Iter.first); + VarLocSet &Pending = Iter.second; + + for (unsigned ID : Pending) { + // The ID location is live-in to MBB -- work out what kind of machine + // location it is and create a DBG_VALUE. + const VarLoc &DiffIt = VarLocIDs[ID]; + const MachineInstr *DebugInstr = &DiffIt.MI; + MachineInstr *MI = nullptr; + + if (DiffIt.isConstant()) { + MachineOperand MO(DebugInstr->getOperand(0)); + MI = BuildMI(MBB, MBB.instr_begin(), DebugInstr->getDebugLoc(), + DebugInstr->getDesc(), false, MO, + 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(), IsIndirect, Reg, + DebugInstr->getDebugVariable(), DebugExpr); + } + LLVM_DEBUG(dbgs() << "Inserted: "; MI->dump();); + } + } +} + /// Calculate the liveness information for the given machine function and /// extend ranges across basic blocks. bool LiveDebugValues::ExtendRanges(MachineFunction &MF) { @@ -1149,6 +1186,9 @@ VarLocInMBB OutLocs; // Ranges that exist beyond bb. VarLocInMBB InLocs; // Ranges that are incoming after joining. TransferMap Transfers; // DBG_VALUEs associated with spills. + VarLocInMBB PendingInLocs; // Ranges that are incoming after joining, but + // that we have deferred creating DBG_VALUE insts + // for immediately. VarToFragments SeenFragments; @@ -1214,6 +1254,7 @@ process(MI, OpenRanges, OutLocs, VarLocIDs, Transfers, DebugEntryVals, dontTransferChanges, OverlapFragments, SeenFragments); } + transferTerminator(&MBB, OpenRanges, OutLocs, VarLocIDs); // Add any entry DBG_VALUE instructions necessitated by parameter // clobbering. for (auto &TR : Transfers) { @@ -1221,6 +1262,9 @@ TR.DebugInst); } Transfers.clear(); + + // Initialize pending inlocs. + PendingInLocs[&MBB] = VarLocSet(); } auto hasNonArtificialLocation = [](const MachineInstr &MI) -> bool { @@ -1257,8 +1301,8 @@ while (!Worklist.empty()) { MachineBasicBlock *MBB = OrderToBB[Worklist.top()]; Worklist.pop(); - MBBJoined = - join(*MBB, OutLocs, InLocs, VarLocIDs, Visited, ArtificialBlocks); + MBBJoined = join(*MBB, OutLocs, InLocs, VarLocIDs, Visited, + ArtificialBlocks, PendingInLocs); Visited.insert(MBB); if (MBBJoined) { MBBJoined = false; @@ -1266,11 +1310,14 @@ // Now that we have started to extend ranges across BBs we need to // examine spill instructions to see whether they spill registers that // correspond to user variables. + // First load any pending inlocs. + OpenRanges.insertFromLocSet(PendingInLocs[MBB], VarLocIDs); for (auto &MI : *MBB) OLChanged |= process(MI, OpenRanges, OutLocs, VarLocIDs, Transfers, DebugEntryVals, transferChanges, OverlapFragments, SeenFragments); + Changed |= transferTerminator(MBB, OpenRanges, OutLocs, VarLocIDs); // Add any DBG_VALUE instructions necessitated by spills. for (auto &TR : Transfers) @@ -1298,6 +1345,10 @@ assert(Pending.empty() && "Pending should be empty"); } + // Deferred inlocs will not have had any DBG_VALUE insts created; do + // that now. + flushPendingLocs(PendingInLocs, VarLocIDs); + LLVM_DEBUG(printVarLocInMBB(MF, OutLocs, VarLocIDs, "Final OutLocs", dbgs())); LLVM_DEBUG(printVarLocInMBB(MF, InLocs, VarLocIDs, "Final InLocs", dbgs())); return Changed; Index: test/DebugInfo/MIR/X86/live-debug-values-restore.mir =================================================================== --- test/DebugInfo/MIR/X86/live-debug-values-restore.mir +++ test/DebugInfo/MIR/X86/live-debug-values-restore.mir @@ -3,7 +3,10 @@ # 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. +# expressions: +# 'g': test for a crash from PR42773 +# 'h': complex expressions should be restored +# 'i': spills should be restored across block boundaries # #define FORCE_SPILL() \ # __asm volatile("" : : : \ @@ -16,10 +19,11 @@ # return *(p + 1); # } -# Pick out DILocalVariable numbers for "p", "q" and "r" +# Pick out DILocalVariable numbers for "p", "q" and "r" etc # CHECK: ![[PVAR:[0-9]+]] = !DILocalVariable(name: "p", # CHECK: ![[QVAR:[0-9]+]] = !DILocalVariable(name: "q", # CHECK: ![[RVAR:[0-9]+]] = !DILocalVariable(name: "r", +# CHECK: ![[SVAR:[0-9]+]] = !DILocalVariable(name: "s", # Ascertain that the spill has been recognized and manifested in a DBG_VALUE. # CHECK: MOV64mr $rsp,{{.*-8.*}}killed{{.*}}$rdi :: (store 8 into %stack.0) @@ -78,6 +82,24 @@ ret i32 %0, !dbg !228 } + define dso_local i32 @i(i32* readonly %p) local_unnamed_addr !dbg !307 { + entry: + br label %foo + + foo: + call void @llvm.dbg.value(metadata i32* %p, metadata !313, metadata !DIExpression()), !dbg !314 + %tobool = icmp eq i32* %p, null, !dbg !315 + br i1 %tobool, label %if.end, label %if.then, !dbg !317 + + 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 !318, !srcloc !320 + br label %if.end, !dbg !321 + + if.end: ; preds = %entry, %if.then + %add.ptr = getelementptr inbounds i32, i32* %p, i64 1, !dbg !322 + %0 = load i32, i32* %add.ptr, align 4, !dbg !323, !tbaa !24 + ret i32 %0, !dbg !328 + } declare void @llvm.dbg.value(metadata, metadata, metadata) @@ -144,6 +166,21 @@ !222 = !DILocation(line: 109, column: 14, scope: !207) !223 = !DILocation(line: 109, column: 10, scope: !207) !228 = !DILocation(line: 109, column: 3, scope: !207) + !301 = !DIBasicType(name: "looong int", size: 64, encoding: DW_ATE_signed) + !307 = distinct !DISubprogram(name: "g", scope: !0, file: !1, line: 105, type: !8, scopeLine: 105, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !312) + !312 = !{!313} + !313 = !DILocalVariable(name: "s", arg: 1, scope: !307, file: !1, line: 105, type: !301) + !314 = !DILocation(line: 105, column: 12, scope: !307) + !315 = !DILocation(line: 106, column: 7, scope: !316) + !316 = distinct !DILexicalBlock(scope: !307, file: !1, line: 106, column: 7) + !317 = !DILocation(line: 106, column: 7, scope: !307) + !318 = !DILocation(line: 107, column: 5, scope: !319) + !319 = distinct !DILexicalBlock(scope: !316, file: !1, line: 106, column: 10) + !320 = !{i32 -2147471544} + !321 = !DILocation(line: 108, column: 3, scope: !319) + !322 = !DILocation(line: 109, column: 14, scope: !307) + !323 = !DILocation(line: 109, column: 10, scope: !307) + !328 = !DILocation(line: 109, column: 3, scope: !307) ... --- name: f @@ -438,4 +475,123 @@ +... +--- +# Function four: test that spill restores are detected across block +# boundaries. The spill has been moved to bb.1, children of which are +# 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. + +# CHECK-LABEL: name: i +# CHECK-LABEL: bb.0.entry: +# 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-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-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 $rdi, $noreg, ![[SVAR]], !DIExpression(DW_OP_plus_uconst, 1) +name: i +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.1 + liveins: $rdi, $rbx, $r12, $r13, $r14, $r15, $rbp + + DBG_VALUE $rdi, $noreg, !313, !DIExpression(DW_OP_plus_uconst, 1), debug-location !314 + JMP_1 %bb.1, debug-location !317 + + bb.1.foo: + successors: %bb.3(0x30000000), %bb.2(0x50000000) + liveins: $rdi, $rbx, $r12, $r13, $r14, $r15, $rbp + + $rax = COPY $rdi + MOV64mr $rsp, 1, $noreg, -8, $noreg, killed renamable $rdi :: (store 8 into %stack.0) + $rdi = MOV64ri 0 + TEST64rr $rax, renamable $rax, implicit-def $eflags, debug-location !315 + JCC_1 %bb.3, 4, implicit $eflags, debug-location !317 + + bb.2.if.then: + successors: %bb.3(0x80000000) + liveins: $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 + 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, !320, debug-location !318 + $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.3.if.end: + liveins: $rbx, $r12, $r13, $r14, $r15, $rbp + + renamable $rdi = MOV64rm $rsp, 1, $noreg, -8, $noreg :: (load 8 from %stack.0) + renamable $eax = MOV32rm killed renamable $rdi, 1, $noreg, 4, $noreg, debug-location !323 :: (load 4 from %ir.add.ptr, !tbaa !24) + RETQ $eax, debug-location !328 + ...