<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On 13 Sep 2017, at 23:13, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Wed, Sep 13, 2017 at 10:52 PM Dean Michael Berris via Phabricator <<a href="mailto:reviews@reviews.llvm.org" class="">reviews@reviews.llvm.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">dberris updated this revision to Diff 115167.<br class="">
dberris edited the summary of this revision.<br class="">
dberris added reviewers: dblaikie, echristo.<br class="">
dberris added a comment.<br class="">
Herald added a subscriber: JDevlieghere.<br class="">
<br class="">
Reworded description, added a test.<br class="">
<br class="">
<br class="">
<a href="https://reviews.llvm.org/D37791" rel="noreferrer" target="_blank" class="">https://reviews.llvm.org/D37791</a><br class="">
<br class="">
Files:<br class="">
  lib/CodeGen/AsmPrinter/AsmPrinter.cpp<br class="">
  test/DebugInfo/X86/xray-split-dwarf-interaction.ll<br class="">
<br class="">
<br class="">
Index: test/DebugInfo/X86/xray-split-dwarf-interaction.ll<br class="">
===================================================================<br class="">
--- /dev/null<br class="">
+++ test/DebugInfo/X86/xray-split-dwarf-interaction.ll<br class="">
@@ -0,0 +1,76 @@<br class="">
+; RUN: %llc_dwarf -split-dwarf-file=input.dwo -O3 -function-sections -data-sections \<br class="">
+; RUN:     -relocation-model=pic -filetype=obj -generate-type-units -debug-compile -o %t %s<br class=""></blockquote><div class=""><br class="">Have you tried this without fission/split-dwarf? I imagine it still reproduces (I think it's only type units that are important?)<br class=""> </div></div></div></div></blockquote><div><br class=""></div><div>Not yet, but this was the case that caused the issues that I can reproduce. I can add another build here too.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+; RUN: llvm-readelf -sections %t | FileCheck %s<br class="">
+; Created from:<br class="">
+; <a href="http://input.cc" class="">input.cc</a>:<br class="">
+;<br class="">
+; class a {<br class="">
+;   int b();<br class="">
+; };<br class="">
+; int a::b() {<br class="">
+;   for (;;)<br class="">
+;     ;<br class="">
+; }<br class=""></blockquote><div class=""><br class="">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.<br class=""><br class=""></div></div></div></div></blockquote><div><br class=""></div><div>We kind-of do, because that's one way to force the XRay instrumentation to be added by the front-end. We could just put the XRay attribute directly, but that's not quite as obvious. The loop I think keeps the function from being elided as a consequence.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class="">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?)<br class=""><br class=""></div></div></div></div></blockquote><div><br class=""></div><div>My suspicion is it only happens for classes that only have one member.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class="">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)<br class=""></div></div></div></div></blockquote><div><br class=""></div><div>We know it also works for class static member functions.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+;<br class="">
+; ModuleID = '<a href="http://input.cc" class="">input.cc</a>'<br class="">
+source_filename = "<a href="http://input.cc" class="">input.cc</a>"<br class="">
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"<br class="">
+target triple = "x86_64-pc-linux"<br class=""></blockquote><div class=""><br class="">I usually strip out these lines ^ I think... <br class=""> </div></div></div></div></blockquote><div><br class=""></div><div>ACK</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br class="">
+%class.a = type { i8 }<br class="">
+<br class="">
+; Function Attrs: nounwind readnone uwtable<br class="">
+define i32 @_ZN1a1bEv(%class.a* nocapture readnone) local_unnamed_addr #0 align 2 !dbg !8 {<br class="">
+  tail call void @llvm.dbg.value(metadata %class.a* %0, metadata !17, metadata !DIExpression()), !dbg !19 </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+  br label %2, !dbg !20<br class="">
+<br class="">
+; <label>:2:                                      ; preds = %2, %1<br class="">
+  br label %2, !dbg !21, !llvm.loop !25<br class="">
+}<br class="">
+<br class="">
+; In this test we want to make sure that the xray_instr_map section for `a::b()` is actually<br class="">
+; associated with the per-function section defined for the function instead of associating with<br class="">
+; the .debug_types.dwo section.<br class="">
+;<br class="">
+; CHECK-DAG: [[FSECT:[0-9]+]]] .text._ZN1a1bEv PROGBITS<br class="">
+; CHECK-DAG: [{{.*}}] .debug_types.dwo PROGBITS<br class="">
+; CHECK-DAG: [{{.*}}] xray_instr_map PROGBITS {{.*}} {{.*}} {{.*}} {{.*}} WAL [[FSECT]]<br class=""></blockquote><div class=""><br class="">Generally I'd put the CHECKs above all the IR, so they're quicker/easier to spot.<br class=""> </div></div></div></div></blockquote><div><br class=""></div>Good idea.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br class="">
+; Function Attrs: nounwind readnone speculatable<br class="">
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1<br class="">
+<br class="">
+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" }<br class="">
+attributes #1 = { nounwind readnone speculatable }<br class=""></blockquote><div class=""><br class="">Usually the attributes are stripped out to simplify the test.<br class=""> </div></div></div></div></blockquote><div><br class=""></div><div>We do need at least the xray-instruction-threshold though. Do you want me to just keep 'nounwind readnone uwtable "xray-instruction-threshold"="200"' ?</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br class="">
+!<a href="http://llvm.dbg.cu/" rel="noreferrer" target="_blank" class="">llvm.dbg.cu</a> = !{!0}<br class="">
+!llvm.module.flags = !{!3, !4, !5, !6}<br class="">
+!llvm.ident = !{!7}<br class="">
+<br class="">
+!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)<br class="">
+!1 = !DIFile(filename: "<a href="http://input.cc" class="">input.cc</a>", directory: "/usr/local/google/home/dberris/tmp")<br class="">
+!2 = !{}<br class="">
+!3 = !{i32 2, !"Debug Info Version", i32 3}<br class="">
+!4 = !{i32 1, !"wchar_size", i32 4}<br class="">
+!5 = !{i32 7, !"PIC Level", i32 2}<br class="">
+!6 = !{i32 7, !"PIE Level", i32 2}<br class="">
+!7 = !{!"clang version trunk (trunk r312634)"}<br class="">
+!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)<br class="">
+!9 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "a", file: !1, line: 1, size: 8, elements: !10, identifier: "_ZTS1a")<br class="">
+!10 = !{!11}<br class="">
+!11 = !DISubprogram(name: "b", linkageName: "_ZN1a1bEv", scope: !9, file: !1, line: 2, type: !12, isLocal: false, isDefinition: false, scopeLine: 2, flags: DIFlagPrototyped, isOptimized: true)<br class="">
+!12 = !DISubroutineType(types: !13)<br class="">
+!13 = !{!14, !15}<br class="">
+!14 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)<br class="">
+!15 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64, flags: DIFlagArtificial | DIFlagObjectPointer)<br class="">
+!16 = !{!17}<br class="">
+!17 = !DILocalVariable(name: "this", arg: 1, scope: !8, type: !18, flags: DIFlagArtificial | DIFlagObjectPointer)<br class="">
+!18 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64)<br class="">
+!19 = !DILocation(line: 0, scope: !8)<br class="">
+!20 = !DILocation(line: 5, column: 3, scope: !8)<br class="">
+!21 = !DILocation(line: 5, column: 3, scope: !22)<br class="">
+!22 = !DILexicalBlockFile(scope: !23, file: !1, discriminator: 2)<br class="">
+!23 = distinct !DILexicalBlock(scope: !24, file: !1, line: 5, column: 3)<br class="">
+!24 = distinct !DILexicalBlock(scope: !8, file: !1, line: 5, column: 3)<br class="">
+!25 = distinct !{!25, !26, !27}<br class="">
+!26 = !DILocation(line: 5, column: 3, scope: !24)<br class="">
+!27 = !DILocation(line: 6, column: 5, scope: !24)<br class="">
Index: lib/CodeGen/AsmPrinter/AsmPrinter.cpp<br class="">
===================================================================<br class="">
--- lib/CodeGen/AsmPrinter/AsmPrinter.cpp<br class="">
+++ lib/CodeGen/AsmPrinter/AsmPrinter.cpp<br class="">
@@ -2785,7 +2785,7 @@<br class="">
   MCSection *InstMap = nullptr;<br class="">
   MCSection *FnSledIndex = nullptr;<br class="">
   if (MF->getSubtarget().getTargetTriple().isOSBinFormatELF()) {<br class="">
-    auto Associated = dyn_cast<MCSymbolELF>(PrevSection->getBeginSymbol());<br class="">
+    auto Associated = dyn_cast<MCSymbolELF>(CurrentFnSym);<br class="">
     assert(Associated != nullptr);<br class="">
     auto Flags = ELF::SHF_WRITE | ELF::SHF_ALLOC | ELF::SHF_LINK_ORDER;<br class="">
     std::string GroupName;<br class="">
<br class="">
<br class="">
</blockquote></div></div>
</div></blockquote></div><br class=""></body></html>