[PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
Chad Rosier via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 22 11:12:37 PST 2016
mcrosier added a subscriber: mcrosier.
mcrosier added a comment.
Would it make sense to include an additional test (in test/Driver) that shows the -fprofile-ir-instr option being passed from the driver to the frontend? Such a test case would land in clang_f_opt.c, which has many examples.
================
Comment at: lib/Driver/Tools.cpp:3279
@@ -3278,1 +3278,3 @@
+
+ Args.AddAllArgs(CmdArgs, options::OPT_fprofile_ir_instr);
}
----------------
I don't think AddAllArgs is what you really want. What if the user specifies the option twice? Do we really want to pass the flag from the driver to the front-end twice? Also, should we warn if the option is passed twice?
I also think some of the logic in CompilerInvocation should land here... see comments below.
================
Comment at: lib/Frontend/CompilerInvocation.cpp:482
@@ +481,3 @@
+ Opts.ProfileIRInstr = Args.hasArg(OPT_fprofile_ir_instr) &&
+ (Args.hasArg(OPT_fprofile_instr_generate) ||
+ Args.hasArg(OPT_fprofile_instr_generate_EQ) ||
----------------
IIRC, the general practice is to put this type of logic in the driver. Specifically, in Driver.cpp include something like
if (Args.hasArg(OPT_fprofile_ir_instr) &&
(Args.hasArg(OPT_fprofile_instr_generate) ||
Args.hasArg(OPT_fprofile_instr_generate_EQ) ||
Args.hasArg(OPT_fprofile_instr_use_EQ)))
// Add -fprofile_ir_instr flag....
http://reviews.llvm.org/D15829
More information about the cfe-commits
mailing list