[cfe-commits] [PATCH]: GPROF implementation

[Daniel Dunbar]

Hi Roman,

Looks good. I have two further small comments:
(1) There is a stray entry in the options table:
+  Args.AddLastArg(CmdArgs, options::OPT_fformat_extensions);

(2) Please add some kind of small test to CodeGen for this feature, at
the -cc1 level.

Otherwise, looks good, please commit!

Thanks,
 - Daniel

On Mon, Feb 7, 2011 at 9:35 AM, Roman Divacky <rdivacky at freebsd.org> wrote:
> hi daniel,
>
> thnx for review! I fixed all the things you didnt like, updated patches
> attached.
>
> may I commit this?
>
> I attached two comments below .
>
>> > +  if (!CGM.getCodeGenOpts().GPROFInstrument)
>> > +    return;
>>
>> I prefer that tests like this be at the call site, not at the caller. As
>> written, this function should be called MaybeEmitMCountInstrumentation(), if
>> that makes sense?
>
> I copied this from EmitFunctionInstrumentation() which does the checking
> in the caller - you may want to change that.
>
>> > -  // Explicitly warn that these options are unsupported, even though
>> > -  // we are allowing compilation to continue.
>> > -  for (arg_iterator it = Args.filtered_begin(options::OPT_pg),
>> > -         ie = Args.filtered_end(); it != ie; ++it) {
>> > -    (*it)->claim();
>> > -    D.Diag(clang::diag::warn_drv_clang_unsupported) <<
>> > (*it)->getAsString(Args);
>> > -  }
>> > +  if (Arg *A = Args.getLastArg(options::OPT_pg))
>> > +    if (Args.hasArg(options::OPT_fomit_frame_pointer))
>> > +      D.Diag(clang::diag::err_drv_argument_not_allowed_with)
>> > +        << A->getAsString(Args) << "-fomit-frame-pointer";
>>
>> I think this error makes more sense if the order is reverse, i.e.,
>>   clang: error: invalid argument '-fomit-frame-pointer' not allowed with
>>   '-pg'
>
> agreed. I copied this from darwin toolchain - you may want to change that
> there too.
>
>