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

Sean Silva via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 25 12:43:28 PST 2016


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





More information about the cfe-commits mailing list