From 2a813ef208283980598202ac6453f3a777c608f5 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Thu, 23 Aug 2018 22:35:58 +0000 Subject: [PATCH] DebugInfo: Improve debug location merging Fix a set of related bugs: * Considering two locations as equivalent when their lines are the same but their scopes are different causes erroneous debug info that attributes a commoned call to be attributed to one of the two calls it was commoned from. * The previous code to compute a new location's scope was inaccurate and would use the inlinedAt that was the /parent/ of the inlinedAt that is the nearest common one, and also used that parent scope instead of the nearest common scope. * Not generating new locations generally seemed like a lower quality choice There was some risk that generating more new locations could hurt object size by making more fine grained line table entries, but it looks like that was offset by the decrease in line table (& address & ranges) size caused by more accurately computing the scope - which likely lead to fewer range entries (more contiguous ranges) & reduced size that way. All up with these changes I saw minor reductions (-1.21%, -1.77%) in .rela.debug_ranges and .rela.debug_addr (in a fission, compressed debug info build) as well as other minor size changes (generally reductinos) across the board (-1.32% debug_info.dwo, -1.28% debug_loc.dwo). Measured in an optimized (-O2) build of the clang binary. If you are investigating a size regression in an optimized debug builds, this is certainly a patch to look into - and I'd be happy to look into any major regressions found & see what we can do to address them. llvm-svn: 340583 --- llvm/include/llvm/IR/DebugInfoMetadata.h | 20 +--- llvm/lib/IR/DebugInfo.cpp | 3 +- llvm/lib/IR/DebugInfoMetadata.cpp | 40 +++++--- llvm/test/DebugInfo/COFF/local-variables.ll | 2 +- llvm/test/DebugInfo/X86/merge_inlined_loc.ll | 100 +++++++++++++++++++ 5 files changed, 130 insertions(+), 35 deletions(-) create mode 100644 llvm/test/DebugInfo/X86/merge_inlined_loc.ll diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h index cfe9198da10e..bb61ca027a21 100644 --- a/llvm/include/llvm/IR/DebugInfoMetadata.h +++ b/llvm/include/llvm/IR/DebugInfoMetadata.h @@ -1479,19 +1479,6 @@ public: return getScope(); } - /// Check whether this can be discriminated from another location. - /// - /// Check \c this can be discriminated from \c RHS in a linetable entry. - /// Scope and inlined-at chains are not recorded in the linetable, so they - /// cannot be used to distinguish basic blocks. - bool canDiscriminate(const DILocation &RHS) const { - return getLine() != RHS.getLine() || - getColumn() != RHS.getColumn() || - getDiscriminator() != RHS.getDiscriminator() || - getFilename() != RHS.getFilename() || - getDirectory() != RHS.getDirectory(); - } - /// Get the DWARF discriminator. /// /// DWARF discriminators distinguish identical file locations between @@ -1539,8 +1526,6 @@ public: /// discriminator. inline const DILocation *cloneWithDuplicationFactor(unsigned DF) const; - enum { NoGeneratedLocation = false, WithGeneratedLocation = true }; - /// When two instructions are combined into a single instruction we also /// need to combine the original locations into a single location. /// @@ -1555,9 +1540,8 @@ public: /// /// \p GenerateLocation: Whether the merged location can be generated when /// \p LocA and \p LocB differ. - static const DILocation * - getMergedLocation(const DILocation *LocA, const DILocation *LocB, - bool GenerateLocation = NoGeneratedLocation); + static const DILocation *getMergedLocation(const DILocation *LocA, + const DILocation *LocB); /// Returns the base discriminator for a given encoded discriminator \p D. static unsigned getBaseDiscriminatorFromDiscriminator(unsigned D) { diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp index 037c2ea31a59..f1c0a01c5d09 100644 --- a/llvm/lib/IR/DebugInfo.cpp +++ b/llvm/lib/IR/DebugInfo.cpp @@ -690,8 +690,7 @@ unsigned llvm::getDebugMetadataVersionFromModule(const Module &M) { void Instruction::applyMergedLocation(const DILocation *LocA, const DILocation *LocB) { - setDebugLoc(DILocation::getMergedLocation(LocA, LocB, - DILocation::WithGeneratedLocation)); + setDebugLoc(DILocation::getMergedLocation(LocA, LocB)); } //===----------------------------------------------------------------------===// diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp index 33bf8edf08a8..0ceded076796 100644 --- a/llvm/lib/IR/DebugInfoMetadata.cpp +++ b/llvm/lib/IR/DebugInfoMetadata.cpp @@ -14,7 +14,7 @@ #include "llvm/IR/DebugInfoMetadata.h" #include "LLVMContextImpl.h" #include "MetadataImpl.h" -#include "llvm/ADT/SmallPtrSet.h" +#include "llvm/ADT/SmallSet.h" #include "llvm/ADT/StringSwitch.h" #include "llvm/IR/DIBuilder.h" #include "llvm/IR/Function.h" @@ -69,28 +69,40 @@ DILocation *DILocation::getImpl(LLVMContext &Context, unsigned Line, } const DILocation *DILocation::getMergedLocation(const DILocation *LocA, - const DILocation *LocB, - bool GenerateLocation) { + const DILocation *LocB) { if (!LocA || !LocB) return nullptr; - if (LocA == LocB || !LocA->canDiscriminate(*LocB)) + if (LocA == LocB) return LocA; - if (!GenerateLocation) - return nullptr; - SmallPtrSet InlinedLocationsA; for (DILocation *L = LocA->getInlinedAt(); L; L = L->getInlinedAt()) InlinedLocationsA.insert(L); - const DILocation *Result = LocB; - for (DILocation *L = LocB->getInlinedAt(); L; L = L->getInlinedAt()) { - Result = L; - if (InlinedLocationsA.count(L)) - break; + SmallSet, 5> Locations; + DIScope *S = LocA->getScope(); + DILocation *L = LocA->getInlinedAt(); + while (S) { + Locations.insert(std::make_pair(S, L)); + S = S->getScope().resolve(); + if (!S && L) { + S = L->getScope(); + L = L->getInlinedAt(); + } } - return DILocation::get(Result->getContext(), 0, 0, Result->getScope(), - Result->getInlinedAt()); + const DILocation *Result = LocB; + S = LocB->getScope(); + L = LocB->getInlinedAt(); + while (S) { + if (Locations.count(std::make_pair(S, L))) + break; + S = S->getScope().resolve(); + if (!S && L) { + S = L->getScope(); + L = L->getInlinedAt(); + } + } + return DILocation::get(Result->getContext(), 0, 0, S, L); } DINode::DIFlags DINode::getFlag(StringRef Flag) { diff --git a/llvm/test/DebugInfo/COFF/local-variables.ll b/llvm/test/DebugInfo/COFF/local-variables.ll index be78478b9afe..b549b1a89211 100644 --- a/llvm/test/DebugInfo/COFF/local-variables.ll +++ b/llvm/test/DebugInfo/COFF/local-variables.ll @@ -60,7 +60,7 @@ ; ASM: leaq 36(%rsp), %rcx ; ASM: [[else_end:\.Ltmp.*]]: ; ASM: .LBB0_3: # %if.end -; ASM: .cv_loc 0 1 17 1 # t.cpp:17:1 +; ASM: .cv_loc 0 1 0 0 # t.cpp:0:0 ; ASM: callq capture ; ASM: nop ; ASM: addq $56, %rsp diff --git a/llvm/test/DebugInfo/X86/merge_inlined_loc.ll b/llvm/test/DebugInfo/X86/merge_inlined_loc.ll new file mode 100644 index 000000000000..576a3d3a199f --- /dev/null +++ b/llvm/test/DebugInfo/X86/merge_inlined_loc.ll @@ -0,0 +1,100 @@ +; RUN: llc %s -mtriple=x86_64-unknown-unknown -o - | FileCheck %s + +; Generated with "clang -g -c -emit-llvm -S -O3" + +; This will test several features of merging debug locations. Importantly, +; locations with the same source line but different scopes should be merged to +; a line zero location at the nearest common scope and inlining. The location +; of the single call to "common" (the two calls are collapsed together by +; BranchFolding) should be attributed to line zero inside the wrapper2 inlined +; scope within f1. + +; void common(); +; inline void wrapper() { common(); } +; extern bool b; +; void sink(); +; inline void wrapper2() { +; if (b) { +; sink(); +; wrapper(); +; } else +; wrapper(); +; } +; void f1() { wrapper2(); } + +; Ensure there is only one inlined_subroutine (for wrapper2, none for wrapper) +; & that its address range includes the call to 'common'. + +; CHECK: jmp _Z6commonv +; CHECK-NEXT: [[LABEL:.*]]: + +; CHECK: .section .debug_info +; CHECK: DW_TAG_subprogram +; CHECK: DW_TAG_subprogram +; CHECK-NOT: {{DW_TAG\|End Of Children}} +; CHECK: DW_TAG_inlined_subroutine +; CHECK-NOT: {{DW_TAG\|End Of Children}} +; CHECK: [[LABEL]]-{{.*}} DW_AT_high_pc +; CHECK-NOT: DW_TAG + + + +@b = external dso_local local_unnamed_addr global i8, align 1 + +; Function Attrs: uwtable +define dso_local void @_Z2f1v() local_unnamed_addr !dbg !7 { +entry: + %0 = load i8, i8* @b, align 1, !dbg !10, !tbaa !14, !range !18 + %tobool.i = icmp eq i8 %0, 0, !dbg !10 + br i1 %tobool.i, label %if.else.i, label %if.then.i, !dbg !19 + +if.then.i: ; preds = %entry + tail call void @_Z4sinkv(), !dbg !20 + tail call void @_Z6commonv(), !dbg !22 + br label %_Z8wrapper2v.exit, !dbg !25 + +if.else.i: ; preds = %entry + tail call void @_Z6commonv(), !dbg !26 + br label %_Z8wrapper2v.exit + +_Z8wrapper2v.exit: ; preds = %if.then.i, %if.else.i + ret void, !dbg !28 +} + +declare dso_local void @_Z4sinkv() local_unnamed_addr + +declare dso_local void @_Z6commonv() local_unnamed_addr + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!3, !4, !5} +!llvm.ident = !{!6} + +!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 8.0.0 (trunk 340559) (llvm/trunk 340572)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None) +!1 = !DIFile(filename: "merge_loc.cpp", directory: "/usr/local/google/home/blaikie/dev/scratch") +!2 = !{} +!3 = !{i32 2, !"Dwarf Version", i32 4} +!4 = !{i32 2, !"Debug Info Version", i32 3} +!5 = !{i32 1, !"wchar_size", i32 4} +!6 = !{!"clang version 8.0.0 (trunk 340559) (llvm/trunk 340572)"} +!7 = distinct !DISubprogram(name: "f1", linkageName: "_Z2f1v", scope: !1, file: !1, line: 12, type: !8, isLocal: false, isDefinition: true, scopeLine: 12, flags: DIFlagPrototyped, isOptimized: true, unit: !0, retainedNodes: !2) +!8 = !DISubroutineType(types: !9) +!9 = !{null} +!10 = !DILocation(line: 6, column: 7, scope: !11, inlinedAt: !13) +!11 = distinct !DILexicalBlock(scope: !12, file: !1, line: 6, column: 7) +!12 = distinct !DISubprogram(name: "wrapper2", linkageName: "_Z8wrapper2v", scope: !1, file: !1, line: 5, type: !8, isLocal: false, isDefinition: true, scopeLine: 5, flags: DIFlagPrototyped, isOptimized: true, unit: !0, retainedNodes: !2) +!13 = distinct !DILocation(line: 13, column: 3, scope: !7) +!14 = !{!15, !15, i64 0} +!15 = !{!"bool", !16, i64 0} +!16 = !{!"omnipotent char", !17, i64 0} +!17 = !{!"Simple C++ TBAA"} +!18 = !{i8 0, i8 2} +!19 = !DILocation(line: 6, column: 7, scope: !12, inlinedAt: !13) +!20 = !DILocation(line: 7, column: 5, scope: !21, inlinedAt: !13) +!21 = distinct !DILexicalBlock(scope: !11, file: !1, line: 6, column: 10) +!22 = !DILocation(line: 2, column: 25, scope: !23, inlinedAt: !24) +!23 = distinct !DISubprogram(name: "wrapper", linkageName: "_Z7wrapperv", scope: !1, file: !1, line: 2, type: !8, isLocal: false, isDefinition: true, scopeLine: 2, flags: DIFlagPrototyped, isOptimized: true, unit: !0, retainedNodes: !2) +!24 = distinct !DILocation(line: 8, column: 5, scope: !21, inlinedAt: !13) +!25 = !DILocation(line: 9, column: 3, scope: !21, inlinedAt: !13) +!26 = !DILocation(line: 2, column: 25, scope: !23, inlinedAt: !27) +!27 = distinct !DILocation(line: 10, column: 5, scope: !11, inlinedAt: !13) +!28 = !DILocation(line: 14, column: 1, scope: !7)