[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