Emit discriminator for inlined callsites.

[danielcdh]

Summary

Inlined callsites need to be emitted in debug info so that sample profile can be annotated to the correct inlined instance.

[dblaikie]

test/DebugInfo/Generic/discriminator.ll

11

perhaps write these with attribute((always_inline)) so you can build the example without running the full optimization pipeline & still get the desired code.

Do you need two different inline functions or two different destination functions? (maybe it makes the example simpler/more obvious, which is fair - but it could just be two of the same)

14

I assume this function doesn't need a parameter

[dblaikie]

btw, there's no need to file a bug as you're providing a fix - feel free to
skip the overhead and just provide the patch & skip the bug

The only real reason to file a bug in the open source tracker is if you
want someone else to look at/work on it/know you are working on it before
you've sent out a patch, etc.

[danielcdh]

Simplify the testcase.

[danielcdh]

It gives a warning:
a.cc:2:34: warning: GCC does not allow 'always_inline' attribute in this position on a function definition [-Wgcc-compat]

[danielcdh]

FYI, the debug info size impact for this change: clang-3.8 binary (built by clang itself in release mode with -g) increases from 1117.1MB to 1118.7MB (0.14% increase)

[danielcdh]

ping...

[dnovillo]

The one thing I don't understand about this patch is why it needs to add a new DW_*discriminator flag? But I don't know enough DWARF to decide whether using the existing flag is OK or not.

Also, this is a final codegen issue? That is, the actual AddDiscriminators pass is DTRT?

[dblaikie]

Couple of minor fixes/improvements, but looks right enough - commit when ready.

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

442

Drop {} around a single-line block (the idiomatic/prevailing style in the LLVM project, though I don't think we have it written in the style guide)

test/DebugInfo/Generic/discriminator.ll

11

Switch the attribute earlier and you can do this without the separate declaration:

static void __attribute__((always_inline)) bar() { xyz(); }

16

CHECK could be a bit more detailed - I'm guessing the first inlined subroutine doesn't need a discriminator, and the second does? Something like:

CHECK: DW_TAG_inlined_subroutine
CHECK-NOT: DW_AT_discriminator
CHECK: DW_TAG_inlined_subroutine
CHECK-NOT: {{DW_TAG|NULL}}
CHECK: DW_AT_discriminator 1

include/llvm/Support/Dwarf.h

enum Attribute : uint16_t {
 
   // Extensions for Fission proposal.
   DW_AT_GNU_dwo_name = 0x2130,
   DW_AT_GNU_dwo_id = 0x2131,
   DW_AT_GNU_ranges_base = 0x2132,
   DW_AT_GNU_addr_base = 0x2133,
   DW_AT_GNU_pubnames = 0x2134,
   DW_AT_GNU_pubtypes = 0x2135,
+  DW_AT_GNU_discriminator = 0x2136,
 
   // LLVM project extensions.
   DW_AT_LLVM_include_path = 0x3e00,
   DW_AT_LLVM_config_macros = 0x3e01,
   DW_AT_LLVM_isysroot = 0x3e02,
 
   // Apple extensions.
   DW_AT_APPLE_optimized = 0x3fe1,

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

DIE *DwarfCompileUnit::constructInlinedScopeDIE(LexicalScope *Scope) {
 
   attachRangesOrLowHighPC(*ScopeDIE, Scope->getRanges());
 
   // Add the call site information to the DIE.
   const DILocation *IA = Scope->getInlinedAt();
   addUInt(*ScopeDIE, dwarf::DW_AT_call_file, None,
           getOrCreateSourceID(IA->getFilename(), IA->getDirectory()));
   addUInt(*ScopeDIE, dwarf::DW_AT_call_line, None, IA->getLine());
+  if (IA->getDiscriminator()) {
+    addUInt(*ScopeDIE, dwarf::DW_AT_GNU_discriminator, None, IA->getDiscriminator());
+  }
 
   // Add name to the name table, we do this here because we're guaranteed
   // to have concrete versions of our DW_TAG_inlined_subprogram nodes.
   DD->addSubprogramNames(InlinedSP, *ScopeDIE);
 
   return ScopeDIE;
 }
 

lib/Support/Dwarf.cpp

case DW_AT_LLVM_isysroot:              return "DW_AT_LLVM_isysroot";
 
     // DWARF5 Fission Extension Attribute
   case DW_AT_GNU_dwo_name:               return "DW_AT_GNU_dwo_name";
   case DW_AT_GNU_dwo_id:                 return "DW_AT_GNU_dwo_id";
   case DW_AT_GNU_ranges_base:            return "DW_AT_GNU_ranges_base";
   case DW_AT_GNU_addr_base:              return "DW_AT_GNU_addr_base";
   case DW_AT_GNU_pubnames:               return "DW_AT_GNU_pubnames";
   case DW_AT_GNU_pubtypes:               return "DW_AT_GNU_pubtypes";
+  case DW_AT_GNU_discriminator:          return "DW_AT_GNU_discriminator";
   }
   return nullptr;
 }
 
 const char *llvm::dwarf::FormEncodingString(unsigned Encoding) {
   switch (Encoding) {
   case DW_FORM_addr:                     return "DW_FORM_addr";
   case DW_FORM_block2:                   return "DW_FORM_block2";

test/DebugInfo/Generic/discriminator.ll

+; REQUIRES: object-emission
+
+; RUN: %llc_dwarf < %s -filetype=obj | llvm-dwarfdump -debug-dump=info - | FileCheck %s
+
+; Given the following source, ensure that the discriminator is emitted for
+; the inlined callsite.
+
+;void abc();
+;void xyz();
+;
+;static void bar() { abc(); }
+;static void baz() { xyz(); }
+;
+;void foo(int i) {
+;  bar(); baz();
+;}
+
+; CHECK: DW_AT_GNU_discriminator
+
+; Function Attrs: uwtable
+define void @_Z3fooi(i32 %i) #0 !dbg !4 { 
+  tail call void @llvm.dbg.value(metadata i32 %i, i64 0, metadata !9, metadata !17), !dbg !18
+  tail call void @_Z3abcv(), !dbg !19
+  tail call void @_Z3xyzv(), !dbg !21
+  ret void, !dbg !24
+}
+
+declare void @_Z3abcv() #1
+
+declare void @_Z3xyzv() #1
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.value(metadata, i64, metadata, metadata) #2
+
+attributes #0 = { uwtable "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #2 = { nounwind readnone }
+
+!llvm.dbg.cu = !{!0} 
+!llvm.module.flags = !{!14, !15}
+!llvm.ident = !{!16}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 3.8.0 (trunk 252497)", isOptimized: true, runtimeVersion: 0, emissionKind: 1, enums: !2, subprograms: !3)
+!1 = !DIFile(filename: "a.cc", directory: "/tmp")
+!2 = !{}
+!3 = !{!4, !10, !13}
+!4 = distinct !DISubprogram(name: "foo", linkageName: "_Z3fooi", scope: !1, file: !1, line: 7, type: !5, isLocal: false, isDefinition: true, scopeLine: 7, flags: DIFlagPrototyped, isOptimized: true, variables: !8)
+!5 = !DISubroutineType(types: !6)
+!6 = !{null, !7}
+!7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!8 = !{!9}
+!9 = !DILocalVariable(name: "i", arg: 1, scope: !4, file: !1, line: 7, type: !7)
+!10 = distinct !DISubprogram(name: "bar", linkageName: "_ZL3barv", scope: !1, file: !1, line: 4, type: !11, isLocal: true, isDefinition: true, scopeLine: 4, flags: DIFlagPrototyped, isOptimized: true, variables: !2)
+!11 = !DISubroutineType(types: !12)
+!12 = !{null}
+!13 = distinct !DISubprogram(name: "baz", linkageName: "_ZL3bazv", scope: !1, file: !1, line: 5, type: !11, isLocal: true, isDefinition: true, scopeLine: 5, flags: DIFlagPrototyped, isOptimized: true, variables: !2)
+!14 = !{i32 2, !"Dwarf Version", i32 4}
+!15 = !{i32 2, !"Debug Info Version", i32 3}
+!16 = !{!"clang version 3.8.0 (trunk 252497)"}
+!17 = !DIExpression()
+!18 = !DILocation(line: 7, column: 14, scope: !4)
+!19 = !DILocation(line: 4, column: 21, scope: !10, inlinedAt: !20)
+!20 = distinct !DILocation(line: 8, column: 3, scope: !4)
+!21 = !DILocation(line: 5, column: 21, scope: !13, inlinedAt: !22)
+!22 = distinct !DILocation(line: 8, column: 10, scope: !23)
+!23 = !DILexicalBlockFile(scope: !4, file: !1, discriminator: 1)
+!24 = !DILocation(line: 9, column: 1, scope: !4)