[PATCH] pr57457

Iyer, Balaji V balaji.v.iyer@intel.com
Tue Jun 4 17:38:00 GMT 2013



> -----Original Message-----
> From: Jeff Law [mailto:law@redhat.com]
> Sent: Monday, June 03, 2013 3:07 PM
> To: Iyer, Balaji V
> Cc: gcc-patches@gcc.gnu.org; Steve Ellcey
> Subject: Re: [PATCH] pr57457
> 
> On 05/31/2013 12:01 PM, Iyer, Balaji V wrote:
> >
> >
> >> -----Original Message----- From: gcc-patches-owner@gcc.gnu.org
> >> [mailto:gcc-patches- owner@gcc.gnu.org] On Behalf Of Jeff Law Sent:
> >> Friday, May 31, 2013 11:50 AM To: Iyer, Balaji V Cc:
> >> gcc-patches@gcc.gnu.org; Steve Ellcey Subject: Re: [PATCH] pr57457
> >>
> >> On 05/31/2013 07:54 AM, Iyer, Balaji V wrote:
> >>> Hello Everyone, This patch will fix a bug reported in PR57457.
> >>> One of the array notation
> >> function was not checking for NULL_TREE before accessing its fields.
> >> This patch should fix that issue. A test case is also added.
> >>>
> >>> Is this OK for trunk?
> >>>
> >>> Here are the ChangeLog Entries:
> >>>
> >>> gcc/c/ChangeLog 2013-05-31  Balaji V. Iyer <balaji.v.iyer@intel.com>
> >>>
> >>> * c-array-notation.c (is_cilkplus_reduce_builtin): Added a check for
> >>> NULL_TREE parameter input.
> >>>
> >>> gcc/testsuite/ChangeLog 2013-05-31  Balaji V. Iyer
> >>> <balaji.v.iyer@intel.com>
> >>>
> >>> PR c/57457 * c-c++-common/cilk-plus/AN/pr57457.c: New testcase.
> >> So what you need to do is explain how you got into this function with
> >> a NULL fndecl and why that's OK.
> >
> > Hi Jeff, I looked into it, and there is another function call called
> > inform_declaration, and that does exactly what I did (i.e. check for
> > NULL fundecl before accessing its fields). From what I can tell,
> > fundecl will be NULL_TREE if a function declaration is a function
> > pointer.
> The problematical calls are coming from convert_arguments which is a bit
> inconsistent in how it handles a null FUNDECL.  In some places it checks it
> direction in convert_arguments and avoids certain actions.  In other places the
> callee checks.
> 
> The code looks like it's screaming to be simplified.  Neither flag_cilkplus nor
> FUNDECL change inside the main loop in convert_arguments, so the first thing
> I'd ponder is pulling that condition out of the loop.  And after doing that we a
> similar condition being used to suppress warnings just after the loop.  I wonder if
> we could have something like
> 
> if (flag_enable_cilkplus
>      && fundecl
>      && is_cilkplus_reduction_builtin (fundecl))
>    return vec_safe_length (values);
> 
> before the loop, then eliminate the the test inside the loop and just after the
> loop.
> 
> That simplifies the code a bit and should fix this problem unless I'm missing
> something?

Yes, that does simplify the whole thing. Here is an updated ChangeLog and patch (with testcode) attached. So, is it Ok for trunk?

gcc/testsuite/ChangeLog
2013-06-04  Balaji V. Iyer  <balaji.v.iyer@intel.com>

       PR C/57457
        * c-c++-common/cilk-plus/AN/pr57457.c: New test.

gcc/c/ChangeLog
2013-06-04  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * c-typeck.c (convert_arguments): Moved checking of builtin cilkplus
        reduction functions outside the for-loop.  Also, added a check if the
        fundecl is non-NULL.

Thanks,

Balaji V. Iyer.

> 
> 
> jeff
> 

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: patch_pr57457.txt
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20130604/8093e65e/attachment.txt>


More information about the Gcc-patches mailing list