[PATCH] D37791: [XRay][CodeGen] Use the current function symbol as the associated symbol for the instrumentation map

Wed Sep 13 23:13:16 PDT 2017

On Wed, Sep 13, 2017 at 10:52 PM Dean Michael Berris via Phabricator <
reviews at reviews.llvm.org> wrote:

> dberris updated this revision to Diff 115167.
> dberris edited the summary of this revision.
> dberris added reviewers: dblaikie, echristo.
> dberris added a comment.
> Herald added a subscriber: JDevlieghere.
>
> Reworded description, added a test.
>
>
> https://reviews.llvm.org/D37791
>
> Files:
>   lib/CodeGen/AsmPrinter/AsmPrinter.cpp
>   test/DebugInfo/X86/xray-split-dwarf-interaction.ll
>
>
> Index: test/DebugInfo/X86/xray-split-dwarf-interaction.ll
> ===================================================================
> --- /dev/null
> +++ test/DebugInfo/X86/xray-split-dwarf-interaction.ll
> @@ -0,0 +1,76 @@
> +; RUN: %llc_dwarf -split-dwarf-file=input.dwo -O3 -function-sections
> -data-sections \
> +; RUN:     -relocation-model=pic -filetype=obj -generate-type-units
> -debug-compile -o %t %s
>

Have you tried this without fission/split-dwarf? I imagine it still
reproduces (I think it's only type units that are important?)

> +; RUN: llvm-readelf -sections %t | FileCheck %s
> +; Created from:
> +; input.cc:
> +;
> +; class a {
> +;   int b();
> +; };
> +; int a::b() {
> +;   for (;;)
> +;     ;
> +; }
>

Does this function need the for loop inside, or the int return type (rather
than void)? I can try to minimize this a bit further if you like.

It might be good to understand exactly under what conditions the switch to
debug_types happens (I can see how it could happen in endFunction for
inlined functions - not entirely sure where it happens for this example,
though - perhaps you could break on switchSection so you can see/tell me,
or I can do it?)

If a member function is what triggers this (what about a non-member
function that takes a struct/class as a parameter?) what about a static
function (wilnl lead to simpler IR, no dbg.values, etc)

> +;
> +; ModuleID = 'input.cc'
> +source_filename = "input.cc"
> +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> +target triple = "x86_64-pc-linux"
>

I usually strip out these lines ^ I think...

> +
> +%class.a = type { i8 }
> +
> +; Function Attrs: nounwind readnone uwtable
> +define i32 @_ZN1a1bEv(%class.a* nocapture readnone) local_unnamed_addr #0
> align 2 !dbg !8 {
> +  tail call void @llvm.dbg.value(metadata %class.a* %0, metadata !17,
> metadata !DIExpression()), !dbg !19

+  br label %2, !dbg !20
> +
> +; <label>:2:                                      ; preds = %2, %1
> +  br label %2, !dbg !21, !llvm.loop !25
> +}
> +
> +; In this test we want to make sure that the xray_instr_map section for
> `a::b()` is actually
> +; associated with the per-function section defined for the function
> instead of associating with
> +; the .debug_types.dwo section.
> +;
> +; CHECK-DAG: [[FSECT:[0-9]+]]] .text._ZN1a1bEv PROGBITS
> +; CHECK-DAG: [{{.*}}] .debug_types.dwo PROGBITS
> +; CHECK-DAG: [{{.*}}] xray_instr_map PROGBITS {{.*}} {{.*}} {{.*}} {{.*}}
> WAL [[FSECT]]
>

Generally I'd put the CHECKs above all the IR, so they're quicker/easier to
spot.

> +
> +; Function Attrs: nounwind readnone speculatable
> +declare void @llvm.dbg.value(metadata, metadata, metadata) #1
> +
> +attributes #0 = { nounwind readnone uwtable
> "correctly-rounded-divide-sqrt-fp-math"="false"
> "disable-tail-calls"="false" "less-precise-fpmad"="false"
> "no-frame-pointer-elim"="false" "no-frame-pointer-elim-non-leaf"
> "no-infs-fp-math"="false" "no-jump-tables"="false"
> "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false"
> "no-trapping-math"="false" "stack-protector-buffer-size"="8"
> "target-cpu"="x86-64"
> "target-features"="+fxsr,+mmx,+sse,+sse2,+sse3,+sse4.1,+ssse3,+x87"
> "unsafe-fp-math"="false" "use-soft-float"="false"
> "xray-instruction-threshold"="200" }
> +attributes #1 = { nounwind readnone speculatable }
>

Usually the attributes are stripped out to simplify the test.

> +
> +!llvm.dbg.cu = !{!0}
> +!llvm.module.flags = !{!3, !4, !5, !6}
> +!llvm.ident = !{!7}
> +
> +!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1,
> producer: "clang version trunk (trunk r312634)", isOptimized: true,
> runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining:
> false, debugInfoForProfiling: true)
> +!1 = !DIFile(filename: "input.cc", directory:
> "/usr/local/google/home/dberris/tmp")
> +!2 = !{}
> +!3 = !{i32 2, !"Debug Info Version", i32 3}
> +!4 = !{i32 1, !"wchar_size", i32 4}
> +!5 = !{i32 7, !"PIC Level", i32 2}
> +!6 = !{i32 7, !"PIE Level", i32 2}
> +!7 = !{!"clang version trunk (trunk r312634)"}
> +!8 = distinct !DISubprogram(name: "b", linkageName: "_ZN1a1bEv", scope:
> !9, file: !1, line: 4, type: !12, isLocal: false, isDefinition: true,
> scopeLine: 4, flags: DIFlagPrototyped, isOptimized: true, unit: !0,
> declaration: !11, variables: !16)
> +!9 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "a", file:
> !1, line: 1, size: 8, elements: !10, identifier: "_ZTS1a")
> +!10 = !{!11}
> +!11 = !DISubprogram(name: "b", linkageName: "_ZN1a1bEv", scope: !9, file:
> !1, line: 2, type: !12, isLocal: false, isDefinition: false, scopeLine: 2,
> flags: DIFlagPrototyped, isOptimized: true)
> +!12 = !DISubroutineType(types: !13)
> +!13 = !{!14, !15}
> +!14 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
> +!15 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64,
> flags: DIFlagArtificial | DIFlagObjectPointer)
> +!16 = !{!17}
> +!17 = !DILocalVariable(name: "this", arg: 1, scope: !8, type: !18, flags:
> DIFlagArtificial | DIFlagObjectPointer)
> +!18 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64)
> +!19 = !DILocation(line: 0, scope: !8)
> +!20 = !DILocation(line: 5, column: 3, scope: !8)
> +!21 = !DILocation(line: 5, column: 3, scope: !22)
> +!22 = !DILexicalBlockFile(scope: !23, file: !1, discriminator: 2)
> +!23 = distinct !DILexicalBlock(scope: !24, file: !1, line: 5, column: 3)
> +!24 = distinct !DILexicalBlock(scope: !8, file: !1, line: 5, column: 3)
> +!25 = distinct !{!25, !26, !27}
> +!26 = !DILocation(line: 5, column: 3, scope: !24)
> +!27 = !DILocation(line: 6, column: 5, scope: !24)
> Index: lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> ===================================================================
> --- lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> +++ lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> @@ -2785,7 +2785,7 @@
>    MCSection *InstMap = nullptr;
>    MCSection *FnSledIndex = nullptr;
>    if (MF->getSubtarget().getTargetTriple().isOSBinFormatELF()) {
> -    auto Associated =
> dyn_cast<MCSymbolELF>(PrevSection->getBeginSymbol());
> +    auto Associated = dyn_cast<MCSymbolELF>(CurrentFnSym);
>      assert(Associated != nullptr);
>      auto Flags = ELF::SHF_WRITE | ELF::SHF_ALLOC | ELF::SHF_LINK_ORDER;
>      std::string GroupName;
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170914/c736d589/attachment-0001.html>