Sean: Adding a new CC1 option ProfileClangInstr will make things cleaner. But this won't solve the problem. The root of all the mess is there is no driver level option for IR instrumentation. I need to either "hijack" the -Xclang option, or move the logic to CompilerInvocation.cpp, which both you don't like.
The reason is I have to reply on the Driver option -fprofile-instr-generate to have the right link line for the profile library. -fprofile-instr-generate will set the Instrumentation to Clang (regardless use of current cc1 option of -fprofile-instr-generate, or the new proposed -fprofile-clang-instr). For IR instrumentation where the user specifies "-Xclang -fprofile-ir-instr", I need to overwrite the driver level option. To get that, I either parse the -Xclang value (this is "hijack), or I handle it in CC1 (in CompilerInvocation.cpp). I don't see a way to avoid it. Can we use a hidden driver option here for IR instrumentation? On Tue, Jan 26, 2016 at 5:01 PM, Sean Silva <chisophu...@gmail.com> wrote: > silvas added inline comments. > > ================ > Comment at: lib/Driver/Tools.cpp:5520 > @@ +5519,3 @@ > + A->claim(); > + if ((StringRef(A->getValue(0)) == "-fprofile-ir-instr") && > + (Args.hasArg(options::OPT_fprofile_instr_generate) || > ---------------- > This is definitely not the right thing to do. Don't hijack -Xclang (which is > a completely generic thing). > > ================ > Comment at: lib/Frontend/CompilerInvocation.cpp:483 > @@ +482,3 @@ > + Opts.ProfileIRInstr = true; > + else > + // Opts.ClangProfInstrGen will be used for Clang instrumentation only. > ---------------- > 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. > > ================ > Comment at: test/CodeGen/pgo-instrumentation.c:5 > @@ +4,3 @@ > +// RUN: %clang -O2 -c -Xclang -fprofile-ir-instr -fprofile-instr-generate %s > -mllvm -debug-pass=Structure 2>&1 | FileCheck %s > -check-prefix=CHECK-PGOGENPASS-INVOKED-V1 > +// CHECK-PGOGENPASS-INVOKED-V1: PGOInstrumentationGenPass > +// > ---------------- > It isn't clear what V1/V2 mean. > > > http://reviews.llvm.org/D15829 > > > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
