[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