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

[David Blaikie via llvm-commits]

On Fri, Jan 27, 2017 at 11:39 AM Dehao Chen via Phabricator <
reviews at reviews.llvm.org> wrote:

> danielcdh added a comment.
>
> In https://reviews.llvm.org/D29203#659019, @mehdi_amini wrote:
>
> > In https://reviews.llvm.org/D29203#658997, @danielcdh wrote:
> >
> > > - What is the motivating example that you want to compile some sources
> with -fdebug-info-for-profiling and some without
> > >   - If the debug info size is a concern for -gmlt binary, then one
> would want to only enable this flag for sources that is performance-critical
> >
> >
> > This is an argument *against* a module flag.
>
>
> Right ;-) I think I need to raise the point before close the patch.
>
> But my opinion is that this use cases (if any) would be rare because:
>
> - 17% seem small, but people may have other opinions
> - adding per-CU flag is manual and tedious
>

Not sure what you mean - manual in what scenario/way?

Oh, sorry, you mean that it would be tedious for users to actually specify
-f(no-)debug-info-for-profiling in a per-file way, rather than a
whole-project way, so the issue of how to resolve these is likely not a
real problem because no one's really going to bother doing this?


> - per-CU flag may need to change as new source coming in/old source change
> that updates the hot CU distribution
>

Not following here either... ah, now I see (& wrote the description above)
that if a user did this they would have to be careful about changing load
on their code - surprising parts might become hot and they might have to
change the flags.

I believe, at least at Google, some projects already live with this where
they explicitly opt out certain objects from their profile because it's too
expensive. (this is for non-sample based profiling, which is much more
expensive in file size, optimization, etc - so not an apples-to-apples
comparison, just to say that there are costs that can cause people to be
motivated to deal with that much hassle, etc)


> That being said, I would think it's better to force this flag in LTO when
> mixed flag situation happens. But will let upstream decide if this would be
> a feature or a bug.
>
> Thanks,
> Dehao
>
>
> https://reviews.llvm.org/D29203
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170127/a3e02109/attachment.html>