[PATCH] D17622: [PGO] clang cc1 option change to enable IR level instrumenation

Sean Silva via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 25 21:52:51 PST 2016

silvas added a comment.

Thanks for splitting this out.

I think the `-fprofile-instrument=llvm` is straightforward and uncontroversial. Let's focus this review on that and split the others into a separate review.

But a quick preview of my thoughs about `-fprofile-instrument={llvm,clang}-use`:
I think that fully decoupling the cc1 options from the driver options like you are doing here is a good cleanup, but we need a more refined approach. For example, the current patch has "use" modes as mutually exclusive with the "gen" modes, which doesn't make sense. In fact, we have already seen a situation where we may want to have "use" and "gen" in the same clang invocation: for profile guided MST instr. And I don't see a reason to artificially prohibit FE "use" to annotate MD_prof metadata and then use that metadata for profile guided MST instr (however, it would be a strange workflow for users...).
Perhaps something like `-fprofile-instrument-use={llvm,clang}` which sets a CodeGenOpt `ProfileUse` which is either {None, Clang, LLVM}. We can use a common PGOInstrumentor enum for both, since there is a 1:1 mapping between the instrumentor and its corresponding "use" logic.
In the long run I imagine we will want two mirror options `-fpgo-gen={none,llvm,clang}` and `-fpgo-use={none,llvm,clang}`, but the details of renaming to the final state can wait until we clearly see the right design (this is cc1 so it is easy to refactor as needed later).


More information about the cfe-commits mailing list