[PATCH] D25434: Generate more debug info in -gmlt

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1182-1184
+  // If -fprofile-debug is enabled, need to emit the subprogram and its source
+  // location.
+  if (Asm->TM.Options.DebugInfoForProfiling || !Minimal)
----------------
danielcdh wrote:
> dblaikie wrote:
> > dblaikie wrote:
> > > Another out of date comment
> > Can this "DebugInfoForProfiling" check be floated up to the caller? (rename the 'Minimal' flag parameter to something more descriptive for the behavior change that applies to both debug-info-for-profiling and otherwise?)
> Changed Minimal to SkipSPAttributes because there is no need to output additional attributes when DebugInfoForProfiling (thus cannot float it to caller). Does it sound OK?
Good name - but I meant remove the testing of DebugInfoForProfiling here, but check it at the call sites?

Or perhaps that doesn't work out - multiple call sites, etc & you want to apply it in more cases. It's a while since I wrote this.

If that's the case (that it doesn't make sense to put this on one or two call sites) - could perhaps collapse it at the start of the function with something like:

  SkipSPAttributes |= Asm->TM.Options.DebugInfoForProfiling;




================
Comment at: lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1195-1197
   // Skip the rest of the attributes under -gmlt to save space.
-  if (Minimal)
+  if (SkipSPAttributes)
     return;
----------------
DebugInfoForProfiling shouldn't be checked here?


https://reviews.llvm.org/D25434