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

[Eric Christopher via llvm-commits]

It doesn't seem like a major burden to make this respect the
make-it-work-like-non-LTO (per-function/CU) so we should do that if we can
I think. It makes it much easier from a lot of various perspectives.

-eric

On Fri, Jan 27, 2017 at 11:49 AM David Blaikie <dblaikie at gmail.com> wrote:

> 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/fa6c0eca/attachment.html>