Add -fauto-profile option to Clang driver

[David Blaikie]

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

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