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

[Chad Rosier via cfe-commits]

mcrosier added inline comments.

================
Comment at: lib/Driver/Tools.cpp:3279
@@ -3278,1 +3278,3 @@
+
+  Args.AddAllArgs(CmdArgs, options::OPT_fprofile_ir_instr);
 }
----------------
silvas wrote:
> 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
Ah, okay.  I still think this type of complexity should land in the driver, but if this is a special case then just ignore my comments.


http://reviews.llvm.org/D15829