[cfe-commits] [PATCH]: GPROF implementation
Daniel Dunbar
daniel at zuster.org
Thu Feb 10 08:50:47 PST 2011
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.
>
>
More information about the cfe-commits
mailing list