[PATCH] D21823: [Driver] Add flags for enabling both types of PGO Instrumentation

Xinliang David Li via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 19 17:01:56 PDT 2016


The new behavior works really well. There is one follow up change I would
like to discuss.

The EQ form of the option -fprofile-generate= takes a directory name as the
argument instead of the filename. Currently the compiler will create a
default 'default.profraw' name for it, but this means, online merging won't
be available unless environment variable is also used, which is not
convenient.  I propose we change the default behavior to enable profile
online merging (i.e., use default_%m.profraw as the default name) for the
following reasons:

1) There is almost no downside enabling profiling merging by default -- new
capability is enabled with no lost functionality
2) WIth profile merging enabled for instrumented programs with instrumented
shared libs, each lib will dump its own profile data in the same dir, but
users of -fprofile-generate= option usually do not care about what/how many
raw profile names are in the directory (note that GCC's behavior is one
profile per object module), they just pack up the while directory.
llvm-profdata also support merging (for indexed profile) with
input-list-file option.
3) GCC's has online merging support, so having online merging by default
with this option matches up better the claim that it is a GCC compatible
option.

What do you think?

thanks,

David



On Sat, Jul 9, 2016 at 4:39 PM, Sean Silva <chisophugis at gmail.com> wrote:

> silvas added a comment.
>
> In http://reviews.llvm.org/D21823#479418, @davidxl wrote:
>
> > I should have brought it up earlier, but I forgot.    I think a better
> (and simpler) proposal is to make -fprofile-generate and -fprofile-use turn
> on IR based PGO.
> >
> > -fprofile-generate and -fprofile-use options were introduced by Diego
> (cc'ed) recently for GCC option compatibility. At that time, IR based PGO
> was not yet ready, so they were made aliases to FE instrumentation options
> -fprofile-instr-generate and -fprofile-instr-use.    Now I think it is time
> to make it right.   The documentation only says that these two options are
> gcc compatible options to control profile generation and use, but does not
> specify the underlying implementation. It is also likely that Google is the
> only user of this option. If a user using this option really want FE
> instrumentation, there is always -fprofile-instr-generate to use.  It also
> more likely that IR instrumentation is what user wants when using GCC
> compatible options (as they behave more similarly).
>
>
> This SGTM.
>
> It may cause some user confusion since there is no obvious distinction
> between "profile generate" and "profile instr generate" from a user
> perspective. But we can avoid that with improved documentation.
>
> My main concern is that the existing documentation already says that
> -fprofile-generate behaves identically to -fprofile-instr-generate
> http://clang.llvm.org/docs/UsersManual.html#cmdoption-fprofile-generate
> However, I think it is reasonable to change this behavior (and of course
> the documentation), as users of this option are likely using it for
> compatibility with GCC and so they likely don't care about the specifics of
> clang FEPGO.
> We probably want to at least leave a small note in the documentation
> regarding this change of behavior.
>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D21823
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160719/99006441/attachment-0001.html>


More information about the cfe-commits mailing list