[PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

[Sean Silva via cfe-commits]

On Mon, Jan 25, 2016 at 12:52 PM, Rong Xu <xur at google.com> wrote:

> OK. Now I understand what you meant. I was trying to avoid driver changes.
> It seems you are saying I can change the driver to support cc1 options more
> cleanly. This can be done.
>
Great! Sorry for the confusion.

-- Sean Silva


> Thanks,
>
> Rong
> On Jan 25, 2016 12:43, "Sean Silva" <chisophugis at gmail.com> wrote:
>
>> silvas added inline comments.
>>
>> ================
>> Comment at: lib/Frontend/CompilerInvocation.cpp:487
>> @@ +486,3 @@
>> +  // Opts.ProfileInstrGenerate will be used for Clang instrumentation
>> only.
>> +  if (!Opts.ProfileIRInstr)
>> +    Opts.ProfileInstrGenerate = Args.hasArg(OPT_fprofile_instr_generate)
>> ||
>> ----------------
>> davidxl wrote:
>> > xur wrote:
>> > > silvas wrote:
>> > > > I don't like this. It breaks the 1:1 mapping of flags to codegen
>> options. Like Chad said, this sort of complexity should be kept in the
>> driver.
>> > > >
>> > > > Some refactoring may be needed to cleanly integrate the new IR
>> instrumentation.
>> > > hmm. I don't think there is 1:1 mapping b/w flags and codegen
>> options, because -fprofile-instr-generate is shared by IR and FE
>> instrumentation.
>> > If you rename the FE PGO CodeGen opt name as proposed, it will be
>> clearer and less confusing.
>> > hmm. I don't think there is 1:1 mapping b/w flags and codegen options,
>> because -fprofile-instr-generate is shared by IR and FE instrumentation.
>>
>> Maybe at the driver level (that discussion has yet to conclude), but at
>> cc1 level options should be factored to be clean and orthogonal. Like Chad
>> said, the complexity of verifying options that are mutually exclusive or
>> that must be present with other options should be done in the driver. cc1
>> is a private interface
>>
>>
>>
>> http://reviews.llvm.org/D15829
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160125/30ee7237/attachment.html>