From patchwork Fri May 31 04:00: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: 247840 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 3B00F2C02AB for ; Fri, 31 May 2013 14:00:34 +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=x7JjAJ6GmnnkxUsKc0aXJvVjZNEj36t0kaedAekCOvBZveq93O Q5rOikM1ZY7ubS3vlFDYd+e4ifI5v3Exl5YTXMtrFYAgM705NVga1VVzSTJ1FTNk If8thqpyYDoEKi2nn3GxIuc4/Rng2NnT83P9n38lkA9T1cuKINpraExDA= 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=548FTqBoUKEbpc7jov9Vt4Foj8w=; b=yaw6S+DMWuCccOyypDTN UAdN6Gvy/YV4bU01OTlSuLOcICouWAPOZD/RtarUtgg3uGnaBrfW9eYbbzHiVvVd EiG9lT9X8NpUr7Vn8eQ+45W6PV9zeN9swkYHHmSKYN9YRUfSVJajpYBhFywTrF8i UoL5f7uADfrNg8Xtnc2bMVA= Received: (qmail 4849 invoked by alias); 31 May 2013 04:00:26 -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 4777 invoked by uid 89); 31 May 2013 04:00:18 -0000 X-Spam-SWARE-Status: No, score=-6.9 required=5.0 tests=AWL, BAYES_00, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL, RP_MATCHES_RCVD, SPF_PASS, TW_FN, TW_TM autolearn=ham version=3.3.1 Received: from mga02.intel.com (HELO mga02.intel.com) (134.134.136.20) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 31 May 2013 04:00:06 +0000 Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP; 30 May 2013 21:00:05 -0700 X-ExtLoop1: 1 Received: from fmsmsx105.amr.corp.intel.com ([10.19.9.36]) by orsmga001.jf.intel.com with ESMTP; 30 May 2013 21:00:04 -0700 Received: from fmsmsx152.amr.corp.intel.com (10.19.17.221) by FMSMSX105.amr.corp.intel.com (10.19.9.36) with Microsoft SMTP Server (TLS) id 14.3.123.3; Thu, 30 May 2013 21:00:04 -0700 Received: from fmsmsx101.amr.corp.intel.com ([169.254.1.135]) by fmsmsx152.amr.corp.intel.com ([169.254.6.169]) with mapi id 14.03.0123.003; Thu, 30 May 2013 21:00:04 -0700 From: "Iyer, Balaji V" To: "gcc-patches@gcc.gnu.org" CC: "rth@redhat.com" , "Aldy Hernandez (aldyh@redhat.com)" , Jeff Law , "Joseph S. Myers" Subject: [PATCH] Expanding array notations inside conditions Date: Fri, 31 May 2013 04:00:04 +0000 Message-ID: MIME-Version: 1.0 X-Virus-Found: No Hello Everyone, When I was looking at the erroneous test on PR 57452, I found out that array notations in conditions were not expanded correctly. The rank for the array notation condition must be same (or equal to zero) as the rank of the array notations inside the else-block and then-block (or they could be zero). I found out that GCC was not detecting these errors. I am very sorry for this mishap. The attached patch should fix that issue. I have also enclosed a test-suite code to make sure this gets caught in future. It is tested on x86_64 and seem to work OK. The only test that it is failing is the erronous test called if_test.c, and if patch specified in (http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01815.html) is applied, then that test will pass. Is this OK for trunk? Here are the ChangeLog entries: gcc/c/ChangeLog 2013-05-30 Balaji V. Iyer * c-typeck.c (c_finish_if_stmt): Added a check to see if the rank of the condition of the if-statement matches the rank of else-block and then- block when array notations are used. * c-parser.c (c_parser_declaration_or_fndef): Expanded array notation expression after the entire function body is parsed. (c_parser_expr_no_commas): Delayed creating array notation expressions to the end of function parsing. * c-array-notation.c (fix_conditional_array_notations_1): Expanded the whole if-statement instead of just the condition. (expand_array_notation_exprs): Added MODIFY_EXPR case. gcc/testsuite/ChangeLog 2013-05-30 Balaji V. Iyer * c-c++-common/cilk-plus/AN/if_test_errors.c (main): New testcase. * c-c++-common/cilk-plus/AN/rank_mismatch.c: Added a '-w' option to dg-option and an header comment. Thanks, Balaji V. Iyer. diff --git gcc/c/ChangeLog gcc/c/ChangeLog old mode 100644 new mode 100755 index d33fa2b..828d7c8 --- gcc/c/ChangeLog +++ gcc/c/ChangeLog @@ -1,3 +1,16 @@ +2013-05-30 Balaji V. Iyer + + * c-typeck.c (c_finish_if_stmt): Added a check to see if the rank of the + condition of the if-statement matches the rank of else_block and then- + block when array notations are used. + * c-parser.c (c_parser_declaration_or_fndef): Expanded array notation + expression after the entire function body is parsed. + (c_parser_expr_no_commas): Delayed creating array notation expressions + to the end of function parsing. + * c-array-notation.c (fix_conditional_array_notations_1): Expanded the + whole if-statement instead of just the condition. + (expand_array_notation_exprs): Added MODIFY_EXPR case. + 2013-05-29 Rainer Orth PR bootstrap/57450 diff --git gcc/c/c-array-notation.c gcc/c/c-array-notation.c old mode 100644 new mode 100755 index bf139a8..681e111 --- gcc/c/c-array-notation.c +++ gcc/c/c-array-notation.c @@ -1875,7 +1875,7 @@ fix_conditional_array_notations_1 (tree stmt) if (!find_rank (location, cond, cond, false, &rank)) return error_mark_node; - extract_array_notation_exprs (cond, false, &array_list); + extract_array_notation_exprs (stmt, false, &array_list); loop_init = push_stmt_list (); for (ii = 0; ii < vec_safe_length (array_list); ii++) { @@ -1895,12 +1895,12 @@ fix_conditional_array_notations_1 (tree stmt) vec_safe_push (sub_list, array_node); vec_safe_push (new_var_list, new_var); add_stmt (builtin_loop); - replace_array_notations (&cond, false, sub_list, new_var_list); + replace_array_notations (&stmt, false, sub_list, new_var_list); } } } - if (!find_rank (location, cond, cond, true, &rank)) + if (!find_rank (location, stmt, stmt, true, &rank)) { pop_stmt_list (loop_init); return error_mark_node; @@ -1911,7 +1911,7 @@ fix_conditional_array_notations_1 (tree stmt) pop_stmt_list (loop_init); return loop_init; } - extract_array_notation_exprs (cond, true, &array_list); + extract_array_notation_exprs (stmt, true, &array_list); if (vec_safe_length (array_list) == 0) return stmt; @@ -2761,6 +2761,18 @@ expand_array_notation_exprs (tree t) expand_array_notation_exprs (*tsi_stmt_ptr (ii_tsi)); } return t; + case MODIFY_EXPR: + { + location_t loc = EXPR_HAS_LOCATION (t) ? EXPR_LOCATION (t) : + UNKNOWN_LOCATION; + tree lhs = TREE_OPERAND (t, 0); + tree rhs = TREE_OPERAND (t, 1); + location_t rhs_loc = EXPR_HAS_LOCATION (rhs) ? EXPR_LOCATION (rhs) : + UNKNOWN_LOCATION; + t = build_array_notation_expr (loc, lhs, TREE_TYPE (lhs), NOP_EXPR, + rhs_loc, rhs, TREE_TYPE (rhs)); + return t; + } case CALL_EXPR: t = fix_array_notation_call_expr (t); return t; diff --git gcc/c/c-parser.c gcc/c/c-parser.c old mode 100644 new mode 100755 index b89d8c1..d6a500e --- gcc/c/c-parser.c +++ gcc/c/c-parser.c @@ -1756,6 +1756,8 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, DECL_STRUCT_FUNCTION (current_function_decl)->function_start_locus = c_parser_peek_token (parser)->location; fnbody = c_parser_compound_statement (parser); + if (flag_enable_cilkplus && contains_array_notation_expr (fnbody)) + fnbody = expand_array_notation_exprs (fnbody); if (nested) { tree decl = current_function_decl; @@ -5445,20 +5447,9 @@ c_parser_expr_no_commas (c_parser *parser, struct c_expr *after) rhs = c_parser_expr_no_commas (parser, NULL); rhs = default_function_array_read_conversion (exp_location, rhs); - /* The line below is where the statement has the form: - A = B, where A and B contain array notation exprs. So this is where - we handle those. */ - if (flag_enable_cilkplus - && (contains_array_notation_expr (lhs.value) - || contains_array_notation_expr (rhs.value))) - ret.value = build_array_notation_expr (op_location, lhs.value, - lhs.original_type, code, - exp_location, rhs.value, - rhs.original_type); - else - ret.value = build_modify_expr (op_location, lhs.value, lhs.original_type, - code, exp_location, rhs.value, - rhs.original_type); + ret.value = build_modify_expr (op_location, lhs.value, lhs.original_type, + code, exp_location, rhs.value, + rhs.original_type); if (code == NOP_EXPR) ret.original_code = MODIFY_EXPR; else diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c old mode 100644 new mode 100755 index 749c8e2..e5e1455 --- gcc/c/c-typeck.c +++ gcc/c/c-typeck.c @@ -8983,6 +8983,34 @@ c_finish_if_stmt (location_t if_locus, tree cond, tree then_block, { tree stmt; + /* If the condition has array notations, then the rank of the then_block and + else_block must be either 0 or be equal to the rank of the condition. If + the condition does not have array notations then break them up as it is + broken up in a normal expression. */ + if (flag_enable_cilkplus && contains_array_notation_expr (cond)) + { + size_t then_rank = 0, cond_rank = 0, else_rank = 0; + if (!find_rank (if_locus, cond, cond, true, &cond_rank)) + return; + if (then_block + && !find_rank (if_locus, then_block, then_block, true, &then_rank)) + return; + if (else_block + && !find_rank (if_locus, else_block, else_block, true, &else_rank)) + return; + if (cond_rank != then_rank && then_rank != 0) + { + error_at (if_locus, "rank-mismatch between if-statement%'s condition" + " and the then-block"); + return; + } + else if (cond_rank != else_rank && else_rank != 0) + { + error_at (if_locus, "rank-mismatch between if-statement%'s condition" + " and the else-block"); + return; + } + } /* Diagnose an ambiguous else if if-then-else is nested inside if-then. */ if (warn_parentheses && nested_if && else_block == NULL) { diff --git gcc/testsuite/ChangeLog gcc/testsuite/ChangeLog old mode 100644 new mode 100755 index 27bf134..5cba191 --- gcc/testsuite/ChangeLog +++ gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2013-05-30 Balaji V. Iyer + + * c-c++-common/cilk-plus/AN/if_test_errors.c (main): New testcase. + * c-c++-common/cilk-plus/AN/rank_mismatch.c: Added a '-w' option to + dg-option and an header comment. + 2013-05-30 Tobias Burnus PR middle-end/57073 diff --git gcc/testsuite/c-c++-common/cilk-plus/AN/if_test_errors.c gcc/testsuite/c-c++-common/cilk-plus/AN/if_test_errors.c new file mode 100755 index 0000000..5e88dce --- /dev/null +++ gcc/testsuite/c-c++-common/cilk-plus/AN/if_test_errors.c @@ -0,0 +1,56 @@ +/* { dg-do compile } */ +/* { dg-options "-fcilkplus" } */ + +#include +int main (int argc, char **argv) +{ + int x = 3, y, z, array[10], array2[10], TwodArray[10][10], jj,kk,ll ; + int array2_check[10], array2d_check[10][10], array2d[10][10]; + int FourDArray[10][10][10][10], array4[10][10][10][10]; + int array4_check[10][10][10][10]; + int ii = 0; + + x = atoi (argv[1])-10; + y = atoi (argv[1])/2; + z = (atoi (argv[1]))/5; + + if (!array[:]) /* This is OK! */ + array2[:] = 5; + else + array2[:] = 10; + if (!(array[0:10:1] + array[0:10:1])) /* { dg-error "condition and the then-block" } */ + array2d[:][:] = 5; + else + array2[:] = 10; + + if (!(array[0:10:1] + array[0:10:1])) /* { dg-error "condition and the else-block" } */ + array2[:] = 5; + else + array2d[:][:] = 10; + + + if (TwodArray[:][:] != 10) /* { dg-error "condition and the then-block" } */ + array2[:] = 10; + else + array2[:] = 5; + + if (FourDArray[43][:][:][:] != 10) /* This is OK! */ + array4[45][:][:][:] = 10; + else + array4[32][:][:][:] = 5; + + /* atoi(argv[1]) == 10, so it will convert all 10's to 5's */ + if (FourDArray[42][0:10:1][9:10:-1][0:5:2] != 10) /* { dg-error "condition and the then-block" } */ + array4[0:10:1][0:5:2][9:10:-1][0:5:2] = 10; + else + array4[0:10:1][0:5:2][9:10:-1][0:5:2] = 5; + + /* atoi(argv[1]) == 10, so it will convert all 10's to 5's */ + if (FourDArray[0:10:1][0:5:2][9:10:-1][x:y:z] + + FourDArray[0:10:1][0:5:2][9:-10:1][x:y:z] != 20) + array4[0:10:1][0:5:2][9:10:-1][x:y:z] = 10; + else + array4[0:10][0:5:2][9:10:-1][x:y:z] = 5; + + return 0; +} diff --git gcc/testsuite/c-c++-common/cilk-plus/AN/rank_mismatch.c gcc/testsuite/c-c++-common/cilk-plus/AN/rank_mismatch.c index a8c9dab..352d670 100644 --- gcc/testsuite/c-c++-common/cilk-plus/AN/rank_mismatch.c +++ gcc/testsuite/c-c++-common/cilk-plus/AN/rank_mismatch.c @@ -1,5 +1,8 @@ /* { dg-do compile } */ -/* { dg-options "-fcilkplus" } */ +/* { dg-options "-fcilkplus -w" } */ + +/* We use -w because in the first error, there will be a warning of setting an + integer to a pointer. Just ignore it to expose the rank mismatch error. */ int main (int argc, char **argv) {