This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [GOOGLE] More strict checking for call args
- From: Xinliang David Li <davidxl at google dot com>
- To: Dehao Chen <dehao at google dot com>
- Cc: Richard Biener <richard dot guenther at gmail dot com>, Duncan Sands <baldrick at free dot fr>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 4 Jun 2013 08:59:39 -0700
- Subject: Re: [GOOGLE] More strict checking for call args
- References: <CAO2gOZWs7maFPDo=EZUe1mPARfNFxxnA5Yg3z0Wo0WS1+2ji2Q at mail dot gmail dot com> <51A85C16 dot 1030505 at free dot fr> <CAAkRFZL7aPp9WSxsj1yaujdBWrs91D=H0d_+KxVcgnh=xnt7Ng at mail dot gmail dot com> <CAO2gOZUZ0xTXoMPmLyhsCRkyFQehu7A7EiuqS1E40hBvd8yvxQ at mail dot gmail dot com> <CAFiYyc2q4nGUWNqDbqcpMunEPr-jQN0f2==h-rmMUCjqTWtTUw at mail dot gmail dot com> <CAO2gOZXqae0ju2nMK=f_XSKZFMOscdmL__P=MYf7x01BBs=jOw at mail dot gmail dot com>
Richard's question is that inlining should deal with extra arguments
just fine -- those paths (the insane profile case) won't be executed
anyway. Do you have a case showing otherwise (i.e., the mismatch
upsets the compiler?)
David
On Tue, Jun 4, 2013 at 8:07 AM, Dehao Chen <dehao@google.com> wrote:
> On Tue, Jun 4, 2013 at 3:56 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>>
>> On Tue, Jun 4, 2013 at 2:55 AM, Dehao Chen <dehao@google.com> wrote:
>> > Hi,
>> >
>> > This patch was committed to google branch. But I think it is of
>> > general interest. So is it ok for trunk?
>> >
>> > Thanks,
>> > Dehao
>> >
>> > gcc/ChangeLog:
>> >
>> > 2013-06-03 Dehao Chen <dehao@google.com>
>> >
>> > *gimple-low.c (gimple_check_call_args): Restrict the call_arg check to
>> > contain same number of args.
>> >
>> > Index: gcc/gimple-low.c
>> > ===================================================================
>> > --- gcc/gimple-low.c (revision 199570)
>> > +++ gcc/gimple-low.c (working copy)
>> > @@ -243,6 +243,8 @@ gimple_check_call_args (gimple stmt, tree fndecl)
>> > && !fold_convertible_p (DECL_ARG_TYPE (p), arg)))
>> > return false;
>> > }
>> > + if (p != NULL)
>> > + return false;
>>
>> Please add a comment here, like
>>
>> /* Not enough parameters to the function call. */
>> if (p != NULL)
>> return false;
>>
>> note that I believe we can deal with this situation just fine during inlining,
>> we just leave the parameters uninitialized.
>>
>> So - why do you think the test is a good idea? The whole function should
>> ideally be not necessary and is just there to avoid situations we cannot
>> deal with during inlining.
>
> This check is necessary when profile does not match. E.g. if an
> indirect call target profile is targeting to an incorrect callee, this
> patch can make sure it's verified before actually transforming code.
> This could happen is you use an out-of-date profile to optimize for
> new code.
>
> Dehao
>
>
>
>>
>> Richard.
>>
>> > }
>> > else if (parms)
>> > {