From patchwork Tue Jul 2 17:07:04 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Iyer, Balaji V" X-Patchwork-Id: 256466 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 1378E2C0085 for ; Wed, 3 Jul 2013 03:07:27 +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=UtnsmLPErXJeOUQb cdwtdKsnQ2KyBXDfEiluPHJMEcyi5/wCN85attk5S9okyc2WrW+8iRU0Iihjzjw4 PWMIQL9sc1m1lWq3h2ZHDU/GZnb1L/+ZNzESpGBAfxarQ2/w7qjjRCbr3Lv6xACt 7DnEZgpfJDAWNiytzBP+OoO8dsg= 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=1ZDItap1j1gMrri5TOvRXb DumF8=; b=xbQ71YUSPdEP6VR3jHSwC7IkzVAGR/e1T2dM4Mka7sCdLwWubtTeqt X8dx+PcnjtztkiaGfVGN4DPaakusPtxMh7o26vn07WBeHo2iVKz+puN/yMIXHnnH zwjFCLYvWG37c7sLidYmXMauyMkV1WnVNAFutRxzSLoY3YAcg94g4= Received: (qmail 23853 invoked by alias); 2 Jul 2013 17:07:20 -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 23843 invoked by uid 89); 2 Jul 2013 17:07:20 -0000 X-Spam-SWARE-Status: No, score=-5.8 required=5.0 tests=AWL, BAYES_00, KHOP_THREADED, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL, RP_MATCHES_RCVD, SPF_PASS, TW_TM autolearn=ham version=3.3.1 Received: from mga14.intel.com (HELO mga14.intel.com) (143.182.124.37) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 02 Jul 2013 17:07:18 +0000 Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by azsmga102.ch.intel.com with ESMTP; 02 Jul 2013 10:07:16 -0700 X-ExtLoop1: 1 Received: from fmsmsx104.amr.corp.intel.com ([10.19.9.35]) by fmsmga001.fm.intel.com with ESMTP; 02 Jul 2013 10:07:58 -0700 Received: from FMSMSX110.amr.corp.intel.com (10.18.116.10) by FMSMSX104.amr.corp.intel.com (10.19.9.35) with Microsoft SMTP Server (TLS) id 14.3.123.3; Tue, 2 Jul 2013 10:07:04 -0700 Received: from fmsmsx101.amr.corp.intel.com ([169.254.1.114]) by FMSMSX110.amr.corp.intel.com ([169.254.1.147]) with mapi id 14.03.0123.003; Tue, 2 Jul 2013 10:07:04 -0700 From: "Iyer, Balaji V" To: "gcc-patches@gcc.gnu.org" CC: "rth@redhat.com" Subject: FW: [PING][PATCH] for for c/PR57541 Date: Tue, 2 Jul 2013 17:07:04 +0000 Message-ID: References: In-Reply-To: MIME-Version: 1.0 X-Virus-Found: No Hello everyone, Is this Patch OK for trunk? -Balaji V. Iyer. > -----Original Message----- > From: Iyer, Balaji V > Sent: Monday, June 17, 2013 8:10 AM > To: gcc-patches@gcc.gnu.org > Subject: [PING][PATCH] for for c/PR57541 > > Hello Everyone, > Is this patch OK for trunk? > > Thanks, > > Balaji V. Iyer. > > > -----Original Message----- > > From: Iyer, Balaji V > > Sent: Wednesday, June 12, 2013 1:22 PM > > To: gcc-patches@gcc.gnu.org > > Cc: anna.m.tikhonova@gmail.com > > Subject: [PATCH] for for c/PR57541 > > > > Hello Everyone, > > Attach, please find a patch that will fix the issues in C/PR57541. > > The issue reported was that the parameters passed into the builtin > > array notation reduction functions were not checked correctly. This patch > should fix that issue. > > It is tested on x86 and x86_64 and it seem to pass all the tests. I > > have also included a testsuite. > > > > Here are the ChangeLog entries: > > > > gcc/c/ChangeLog > > 2013-06-12 Balaji V. Iyer > > > > * c-array-notation.c (fix_builtin_array_notation_fn): Added a call to > > valid_no_reduce_fn_params_p and valid_reduce_fn_params_p. > > (build_array_notation_expr): Added a check for capturing the return > > value (i.e. void) of __sec_reduce_mutating function. > > > > > > gcc/c-family/ChangeLog: > > 2013-06-12 Balaji V. Iyer > > > > * array-notation-common.c (valid_reduce_fn_params_p): New function. > > (valid_no_reduce_fn_params_p): Likewise. > > * c-common.h (valid_reduce_fn_params_p): Added a new prototype. > > (valid_no_reduce_fn_params_p): Likewise. > > > > gcc/testsuite/ChangeLog > > 2013-06-12 Balaji V. Iyer > > > > PR c/57541 > > * c-c++-common/cilk-plus/AN/pr57541-2.c: New test. > > * c-c++-common/cilk-plus/AN/rank_mismatch2.c: Fixed a bug by replacing > > a comma with an operation. > > > > > > Thanks, > > > > Balaji V. Iyer. diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog old mode 100644 new mode 100755 index 3d8f68f..52a58a6 Binary files a/gcc/c-family/ChangeLog and b/gcc/c-family/ChangeLog differ diff --git a/gcc/c-family/array-notation-common.c b/gcc/c-family/array-notation-common.c old mode 100644 new mode 100755 index 489b67c..6c1c7e2 --- a/gcc/c-family/array-notation-common.c +++ b/gcc/c-family/array-notation-common.c @@ -560,3 +560,125 @@ find_correct_array_notation_type (tree op) } return return_type; } + +/* Returns true if the function call in BUILTIN_FN (of type CALL_EXPR) if the + number of parameters for the array notation reduction functions are + correct. */ + +bool +valid_no_reduce_fn_params_p (tree builtin_fn) +{ + switch (is_cilkplus_reduce_builtin (CALL_EXPR_FN (builtin_fn))) + { + case BUILT_IN_CILKPLUS_SEC_REDUCE_ADD: + case BUILT_IN_CILKPLUS_SEC_REDUCE_MUL: + case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_ZERO: + case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_ZERO: + case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX: + case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN: + case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN_IND: + case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX_IND: + case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_NONZERO: + case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_NONZERO: + if (call_expr_nargs (builtin_fn) != 1) + { + error_at (EXPR_LOCATION (builtin_fn), + "builtin function %qE can only have one argument", + CALL_EXPR_FN (builtin_fn)); + return false; + } + break; + case BUILT_IN_CILKPLUS_SEC_REDUCE: + case BUILT_IN_CILKPLUS_SEC_REDUCE_MUTATING: + if (call_expr_nargs (builtin_fn) != 3) + { + error_at (EXPR_LOCATION (builtin_fn), + "builtin function %qE must have 3 arguments", + CALL_EXPR_FN (builtin_fn)); + return false; + } + break; + default: + /* If it is not a builtin function, then no reason for us do any checking + here. */ + return true; + } + return true; +} + +/* Returns true if the parameters of BUILTIN_FN (array notation builtin + function): IDENTITY_VALUE and FUNC_PARM are valid. */ + +bool +valid_reduce_fn_params_p (tree builtin_fn, tree identity_value, tree func_parm) +{ + switch (is_cilkplus_reduce_builtin (CALL_EXPR_FN (builtin_fn))) + { + case BUILT_IN_CILKPLUS_SEC_REDUCE_ADD: + case BUILT_IN_CILKPLUS_SEC_REDUCE_MUL: + case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_ZERO: + case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_ZERO: + case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX: + case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN: + case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN_IND: + case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX_IND: + case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_NONZERO: + case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_NONZERO: + func_parm = CALL_EXPR_ARG (builtin_fn, 0); + if (!contains_array_notation_expr (func_parm)) + { + error_at (EXPR_LOCATION (builtin_fn), + "builtin function %qE must contain one argument with " + "array notations", CALL_EXPR_FN (builtin_fn)); + return false; + } + if (TREE_CODE (func_parm) == CALL_EXPR + && is_cilkplus_reduce_builtin (CALL_EXPR_FN (func_parm)) != + BUILT_IN_NONE) + { + error_at (EXPR_LOCATION (builtin_fn), + "builtin functions cannot be used as arguments for %qE", + CALL_EXPR_FN (builtin_fn)); + return false; + } + return true; + case BUILT_IN_CILKPLUS_SEC_REDUCE: + if (!contains_array_notation_expr (func_parm)) + { + error_at (EXPR_LOCATION (builtin_fn), + "second argument of %qE must contain array notation " + "expressions", CALL_EXPR_FN (builtin_fn)); + return false; + } + if (TREE_CODE (func_parm) == CALL_EXPR + && is_cilkplus_reduce_builtin (CALL_EXPR_FN (func_parm)) != + BUILT_IN_NONE) + { + error_at (EXPR_LOCATION (builtin_fn), + "builtin functions cannot be used as arguments for %qE", + CALL_EXPR_FN (builtin_fn)); + return false; + } + return true; + case BUILT_IN_CILKPLUS_SEC_REDUCE_MUTATING: + if (!contains_array_notation_expr (func_parm)) + { + error_at (EXPR_LOCATION (builtin_fn), + "builtin function %qE must contain one argument with array " + "notations", CALL_EXPR_FN (builtin_fn)); + return false; + } + if (TREE_CODE (identity_value) != ADDR_EXPR + && !POINTER_TYPE_P (TREE_TYPE (identity_value))) + { + error_at (EXPR_LOCATION (builtin_fn), + "identity value of __sec_reduce function must be a " + "pointer or reference"); + return false; + } + return true; + default: + return true; + } + return true; +} diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h old mode 100644 new mode 100755 index 8eaf54f..74a1c6f --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -1179,4 +1179,6 @@ extern void replace_array_notations (tree *, bool, vec *, extern tree find_inv_trees (tree *, int *, void *); extern tree replace_inv_trees (tree *, int *, void *); extern tree find_correct_array_notation_type (tree op); +extern bool valid_no_reduce_fn_params_p (tree); +extern bool valid_reduce_fn_params_p (tree, tree, tree); #endif /* ! GCC_C_COMMON_H */ diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog old mode 100644 new mode 100755 index 72d182b..ea79745 Binary files a/gcc/c/ChangeLog and b/gcc/c/ChangeLog differ diff --git a/gcc/c/c-array-notation.c b/gcc/c/c-array-notation.c old mode 100644 new mode 100755 index 3285969..22ebd5f --- a/gcc/c/c-array-notation.c +++ b/gcc/c/c-array-notation.c @@ -139,6 +139,11 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var) if (an_type == BUILT_IN_NONE) return NULL_TREE; + /* Checking if the number of parameters passed into the reduction function + is valid. */ + if (!valid_no_reduce_fn_params_p (an_builtin_fn)) + return error_mark_node; + if (an_type == BUILT_IN_CILKPLUS_SEC_REDUCE || an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MUTATING) { @@ -154,7 +159,12 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var) /* Fully fold any EXCESSIVE_PRECISION EXPR that can occur in the function parameter. */ func_parm = c_fully_fold (func_parm, false, NULL); - + + /* Checking if the parameter types passed into the reduction function is + valid. */ + if (!valid_reduce_fn_params_p (an_builtin_fn, identity_value, func_parm)) + return error_mark_node; + location = EXPR_LOCATION (an_builtin_fn); if (!find_rank (location, an_builtin_fn, an_builtin_fn, true, &rank)) @@ -721,7 +731,14 @@ build_array_notation_expr (location_t location, tree lhs, tree lhs_origtype, tree rhs_node = (*rhs_list)[ii]; if (TREE_CODE (rhs_node) == CALL_EXPR) { - builtin_loop = fix_builtin_array_notation_fn (rhs_node, &new_var); + if (is_cilkplus_reduce_builtin (CALL_EXPR_FN (rhs_node)) == + BUILT_IN_CILKPLUS_SEC_REDUCE_MUTATING) + { + error_at (location, "void value not ignored as it ought to be"); + builtin_loop = error_mark_node; + } + else + builtin_loop = fix_builtin_array_notation_fn (rhs_node, &new_var); if (builtin_loop == error_mark_node) { pop_stmt_list (an_init); diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog old mode 100644 new mode 100755 index 187f2ef..65d7576 Binary files a/gcc/testsuite/ChangeLog and b/gcc/testsuite/ChangeLog differ diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541-2.c b/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541-2.c new file mode 100644 index 0000000..b0e67bf --- /dev/null +++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541-2.c @@ -0,0 +1,22 @@ +int A[10]; +int j = 0; +int main () { + int a; + int *q; + const int c = 5; + int my_func (int, int); + int my_func2 (int *, int); + a = __sec_reduce_add (1); /* { dg-error "must contain one argument with" } */ + a = __sec_reduce_mul (A[5]); /* { dg-error "must contain one argument with" } */ + a = __sec_reduce_max (A[:], 2); /* { dg-error "can only have one argument" } */ + a = __sec_reduce (1); /* { dg-error "must have 3 arguments" } */ + a = __sec_reduce (1, 2, 3); /* { dg-error "must contain array notation" } */ + a = __sec_reduce (j, A[:], my_func); /* This is OK. */ + a = __sec_reduce (c, A[:], my_func); /* This is OK. */ + __sec_reduce_mutating (&j, A[:], my_func2); /* This IS OK. */ + __sec_reduce_mutating (j, A[:], my_func2); /* { dg-error "pointer or" } */ + __sec_reduce_mutating (*q, A[:], my_func2); /* { dg-error "pointer or" } */ + a = __sec_reduce_mutating (&j, A[:], my_func2); /* { dg-error "void value not ignored as it ought to be" } */ + a = __sec_reduce_add (__sec_reduce_add (A[:])); /* { dg-error "builtin functions cannot be used as arguments for" } */ + return 0; +} diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/rank_mismatch2.c b/gcc/testsuite/c-c++-common/cilk-plus/AN/rank_mismatch2.c index 4a4882d..9346726 100644 --- a/gcc/testsuite/c-c++-common/cilk-plus/AN/rank_mismatch2.c +++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/rank_mismatch2.c @@ -20,7 +20,7 @@ int main (void) argc += function_call (function_call (array[:] + array2[5:10:2][:])); /* { dg-error "rank mismatch between" } */ - argc += __sec_reduce_add (array[:], array2[:][:]); /* { dg-error "rank mismatch between" } */ + argc += __sec_reduce_add (array[:] / array2[:][:]); /* { dg-error "rank mismatch between" } */ argc += __sec_reduce_add (array2[:][:]) + argc; /* This is OK. */ return argc;