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

Vedant Kumar via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 19 17:06:12 PDT 2016


> On Jul 19, 2016, at 5:01 PM, Xinliang David Li <davidxl at google.com> wrote:
> 
> 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

To be clear, this is a behavior change, but IMO the new behavior would be
better.

Instead of creating programs that clobber over existing profiles by default
they would merge into a set of profiles.


> 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?

+ 1

thanks,
vedant

> 
> 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
> 
> 
> 
> 



More information about the cfe-commits mailing list