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

Rong Xu via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 27 11:18:24 PST 2016


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 <chisophugis at 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
>
>
>


More information about the cfe-commits mailing list