From patchwork Wed Jul 10 14:53:14 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: 258098 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 950A32C0178 for ; Thu, 11 Jul 2013 00:53:31 +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:content-type:mime-version; q=dns; s=default; b=T1Z+Axs1uCVb17j5wGyCvG1/yFb6M8zomZl2wDoa4CZj+hLALl QHOftmyw6sXYmCF5P+RquM8H7xGBM1ayD/JhcL9ltI3741hir7gPAQcD3s0O959C v+Z761eFNzy9VYqU9/CAM4NZHFc5hjVqE6Y+TjDzSY8IZzZd5SJBtqGGk= 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:content-type:mime-version; s= default; bh=qQDAzpFtJM/D6JqCKLLHoG6LWMo=; b=piO3on+oCHZ6I6M0q30J nIQR39dDJSVzuO/CTZ9eZqqX1EdPPOS95vGtQRNMKFa6uR8SoL7IuUXvwSg57s28 whCk18tR2hjBSc68FnzpFomG/8YUgsYUW2ulrndV9X04DYjQsZLAMWEt5MojabiB Ed3OL3ATBhSOe90QPeWatdE= Received: (qmail 27413 invoked by alias); 10 Jul 2013 14:53:24 -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 27386 invoked by uid 89); 10 Jul 2013 14:53:19 -0000 X-Spam-SWARE-Status: No, score=-5.2 required=5.0 tests=AWL, BAYES_00, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL, RP_MATCHES_RCVD, SPF_PASS, TW_TM 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, 10 Jul 2013 14:53:18 +0000 Received: from azsmga001.ch.intel.com ([10.2.17.19]) by azsmga101.ch.intel.com with ESMTP; 10 Jul 2013 07:53:16 -0700 X-ExtLoop1: 1 Received: from fmsmsx104.amr.corp.intel.com ([10.19.9.35]) by azsmga001.ch.intel.com with ESMTP; 10 Jul 2013 07:53:15 -0700 Received: from fmsmsx151.amr.corp.intel.com (10.19.17.220) by FMSMSX104.amr.corp.intel.com (10.19.9.35) with Microsoft SMTP Server (TLS) id 14.3.123.3; Wed, 10 Jul 2013 07:53:15 -0700 Received: from fmsmsx101.amr.corp.intel.com ([169.254.1.114]) by FMSMSX151.amr.corp.intel.com ([169.254.7.186]) with mapi id 14.03.0123.003; Wed, 10 Jul 2013 07:53:14 -0700 From: "Iyer, Balaji V" To: "gcc-patches@gcc.gnu.org" CC: "rth@redhat.com" Subject: [PING][PING][PATCH] for for c/PR57541 Date: Wed, 10 Jul 2013 14:53:14 +0000 Message-ID: MIME-Version: 1.0 X-Virus-Found: No Did anyone get a chance to look at this? If so, is this OK for trunk? It is a relatively minor change... Thanks, -Balaji V. Iyer. > -----Original Message----- > From: Iyer, Balaji V > Sent: Tuesday, July 02, 2013 1:07 PM > To: gcc-patches@gcc.gnu.org > Cc: rth@redhat.com > Subject: FW: [PING][PATCH] for for c/PR57541 > > 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;