On Oct 21, 2013 8:18 AM, "Diego Novillo" <[email protected]> wrote: > > On Thu, Oct 10, 2013 at 2:48 PM, Hans Wennborg <[email protected]> wrote: > > On Mon, Oct 7, 2013 at 11:09 AM, Diego Novillo <[email protected]> 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 <[email protected]> > >> 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
