[PATCH] D29203: Change debug-info-for-profiling from a TargetOption to a function attribute.

[Dehao Chen via Phabricator via llvm-commits]

danielcdh added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1138
+    HasDebugInfoForProfiling=true;
+
   // If there's no debug info for the function we're not going to do anything.
----------------
mehdi_amini wrote:
> danielcdh wrote:
> > mehdi_amini wrote:
> > > probinson wrote:
> > > > danielcdh wrote:
> > > > > mehdi_amini wrote:
> > > > > > danielcdh wrote:
> > > > > > > mehdi_amini wrote:
> > > > > > > > ahatanak wrote:
> > > > > > > > > Is debug-info-for-profiling used to turn on and off debug info for profiling on a per-function basis? I'm not very familiar with this function, but it looks like HasDebugInfoForProfiling is initialized to false in the constructor and then it will always be true after beginFunction is called on a function that has attribute debug-info-for-profiling=true.
> > > > > > > > Yep that seems broken to me. If foo has the attribute and not bar, we'll emit the profiling debug info for bar if foo is processed first.
> > > > > > > The issue is that I cannot check for function's attributes in DwarfUnit::applySubprogramAttributes, any suggestions on how I can pass down this attribute to that function?
> > > > > > Why not do here `HasDebugInfoForProfiling = MF->getFunction()->debugInfoForProfiling();` ?
> > > > > Because the llvm::DwarfDebug::beginFunction is called first before llvm::DwarfUnit::applySubprogramAttributes are called.
> > > > > 
> > > > > E.g. you have f1, f2, f3
> > > > > 
> > > > > the call sequence is:
> > > > > beginFunction(f1)
> > > > > beginFunction(f2)
> > > > > beginFunction(f3)
> > > > > applySubprogramAttributes(subprogramof_f1)
> > > > > applySubprogramAttributes(subprogramof_f2)
> > > > > applySubprogramAttributes(subprogramof_f3)
> > > > > 
> > > > > The current logic is, if there is any function that has HasDebugInfoForProfiling=true, then all function will have it.
> > > > All the more reason to put it at module or CU level, rather than function level.
> > > If the behavior you want is to have it enabled for the module, then yes like @probinson  says a module level construct would make more sense.
> > > 
> > > I don't enough about `-gmlt` to know what we would expect if we have a.cpp and b.cpp and we build with LTO one with -g and the other one with -gmlt ?
> > > 
> > Updated to use module flag for this.
> Can you clarify what is the behavior in this case:
> 
> > what we would expect if we have a.cpp and b.cpp and we build with LTO one with -g and the other one with -gmlt ?
I am not debug info expert, Eric may have more accurate answer.

My understanding is that one will have more debug info emitted, the other will only have line table emitted. What will happen in LTO?


https://reviews.llvm.org/D29203