[PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
Sean Silva via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 22 12:59:27 PST 2016
silvas added inline comments.
================
Comment at: lib/Driver/Tools.cpp:3279
@@ -3278,1 +3278,3 @@
+
+ Args.AddAllArgs(CmdArgs, options::OPT_fprofile_ir_instr);
}
----------------
xur wrote:
> mcrosier wrote:
> > 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.
> Thanks for pointing thi out. What about add a guard, like:
> - Args.AddAllArgs(CmdArgs, options::OPT_fprofile_ir_instr);
> + if (Args.hasArg(options::OPT_fprofile_ir_instr))
> + Args.AddAllArgs(CmdArgs, options::OPT_fprofile_ir_instr);
>
> But looking at it again, I think i need to remove this stmt in the most recently patch as OPT_profile_ir_instr is now a CC1 option. it will not be seen here, right?
>
>
> But looking at it again, I think i need to remove this stmt in the most recently patch as OPT_profile_ir_instr is now a CC1 option. it will not be seen here, right?
Yes. The patch should make no changes to lib/Driver and should require no tests in test/Driver
http://reviews.llvm.org/D15829
More information about the cfe-commits
mailing list