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


http://reviews.llvm.org/D17622



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to