From patchwork Wed Jun 5 15:18:46 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Iyer, Balaji V" X-Patchwork-Id: 249109 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id DBB5D2C00A0 for ; Thu, 6 Jun 2013 01:19:46 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:references:in-reply-to :content-type:mime-version; q=dns; s=default; b=GSOMH3/UbJyyC6TR fjwMm4gn8XWEyh7wIwyBJnMGZl/+53Bnm2PhUIX2k65r8cJy5w3ayVkFUaqmBt92 PiLL9tF3NPgzwXd5qPGZI6qG4AjGRNfGqjsVcprHer5fRJFYHiaWmZKa2hkt0Bgx TepeVIoxXhvJFmcS2hb7VJ1d1FM= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:references:in-reply-to :content-type:mime-version; s=default; bh=K6iyVUqKKM78f7ogm5f9jB +gxjg=; b=qo2HTjm2hWAL9/6xa8DlmFUV8D+bbzTJhkw1mIIKLxIN0+7PgONtWB 1+QlDu1s2oVSe2VFqggDwcOWQU81S1kxAZQMag5kGuS4/YCZV0QnaHqkcFTk0Sm5 wks7W4nYsFkogJvY02yqTNAStr/1zGf2qI2zm9GQbIU9gvuEs8pF8= Received: (qmail 21596 invoked by alias); 5 Jun 2013 15:19:40 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 21586 invoked by uid 89); 5 Jun 2013 15:19:40 -0000 X-Spam-SWARE-Status: No, score=-6.1 required=5.0 tests=AWL, BAYES_00, KHOP_THREADED, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL, RP_MATCHES_RCVD, SPF_PASS, TW_RG autolearn=ham version=3.3.1 Received: from mga03.intel.com (HELO mga03.intel.com) (143.182.124.21) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Wed, 05 Jun 2013 15:19:39 +0000 Received: from azsmga002.ch.intel.com ([10.2.17.35]) by azsmga101.ch.intel.com with ESMTP; 05 Jun 2013 08:19:35 -0700 X-ExtLoop1: 1 Received: from fmsmsx106.amr.corp.intel.com ([10.19.9.37]) by AZSMGA002.ch.intel.com with ESMTP; 05 Jun 2013 08:19:04 -0700 Received: from FMSMSX110.amr.corp.intel.com (10.18.116.10) by FMSMSX106.amr.corp.intel.com (10.19.9.37) with Microsoft SMTP Server (TLS) id 14.3.123.3; Wed, 5 Jun 2013 08:18:47 -0700 Received: from fmsmsx101.amr.corp.intel.com ([169.254.1.135]) by FMSMSX110.amr.corp.intel.com ([169.254.1.115]) with mapi id 14.03.0123.003; Wed, 5 Jun 2013 08:18:47 -0700 From: "Iyer, Balaji V" To: Jeff Law CC: "gcc-patches@gcc.gnu.org" , Steve Ellcey Subject: RE: [PATCH] pr57457 Date: Wed, 5 Jun 2013 15:18:46 +0000 Message-ID: References: <51A8C6B0.90302@redhat.com> <51ACE93A.6000307@redhat.com> <51AE2CD3.5040001@redhat.com> <51AEB51F.3080405@redhat.com> In-Reply-To: <51AEB51F.3080405@redhat.com> MIME-Version: 1.0 X-Virus-Found: No > -----Original Message----- > From: Jeff Law [mailto:law@redhat.com] > Sent: Tuesday, June 04, 2013 11:49 PM > To: Iyer, Balaji V > Cc: gcc-patches@gcc.gnu.org; Steve Ellcey > Subject: Re: [PATCH] pr57457 > > On 06/04/13 12:58, Iyer, Balaji V wrote: > > > > > > Actually, you can eliminate the entire if-statement (i.e. remove > > if-statement and make the body unconditional). This is because, if > > flag_enable_cilkplus is true and is_cilkplus_reduce_builtin (fundecl) > > is true, then it would have returned vec_safe_length(values) and will > > not even get to this point in the first place. So, this is technically > > equivalent to if (1). So, can I remove that and check it in also? It > > passes all my regression tests. > I originally thought it could be eliminated as well, but after further reflection I > couldn't convince myself it'd do the right thing for the case when > flag_enable_cilkplus is true but is_cilkplus_reduce_builtin was false. > > Note that triggering that code my be nontrivial, AFAICT we're suppressing a > diagnostic. So you're going to need to write invalid code to get into that > condition at the bottom of the loop at all. Hi Jeff, This email is going to be a bit long, so I apologize for it ahead of time. Here is the diff of c-typeck.c (to show that I have removed the unwanted if statement) ============================================================================= Here is the testcode that I have written that should trigger the "too few arguments to function error" with the appropriate note to say where the function declaration is. ---------------------------------- /* { dg-do compile } */ /* { dg-options "-fcilkplus" } */ /* Test-case contains no array notation but is compiled with -fcilkplus. It will still print the too few arguments func, thereby saying the if-statement after the for-loop to check for !flag_enable_cilkplus || !is_cilkplus_reduce_function (fundecl) is not valid is always taken. */ int func (int, int); /* { dg-message "declared here" } */ int main (void) { int a = 5, b = 2; return func (a); /* { dg-error "too few arguments to function" } */ } -------------------------------- Here is the output when I run this: bviyer@lakshmi2a:/export/users/gcc-svn/b-trunk-gcc/gcc> ../../install_dir/trunk-install/bin/gcc -fcilkplus test.c test.c: In function âmainâ: test.c:14:3: error: too few arguments to function âfuncâ return func (a); /* { dg-error "too few arguments to function" } */ ^ test.c:9:5: note: declared here int func (int, int); /* { dg-message "declared here" } */ ^ The whole patch with testcode is attached, so is this Ok for trunk? Here are the ChangeLog entries: gcc/c/ChangeLog 2013-06-05 Balaji V. Iyer * c-typeck.c (convert_arguments): Moved checking of builtin cilkplus reduction functions outside the for-loop. Added a check if the fundecl is non-NULL. Finally, removed an unwanted if-statement, and made the body unconditional gcc/testsuite/ChangeLog 2013-06-05 Balaji V. Iyer PR C/57457 * c-c++-common/cilk-plus/AN/pr57457.c: New test. * c-c++-common/cilk-plus/AN/pr57457-2.c: Likewise. Thanks, Balaji V. Iyer. > > jeff Index: gcc/c/ChangeLog =================================================================== --- gcc/c/ChangeLog (revision 199672) +++ gcc/c/ChangeLog (working copy) @@ -1,3 +1,9 @@ +2013-06-04 Balaji V. Iyer + + * 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. + 2013-06-03 Balaji V. Iyer * c-typeck.c (c_finish_if_stmt): Added a check to see if the rank of the Index: gcc/c/c-typeck.c =================================================================== --- gcc/c/c-typeck.c (revision 199672) +++ gcc/c/c-typeck.c (working copy) @@ -2942,6 +2942,8 @@ break; } } + if (flag_enable_cilkplus && fundecl && is_cilkplus_reduce_builtin (fundecl)) + return vec_safe_length (values); /* Scan the given expressions and types, producing individual converted arguments. */ @@ -2959,17 +2961,6 @@ bool npc; tree parmval; - // FIXME: I assume this code is here to handle the overloaded - // behavior of the __sec_reduce* builtins, and avoid giving - // argument mismatch warnings/errors. We should probably handle - // this with the resolve_overloaded_builtin infrastructure. - /* If the function call is a builtin function call, then we do not - worry about it since we break them up into its equivalent later and - we do the appropriate checks there. */ - if (flag_enable_cilkplus - && is_cilkplus_reduce_builtin (fundecl)) - continue; - if (type == void_type_node) { if (selector) @@ -3207,16 +3198,10 @@ if (typetail != 0 && TREE_VALUE (typetail) != void_type_node) { - /* If array notation is used and Cilk Plus is enabled, then we do not - worry about this error now. We will handle them in a later place. */ - if (!flag_enable_cilkplus - || !is_cilkplus_reduce_builtin (fundecl)) - { - error_at (input_location, - "too few arguments to function %qE", function); - inform_declaration (fundecl); - return -1; - } + error_at (input_location, + "too few arguments to function %qE", function); + inform_declaration (fundecl); + return -1; } return error_args ? -1 : (int) parmnum; Index: gcc/testsuite/ChangeLog =================================================================== --- gcc/testsuite/ChangeLog (revision 199672) +++ gcc/testsuite/ChangeLog (working copy) @@ -4,6 +4,11 @@ 2013-06-04 Balaji V. Iyer + PR C/57457 + * c-c++-common/cilk-plus/AN/pr57457.c: New test. + +2013-06-04 Balaji V. Iyer + * c-c++-common/cilk-plus/AN/array_test1.c (main): Replaced argc, argv parameters with void. (main2): Removed argc parameter. Index: gcc/testsuite/c-c++-common/cilk-plus/AN/pr57457-2.c =================================================================== --- gcc/testsuite/c-c++-common/cilk-plus/AN/pr57457-2.c (revision 0) +++ gcc/testsuite/c-c++-common/cilk-plus/AN/pr57457-2.c (revision 0) @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-fcilkplus" } */ + +/* Test-case contains no array notation but is compiled with -fcilkplus. + It will still print the too few arguments func, thereby saying the + if-statement after the for-loop to check for !flag_enable_cilkplus || + !is_cilkplus_reduce_function (fundecl) is not valid is always taken. */ + +int func (int, int); /* { dg-message "declared here" } */ + +int main (void) +{ + int a = 5, b = 2; + return func (a); /* { dg-error "too few arguments to function" } */ +} Index: gcc/testsuite/c-c++-common/cilk-plus/AN/pr57457.c =================================================================== --- gcc/testsuite/c-c++-common/cilk-plus/AN/pr57457.c (revision 0) +++ gcc/testsuite/c-c++-common/cilk-plus/AN/pr57457.c (revision 0) @@ -0,0 +1,39 @@ +/* { dg-do compile } */ +/* { dg-options "-fcilkplus" } */ + +/* This test has no array notation components in it and thus should compile + fine without crashing. */ + +typedef unsigned int size_t; +typedef int (*__compar_fn_t) (const void *, const void *); +extern void *bsearch (const void *__key, const void *__base, + size_t __nmemb, size_t __size, __compar_fn_t + __compar) + __attribute__ ((__nonnull__ (1, 2, 5))) ; +extern __inline __attribute__ ((__gnu_inline__)) void * +bsearch (const void *__key, const void *__base, size_t __nmemb, size_t + __size, + __compar_fn_t __compar) +{ + size_t __l, __u, __idx; + const void *__p; + int __comparison; + __l = 0; + __u = __nmemb; + while (__l < __u) + { + __idx = (__l + __u) / 2; + __p = (void *) (((const char *) __base) + + (__idx * __size)); + __comparison = (*__compar) (__key, + __p); + if (__comparison < 0) + __u = __idx; + else if (__comparison > 0) + __l = __idx + 1; + else + return (void *) + __p; + } + return ((void *)0); +} Index: gcc/c/c-typeck.c =================================================================== --- gcc/c/c-typeck.c (revision 199672) +++ gcc/c/c-typeck.c (working copy) @@ -2942,6 +2942,8 @@ break; } } + if (flag_enable_cilkplus && fundecl && is_cilkplus_reduce_builtin (fundecl)) + return vec_safe_length (values); /* Scan the given expressions and types, producing individual converted arguments. */ @@ -2959,17 +2961,6 @@ bool npc; tree parmval; - // FIXME: I assume this code is here to handle the overloaded - // behavior of the __sec_reduce* builtins, and avoid giving - // argument mismatch warnings/errors. We should probably handle - // this with the resolve_overloaded_builtin infrastructure. - /* If the function call is a builtin function call, then we do not - worry about it since we break them up into its equivalent later and - we do the appropriate checks there. */ - if (flag_enable_cilkplus - && is_cilkplus_reduce_builtin (fundecl)) - continue; - if (type == void_type_node) { if (selector) @@ -3207,16 +3198,10 @@ if (typetail != 0 && TREE_VALUE (typetail) != void_type_node) { - /* If array notation is used and Cilk Plus is enabled, then we do not - worry about this error now. We will handle them in a later place. */ - if (!flag_enable_cilkplus - || !is_cilkplus_reduce_builtin (fundecl)) - { - error_at (input_location, - "too few arguments to function %qE", function); - inform_declaration (fundecl); - return -1; - } + error_at (input_location, + "too few arguments to function %qE", function); + inform_declaration (fundecl); + return -1; } return error_args ? -1 : (int) parmnum;