<p dir="ltr"><br>
On Oct 21, 2013 8:18 AM, "Diego Novillo" <<a href="mailto:dnovillo@google.com">dnovillo@google.com</a>> wrote:<br>
><br>
> On Thu, Oct 10, 2013 at 2:48 PM, Hans Wennborg <<a href="mailto:hans@chromium.org">hans@chromium.org</a>> wrote:<br>
> > On Mon, Oct 7, 2013 at 11:09 AM, Diego Novillo <<a href="mailto:dnovillo@google.com">dnovillo@google.com</a>> wrote:<br>
> >> This adds a new option -fauto-profile=filename to Clang. It tells the<br>
> >> driver to schedule the auto-profile pass and passes on the name of the<br>
> >> profile file to use.<br>
> >><br>
> >> This patch depends on the initial auto profile patch I posted a<br>
> >> couple of weeks ago: <a href="http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130923/188838.html">http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130923/188838.html</a><br>
> ><br>
> > This looks good as far as I can tell, just minor comments:<br>
><br>
> Thanks, Hans.<br>
><br>
> ><br>
> >> From bcfe88d501ad4ea299f0b64ce98b375c248a45c9 Mon Sep 17 00:00:00 2001<br>
> >> From: Diego Novillo <<a href="mailto:dnovillo@google.com">dnovillo@google.com</a>><br>
> >> Date: Wed, 2 Oct 2013 10:46:34 -0400<br>
> >> Subject: [PATCH] Add -fauto-profile to Clang's driver.<br>
> >><br>
> >> This adds a new option -fauto-profile=filename to Clang. It tells the<br>
> >> driver to schedule the auto-profile pass and passes on the name of the<br>
> >> profile file to use.<br>
> ><br>
> >> +++ b/lib/CodeGen/BackendUtil.cpp<br>
> >> @@ -154,6 +154,14 @@ static void addObjCARCOptPass(const PassManagerBuilder &Builder, PassManagerBase<br>
> >>      PM.add(createObjCARCOptPass());<br>
> >>  }<br>
> >><br>
> >> +static void addAutoProfilePass(const PassManagerBuilder &Builder,<br>
> >> +                               PassManagerBase &PM) {<br>
> >> +  const PassManagerBuilderWrapper &BuilderWrapper =<br>
> >> +      static_cast<const PassManagerBuilderWrapper&>(Builder);<br>
> >> +  const CodeGenOptions &opts = BuilderWrapper.getCGOpts();<br>
> ><br>
> > I'd have called the variable CGOpts, but whatever.<br>
><br>
> Is there a guideline for capitalization in names?  I don't mind camel<br>
> case, but camel casing in local symbols is confusing to me (I tend to<br>
> think of them as external symbols).<br></p>
<p dir="ltr"><a href="http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly">http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly</a><br>

><br>
> ><br>
> >> +++ b/lib/Driver/Tools.cpp<br>
> >> @@ -3020,6 +3020,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,<br>
> >><br>
> >>    // Forward -f options with positive and negative forms; we translate<br>
> >>    // these by hand.<br>
> >> +  if (Arg *A = Args.getLastArg(options::OPT_fauto_profile_EQ)) {<br>
> >> +    StringRef fname = A->getValue();<br>
> >> +    CmdArgs.push_back(Args.MakeArgString("-fauto-profile=" + fname));<br>
> ><br>
> > I think you can just use A->render(Args, CmdArgs); here, saving you<br>
> > the trouble of doing the MakeArgString dance.<br>
><br>
> Sure.<br>
><br>
> > Would this be a good point to check for the existence of the file, or<br>
> > what's the plan there?<br>
><br>
> If the file does not exist, the backend will complain.  But I agree<br>
> with you in that it's better for the FE to diagnose this.<br>
><br>
> Diego.<br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</p>