Add -fdebug-info-for-profiling to emit more debug info for sample pgo profile collection

[danielcdh]

Summary

SamplePGO uses profile with debug info to collect profile. Unlike the traditional debugging purpose, sample pgo needs more accurate debug info to represent the profile. We add -femit-accurate-debug-info for this purpose. It can be combined with all debugging modes (-g, -gmlt, etc). It makes sure that the following pieces of info is always emitted:

• start line of all subprograms
• linkage name of all subprograms
• standalone subprograms (functions that has neither inlined nor been inlined)

The impact on speccpu2006 binary size (size increase comparing with -g0 binary, also includes data for -g binary, which does not change with this patch):

-gmlt(orig) -gmlt(patched) -g

433.milc 4.68% 5.40% 19.73%
444.namd 8.45% 8.93% 45.99%
447.dealII 97.43% 115.21% 374.89%
450.soplex 27.75% 31.88% 126.04%
453.povray 21.81% 26.16% 92.03%
470.lbm 0.60% 0.67% 1.96%
482.sphinx3 5.77% 6.47% 26.17%
400.perlbench 17.81% 19.43% 73.08%
401.bzip2 3.73% 3.92% 12.18%
403.gcc 31.75% 34.48% 122.75%
429.mcf 0.78% 0.88% 3.89%
445.gobmk 6.08% 7.92% 42.27%
456.hmmer 10.36% 11.25% 35.23%
458.sjeng 5.08% 5.42% 14.36%
462.libquantum 1.71% 1.96% 6.36%
464.h264ref 15.61% 16.56% 43.92%
471.omnetpp 11.93% 15.84% 60.09%
473.astar 3.11% 3.69% 14.18%
483.xalancbmk 56.29% 81.63% 353.22%
geomean 15.60% 18.30% 57.81%

Debug info size change for -gmlt binary with this patch:

433.milc 13.46%
444.namd 5.35%
447.dealII 18.21%
450.soplex 14.68%
453.povray 19.65%
470.lbm 6.03%
482.sphinx3 11.21%
400.perlbench 8.91%
401.bzip2 4.41%
403.gcc 8.56%
429.mcf 8.24%
445.gobmk 29.47%
456.hmmer 8.19%
458.sjeng 6.05%
462.libquantum 11.23%
464.h264ref 5.93%
471.omnetpp 31.89%
473.astar 16.20%
483.xalancbmk 44.62%
geomean 16.83%

[probinson]

lib/CodeGen/CGDebugInfo.cpp

2746

Need to update the comment to match how the code changed.

[danielcdh]

Add -femit-accurate-debug-info flag to guard the change.

[danielcdh]

Change the flag to -fprof-debug, which is more concise. The flag name is still open for discussion.

[gbedwell]

In D25435#608348, @danielcdh wrote:

Change the flag to -fprof-debug, which is more concise. The flag name is still open for discussion.

Well, since I have permission to bikeshed... :)

I'd prefer to have 'profile' rather than 'prof' in the name. This fits the established convention with -fprofile-sample-use etc.

[danielcdh]

change the flag name to -fprofile-debug

[danielcdh]

ping....

[echristo]

lib/CodeGen/CGDebugInfo.cpp

2743–2745

Should we be encapsulating all of these for profile debug info? I.e. I think coverage analysis is going to want the same things.

[danielcdh]

lib/CodeGen/CGDebugInfo.cpp

2743–2745

Do you mean that -fcoverage also implies -fprofile-debug?

I think the reason we introduce -fprofile-debug is that it has different requirments for debug info than coverage/sanitizer. E.g. we want to emit discriminator for -fprofile-debug, but not coverage/sanitizer.

[echristo]

lib/CodeGen/CGDebugInfo.cpp

2743–2745

So, what are the differences here? I imagine that profile debugging will want accurate source information. Perhaps we should hash this out in the thread that dblaikie has started on llvm-dev.

[danielcdh]

lib/CodeGen/CGDebugInfo.cpp

2743–2745

The discussion about introducing this flag is in the thread of http://lists.llvm.org/pipermail/llvm-dev/2016-November/107645.html

Basically -fprofile-debug not only requires accurate source information, it also requires:

1. emit linkage name in all subprograms
2. emit discriminator with dup-factor copy-factor encoded
3. use-unknown-locations=true

The above 3 items are not required by -fcoverage. That's why we introduce -fprofile-debug to record these extra debug info.

[hfinkel]

In D25435#609320, @danielcdh wrote:

change the flag name to -fprofile-debug

I don't really like this name because it sounds like it might be enabling some kind of debugging of the profiling. How about -fdebug-info-for-profiling. Another options is to make it a mode for -g such as -gprofiling.

[danielcdh]

In D25435#619169, @hfinkel wrote:

In D25435#609320, @danielcdh wrote:

change the flag name to -fprofile-debug

I don't really like this name because it sounds like it might be enabling some kind of debugging of the profiling. How about -fdebug-info-for-profiling. Another options is to make it a mode for -g such as -gprofiling.

Thanks for the comment. I'm OK with -fdebug-info-for-profiling. If no one objects, I'll update the patch tomorrow morning.

About -gprofiling, as discussed in http://lists.llvm.org/pipermail/llvm-dev/2016-November/107454.html, this should be a -f option instead of -g option.

[danielcdh]

update option name

[danielcdh]

ping...

Thanks,
Dehao

[danielcdh]

Thanks David for the reivew.

Could you also help take a look at https://reviews.llvm.org/D25434, as it depends on the TargetOptions.h change in that patch.

Thanks,
Dehao

[echristo]

I don't think this ever was hashed out in the llvm-dev thread?

[echristo]

Thanks for explaining all of this and going through it Dehao.

LGTM.

-eric

include/clang/Driver/Options.td

def fno_gnu_inline_asm : Flag<["-"], "fno-gnu-inline-asm">, Group<f_Group>,
   Flags<[DriverOption, CC1Option]>,
   HelpText<"Disable GNU style inline asm">;
 
 def fprofile_sample_use_EQ : Joined<["-"], "fprofile-sample-use=">,
     Group<f_Group>, Flags<[DriverOption, CC1Option]>,
     HelpText<"Enable sample-based profile guided optimizations">;
 def fauto_profile_EQ : Joined<["-"], "fauto-profile=">,
     Alias<fprofile_sample_use_EQ>;
+def fprofile_debug : Flag<["-"], "fprofile-debug">, Group<f_Group>,
+    Flags<[CC1Option]>,
+    HelpText<"Emit extra debug info to make sample profile more accurate.">;
+def fno_profile_debug : Flag<["-"], "fno-profile-debug">, Group<f_Group>,
+    Flags<[DriverOption]>,
+    HelpText<"Do not emit extra debug info for sample profiler.">;
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
     Group<f_Group>, Flags<[DriverOption]>,
     HelpText<"Generate instrumented code to collect execution counts into default.profraw file (overriden by '=' form of option or LLVM_PROFILE_FILE env var)">;
 def fprofile_instr_generate_EQ : Joined<["-"], "fprofile-instr-generate=">,
     Group<f_Group>, Flags<[DriverOption]>, MetaVarName<"<file>">,
     HelpText<"Generate instrumented code to collect execution counts into <file> (overridden by LLVM_PROFILE_FILE env var)">;
 def fprofile_instr_use : Flag<["-"], "fprofile-instr-use">, Group<f_Group>,
     Flags<[DriverOption]>;

include/clang/Frontend/CodeGenOptions.def

 VALUE_CODEGENOPT(EmitCheckPathComponentsToStrip, 32, 0)
 
 /// Whether to report the hotness of the code region for optimization remarks.
 CODEGENOPT(DiagnosticsWithHotness, 1, 0)
 
 /// Whether copy relocations support is available when building as PIE.
 CODEGENOPT(PIECopyRelocations, 1, 0)
 
+/// Whether emit extra debug info for sample pgo profile collection.
+CODEGENOPT(ProfileDebug, 1, 0)
+
 #undef CODEGENOPT
 #undef ENUM_CODEGENOPT
 #undef VALUE_CODEGENOPT
 

lib/CodeGen/BackendUtil.cpp

case CodeGenOptions::FPC_Fast:
     Options.AllowFPOpFusion = llvm::FPOpFusion::Fast;
     break;
   }
 
   Options.UseInitArray = CodeGenOpts.UseInitArray;
   Options.DisableIntegratedAS = CodeGenOpts.DisableIntegratedAS;
   Options.CompressDebugSections = CodeGenOpts.CompressDebugSections;
   Options.RelaxELFRelocations = CodeGenOpts.RelaxELFRelocations;
+  Options.ProfileDebug = CodeGenOpts.ProfileDebug;
 
   // Set EABI version.
   Options.EABIVersion = llvm::StringSwitch<llvm::EABI>(TargetOpts.EABIVersion)
                             .Case("4", llvm::EABI::EABI4)
                             .Case("5", llvm::EABI::EABI5)
                             .Case("gnu", llvm::EABI::GNU)
                             .Default(llvm::EABI::Default);
 

lib/CodeGen/CGDebugInfo.cpp

void CGDebugInfo::collectFunctionDeclProps(GlobalDecl GD, llvm::DIFile *Unit,
   Name = getFunctionName(FD);
   // Use mangled name as linkage name for C/C++ functions.
   if (FD->hasPrototype()) {
     LinkageName = CGM.getMangledName(GD);
     Flags |= llvm::DINode::FlagPrototyped;
   }
   // No need to replicate the linkage name if it isn't different from the
   // subprogram name, no need to have it at all unless coverage is enabled or
-  // debug is set to more than just line tables.
+  // debug is set to more than just line tables or extra debug info is needed.
   if (LinkageName == Name || (!CGM.getCodeGenOpts().EmitGcovArcs &&
                               !CGM.getCodeGenOpts().EmitGcovNotes &&
+                              !CGM.getCodeGenOpts().ProfileDebug &&
                               DebugKind <= codegenoptions::DebugLineTablesOnly))
     LinkageName = StringRef();
 
   if (DebugKind >= codegenoptions::LimitedDebugInfo) {
     if (const NamespaceDecl *NSDecl =
         dyn_cast_or_null<NamespaceDecl>(FD->getDeclContext()))
       FDContext = getOrCreateNameSpace(NSDecl);
     else if (const RecordDecl *RDecl =
              dyn_cast_or_null<RecordDecl>(FD->getDeclContext())) {

lib/Driver/Tools.cpp

   if (Arg *A = Args.getLastArg(options::OPT_fprofile_sample_use_EQ)) {
     StringRef fname = A->getValue();
     if (!llvm::sys::fs::exists(fname))
       D.Diag(diag::err_drv_no_such_file) << fname;
     else
       A->render(Args, CmdArgs);
   }
 
+  if (Args.hasFlag(options::OPT_fprofile_debug,
+                   options::OPT_fno_profile_debug, false))
+    CmdArgs.push_back("-fprofile-debug");
+
   // -fbuiltin is default unless -mkernel is used.
   bool UseBuiltins =
       Args.hasFlag(options::OPT_fbuiltin, options::OPT_fno_builtin,
                    !Args.hasArg(options::OPT_mkernel));
   if (!UseBuiltins)
     CmdArgs.push_back("-fno-builtin");
 
   // -ffreestanding implies -fno-builtin.

lib/Frontend/CompilerInvocation.cpp

static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
   Opts.UnrollLoops =
       Args.hasFlag(OPT_funroll_loops, OPT_fno_unroll_loops,
                    (Opts.OptimizationLevel > 1));
   Opts.RerollLoops = Args.hasArg(OPT_freroll_loops);
 
   Opts.DisableIntegratedAS = Args.hasArg(OPT_fno_integrated_as);
   Opts.Autolink = !Args.hasArg(OPT_fno_autolink);
   Opts.SampleProfileFile = Args.getLastArgValue(OPT_fprofile_sample_use_EQ);
+  Opts.ProfileDebug = Args.hasFlag(OPT_fprofile_debug,
+                                OPT_fno_profile_debug, false);
 
   setPGOInstrumentor(Opts, Args, Diags);
   Opts.InstrProfileOutput =
       Args.getLastArgValue(OPT_fprofile_instrument_path_EQ);
   Opts.ProfileInstrumentUsePath =
       Args.getLastArgValue(OPT_fprofile_instrument_use_path_EQ);
   if (!Opts.ProfileInstrumentUsePath.empty())
     setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath);

test/Driver/clang_f_opts.c

 // Make sure we don't match the -NOT lines with the linker invocation.
 // Delimiters match the start of the cc1 and the start of the linker lines
 // DELIMITERS: {{^ *"}}
 // CHECK-WCHAR1: -fno-short-wchar
 // CHECK-WCHAR1-NOT: -fshort-wchar
 // CHECK-WCHAR2: -fshort-wchar
 // CHECK-WCHAR2-NOT: -fno-short-wchar
 // DELIMITERS: {{^ *"}}
+
+// RUN: %clang -### -S -fno-profile-debug -fprofile-debug %s 2>&1 | FileCheck -check-prefix=CHECK-PROFILE-DEBUG %s
+// RUN: %clang -### -S -fprofile-debug -fno-profile-debug %s 2>&1 | FileCheck -check-prefix=CHECK-NO-PROFILE-DEBUG %s
+// CHECK-PROFILE-DEBUG: -fprofile-debug
+// CHECK-NO-PROFILE-DEBUG-NOT: -fprofile-debug