Add -fauto-profile option to Clang driver

[Diego Novillo]

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).

>
>> +++ 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.