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

Sean Silva via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 27 16:49:34 PST 2016


silvas added inline comments.

================
Comment at: lib/Frontend/CompilerInvocation.cpp:483
@@ +482,3 @@
+    Opts.ProfileIRInstr = true;
+  else
+    // Opts.ClangProfInstrGen will be used for Clang instrumentation only.
----------------
davidxl wrote:
> silvas wrote:
> > This still isn't factored right. I think at this point the meaning of the driver-level options is not really useful at CC1 level (too convoluted) for it to be useful to pass them through.
> > 
> > The right thing to do here is probably more like:
> > - refactor so that we have 4 individual CC1 options for InstrProfileOutput, InstrProfileInput, ProfileIRInstr, ClangProfInstrGen (although probably rename ClangProfInstrGen and ProfileIRInstr to be consistent with each other, e.g. "ProfileIRInstr" and "ProfileClangInstr").
> > - add logic in Driver to convert from the driver-level options to the CC1 options.
> It is already pretty close to your proposal -- the only missing thing is cc1 option for ClangProfInstrGen. However given that IR and Clang InstrGen are mutually exclusive, is an additional CC1 option needed?
There are 3 possibilities:
- clang instr
- ir instr
- no instr

A simple binary flag can only encode 2. So some sort of extra information needs to be passed. Off the top of my head, I thought of just adding an extra flag, but I like your suggestion `-fprofile-instrumentor=[clang|LLVM]`. That is clean and avoids having to deal with an error for the "clang instr + ir instr" case.


http://reviews.llvm.org/D15829





More information about the cfe-commits mailing list