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

[Xinliang David Li via cfe-commits]

On Fri, Jul 1, 2016 at 4:32 PM, Sean Silva <chisophugis at gmail.com> wrote:

> silvas added inline comments.
>
> ================
> Comment at: lib/Driver/Tools.cpp:3560
> @@ +3559,3 @@
> +    if (PGOTrainArg->getOption().matches(options::OPT_fpgo_train_EQ)) {
> +      if (StringRef(PGOTrainArg->getValue()) == "source-cfg")
> +        CmdArgs.push_back("-fprofile-instrument=clang");
> ----------------
> davidxl wrote:
> > I think it is better to make the selector  'source' vs 'cfg'.
> >
> > -fpgo-train=source
> > -fpgo-train=cfg
> So would `-fpgo-train=cfg` enable icall instrumentation or not?
>
> My thinking is that down the road we will have one flag for each
> independent instrumentation capability (and of course some are mutually
> incompatible). This mirrors what the sanitizers do. For example, we would
> have:
> `-fpgo-train=optimizer-cfg` --> IR edge profiling
> `-fpgo-train=optimizer-icall` --> IR icall value profiling
> `-fpgo-train=optimizer-...` --> other independent instrumentation we can
> do in IR instrumentation.
> `-fpgo-train=source-cfg` --> FE edge profiling
> `-fpgo-train=source-icall` --> FE icall profiling (if that even exists; I
> see some code but there is no user-visible flag)
> `-fpgo-train=source-...` --> other FE instrumentation.
>
> We can then have `-fpgo-train=optimizer` enable e.g.
> `-fpgo-train=optimizer-cfg,optimizer-icall`.
> We can also have `-fpgo-train=source` enable e.g. `-fpgo-train=source-cfg`.
>
> Since instrumentation can have different overheads or different runtime
> requirements, users may want to disable some instrumentation.
>
>

-fpgo-train is the new umbrella option that turn on at least edge profiling
and some other flavor of value profiling (icall, or stringop, etc in the
the future). There is a default setting for source or cfg based PGO. The
fine grain control of each sub-option should be done via a different flag
-- otherwise we will have to introduce 2 x N sub-options if used with
-fpgo-train.  In other words, -fpgo-train=cfg turns on edge and icall
profiling as of today.

To summarize, I think the following behavior will be nice to users:

1) -fpgo-train when used without any option, it defaults to IR pgo (since
it is new option)
2) -fpgo-train=cfg (or some other name) turns on IR pgo
3) -fpgo-train=source turns on FE pgo

4) -fpgo-control=edge:icall to turn on edge profiling and icall profiling.
Edge will be turn on by default always, so it is redundant to specify. To
turn icall off:
    -fpgo-control=^icall

5) -fpgo-train-file=<path> to specify path of raw profile

I am not sure if we want -fpgo-apply= as an alias to -fprofile-instr-use=,
but not object to it.

6) I also like the capability to combine 1) and 5) by using tags to
disambiguate path and cfg|source, but that can be done later.

thanks,

David




>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D21823
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160702/3527bc1a/attachment.html>