From patchwork Fri Jun 7 17:12:41 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 249765 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 1242C2C0092 for ; Sat, 8 Jun 2013 03:12:51 +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 :message-id:date:from:mime-version:to:cc:subject:content-type; q=dns; s=default; b=wp22WhAo2lgKqRUjXgYo+bwc0mgOF6mbqa/TZDg5Qh+ 23jN1v7bU5ZRM7knif/ePvNK1QJATZCl7YRDyceNjcKSfBYfhFfcGBpkdMSrbgpB C9Vx8Cc0SPpZvPkxxx/oRPEttgNEsBugSXRHFOUIr1hI9vZUwA6Uu88S/c8jQptA = 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 :message-id:date:from:mime-version:to:cc:subject:content-type; s=default; bh=JtKv7nUp0juFLbhJB8msbbIFvig=; b=WIvk6wTAhkzkC7Ja5 +KZZIjGkn0AnRcF++Ho/xnTJEWAalusu/1prXBfYDTZx4o3I1DG+kx1A5OFOqdZW h/MlaCYn2E3xESqpvt5QTF4FAs7NsGg5GYy23z1h8nY3HbJOxjGAjT+GiimZmdvT reE7E8J471qu2ndN9yTwBnB0wA= Received: (qmail 927 invoked by alias); 7 Jun 2013 17:12:45 -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 916 invoked by uid 89); 7 Jun 2013 17:12:45 -0000 X-Spam-SWARE-Status: No, score=-6.6 required=5.0 tests=AWL, BAYES_00, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS, TW_TM autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 07 Jun 2013 17:12:44 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r57HCgWd007869 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 7 Jun 2013 13:12:42 -0400 Received: from houston.quesejoda.com (vpn-52-236.rdu2.redhat.com [10.10.52.236]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r57HCf9q031456; Fri, 7 Jun 2013 13:12:41 -0400 Message-ID: <51B21489.7010806@redhat.com> Date: Fri, 07 Jun 2013 12:12:41 -0500 From: Aldy Hernandez User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6 MIME-Version: 1.0 To: "Iyer, Balaji V" CC: gcc-patches , Jakub Jelinek Subject: [cilkplus] cleanup C++ pragma simd implementation X-Virus-Found: No The attached patch cleans up the C++ pragma simd implementation to share more things with the C front-end, particularly the type checking. For instance, I got rid of all the specialized parsing routines to just use generic cp_parser_*expression* and then perform the actual typechecking in the C shared routines. I also reworded some of the tests to match the C FE so we can share the tests there too. There are also some fixes scattered throughout, all in an effort to pass all the c-c++-common Cilk Plus pragma simd tests. Almost there... :). One bug I found that I don't know how to handle is: @@ -30315,6 +30319,9 @@ cp_parser_simd_for_init_statement (cp_parser *parser, tree *init, && CLASS_TYPE_P (TREE_TYPE (decl))) { tree rhs, new_expr; + // ?? FIXME: I don't see any definition for *init in this + // code path. ?? + gcc_unreachable (); cp_parser_parse_definitely (parser); cp_parser_require (parser, CPP_EQ, RT_EQ); AFAICT, *init is not initialized in this code path and the caller uses it. Balaji, what's the intent here? Committing to the aldyh/cilk-in-gomp branch. commit 51d8e99dd59ec4beb523101a35eedd21f2d439a4 Author: Aldy Hernandez Date: Fri Jun 7 12:06:12 2013 -0500 Clean up C++ implementation of pragma simd to use shared C/C++ type checking. Change error messages to match C front-end when possible. Misc fixes. diff --git a/gcc/c-family/c-cilkplus.c b/gcc/c-family/c-cilkplus.c index 84f1645..c02851e 100644 --- a/gcc/c-family/c-cilkplus.c +++ b/gcc/c-family/c-cilkplus.c @@ -207,12 +207,16 @@ c_check_cilk_loop_body (tree body) /* Validate a _Cilk_for construct (or a #pragma simd for loop, which has the same syntactic restrictions). Returns TRUE if there were no errors, FALSE otherwise. LOC is the location of the for. DECL - is the controlling variable. COND is the condition. INCR is the - increment expression. BODY is the body of the LOOP. */ + is the controlling variable. COND is the condition. INCRP is a + pointer the increment expression (in case, the increment needs to + be canonicalized). BODY is the body of the LOOP. */ static bool -c_check_cilk_loop (location_t loc, tree decl, tree cond, tree incr, tree body) +c_check_cilk_loop (location_t loc, tree decl, tree cond, tree *incrp, + tree body) { + tree incr = *incrp; + if (decl == error_mark_node || cond == error_mark_node || incr == error_mark_node @@ -284,10 +288,11 @@ c_check_cilk_loop (location_t loc, tree decl, tree cond, tree incr, tree body) return false; } - /* Validate the increment. */ + /* Validate and canonicalize the increment. */ incr = c_check_cilk_loop_incr (loc, decl, incr); if (incr == error_mark_node) return false; + *incrp = incr; if (!c_check_cilk_loop_body (body)) return false; @@ -325,7 +330,7 @@ c_finish_cilk_simd_loop (location_t loc, { location_t rhs_loc; - if (!c_check_cilk_loop (loc, decl, cond, incr, body)) + if (!c_check_cilk_loop (loc, decl, cond, &incr, body)) return NULL; /* In the case of "for (int i = 0...)", init will be a decl. It should @@ -345,7 +350,11 @@ c_finish_cilk_simd_loop (location_t loc, init = build_modify_expr (loc, decl, NULL_TREE, NOP_EXPR, rhs_loc, init, NULL_TREE); } - gcc_assert (TREE_CODE (init) == MODIFY_EXPR); + + // The C++ parser just gives us the rhs. + if (TREE_CODE (init) != MODIFY_EXPR) + init = build2 (MODIFY_EXPR, void_type_node, decl, init); + gcc_assert (TREE_OPERAND (init, 0) == decl); tree initv = make_tree_vec (1); diff --git a/gcc/cp/ChangeLog.cilkplus b/gcc/cp/ChangeLog.cilkplus index 7254b81..e7f7596 100644 --- a/gcc/cp/ChangeLog.cilkplus +++ b/gcc/cp/ChangeLog.cilkplus @@ -1,9 +1,12 @@ 2013-05-21 Balaji V. Iyer + Aldy Hernandez * cp-tree.h (p_simd_valid_stmts_in_body_p): New prototype. + (finish_cilk_for_cond): Likewise. * parser.h (IN_CILK_P_SIMD_FOR): New #define. * Make-lang.in (CXX_AND_OBJCXX_OBJS): Added new obj-file cp-cilkplus.o * cp-cilkplus.c: New file. + * semantics.c (finish_cilk_for_cond): New. * parser.c (cp_parser_pragma): Added a PRAGMA_CILK_SIMD case. (cp_parser_cilk_simd_vectorlength): New function. (cp_parser_cilk_simd_linear): Likewise. diff --git a/gcc/cp/cp-cilkplus.c b/gcc/cp/cp-cilkplus.c index f387e2f..e6bdf8a 100644 --- a/gcc/cp/cp-cilkplus.c +++ b/gcc/cp/cp-cilkplus.c @@ -46,9 +46,6 @@ cpp_validate_cilk_plus_loop_aux (tree *tp, int *walk_subtrees, void *data) if (!tp || !*tp) return NULL_TREE; - // Call generic C version. - (void) c_validate_cilk_plus_loop (tp, walk_subtrees, data); - if (TREE_CODE (*tp) == THROW_EXPR) { error_at (loc, "throw expressions are not allowed inside loops " diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 97e4e3c..ba088a5 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -5790,6 +5790,7 @@ extern void finish_omp_taskyield (void); extern void finish_omp_taskgroup (tree); extern void finish_omp_cancel (tree); extern void finish_omp_cancellation_point (tree); +extern tree finish_cilk_for_cond (tree); extern tree begin_transaction_stmt (location_t, tree *, int); extern void finish_transaction_stmt (tree, tree, int, tree); extern tree build_transaction_expr (location_t, tree, int, tree); diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 68a277b..c93ea9e 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -29899,7 +29899,11 @@ cp_parser_pragma (cp_parser *parser, enum pragma_context context) case PRAGMA_CILK_SIMD: if (context == pragma_external) - goto bad_stmt; + { + error_at (pragma_tok->location, + "%<#pragma simd%> must be inside a function"); + break; + } cp_parser_cilk_simd_construct (parser, pragma_tok); return true; @@ -30198,7 +30202,7 @@ cp_parser_cilk_simd_construct (cp_parser *parser, cp_token *pragma_token) if (cp_lexer_next_token_is_not_keyword (parser->lexer, RID_FOR)) { error_at (cp_lexer_peek_token (parser->lexer)->location, - "expected for-statement after %<#pragma simd%> clauses"); + "for statement expected"); return; } @@ -30229,7 +30233,7 @@ cp_parser_simd_for_init_statement (cp_parser *parser, tree *init, tree this_pre_body = push_stmt_list (); if (token->type == CPP_SEMICOLON) { - error_at (loc, "for-loop initializer must declare variable"); + error_at (loc, "expected iteration declaration"); return error_mark_node; } cp_parser_parse_tentatively (parser); @@ -30315,6 +30319,9 @@ cp_parser_simd_for_init_statement (cp_parser *parser, tree *init, && CLASS_TYPE_P (TREE_TYPE (decl))) { tree rhs, new_expr; + // ?? FIXME: I don't see any definition for *init in this + // code path. ?? + gcc_unreachable (); cp_parser_parse_definitely (parser); cp_parser_require (parser, CPP_EQ, RT_EQ); rhs = cp_parser_assignment_expression (parser, false, NULL); @@ -30338,151 +30345,6 @@ cp_parser_simd_for_init_statement (cp_parser *parser, tree *init, return decl; } - -/* Parses the increment expresion for a cilk_for or for statement with - #pragma simd. */ - -static tree -cp_parser_cilk_for_expression_iterator (cp_parser *parser) -{ - cp_token *token = cp_lexer_peek_token (parser->lexer); - tree name = NULL_TREE, expr = NULL_TREE; - enum tree_code t_code = NOP_EXPR; - - if (token->type == CPP_SEMICOLON) - { - error_at (token->location, "missing loop expression"); - return error_mark_node; - } - if (token->type == CPP_PLUS_PLUS || token->type == CPP_MINUS_MINUS) - { - cp_lexer_consume_token (parser->lexer); - token = cp_lexer_peek_token (parser->lexer); - t_code = token->type == CPP_PLUS_PLUS ? PREINCREMENT_EXPR - : PREDECREMENT_EXPR; - } - - if (token->type != CPP_NAME) - { - error_at (token->location, "invalid loop expression"); - cp_parser_skip_to_end_of_statement (parser); - return error_mark_node; - } - - name = cp_parser_lookup_name (parser, token->u.value, none_type, false, false, - false, NULL, token->location); - if (name == error_mark_node) - return error_mark_node; - - /* If name is not a declaration, then the loop is not valid. */ - if (!DECL_P (name)) - { - error_at (token->location, "invalid loop increment expression"); - return error_mark_node; - } - cp_lexer_consume_token (parser->lexer); - token = cp_lexer_peek_token (parser->lexer); - - if (t_code != NOP_EXPR) - { - if (token->type != CPP_CLOSE_PAREN) - { - error_at (token->location, "invalid loop expression"); - return error_mark_node; - } - return build2 (t_code, void_type_node, name, NULL_TREE); - } - - if (token->type == CPP_CLOSE_PAREN) - { - error_at (token->location, - "loop expression must modify control variable"); - return error_mark_node; - } - - cp_lexer_consume_token (parser->lexer); - if (token->type == CPP_PLUS_PLUS || token->type == CPP_MINUS_MINUS) - return build2 (token->type == CPP_PLUS_PLUS ? POSTINCREMENT_EXPR - : POSTDECREMENT_EXPR, void_type_node, name, NULL_TREE); - else if (token->type == CPP_EQ) - { - sorry ("loop with = operator"); - return error_mark_node; - } - else if (token->type == CPP_PLUS_EQ || token->type == CPP_MINUS_EQ) - t_code = token->type == CPP_PLUS_EQ ? PLUS_EXPR : MINUS_EXPR; - else if (token->type == CPP_MOD_EQ || token->type == CPP_XOR_EQ - || token->type == CPP_DIV_EQ || token->type == CPP_AND_EQ - || token->type == CPP_OR_EQ || token->type == CPP_AND_EQ - || token->type == CPP_LSHIFT_EQ || token->type == CPP_RSHIFT_EQ) - { - error_at (token->location, "invalid loop increment operation"); - return error_mark_node; - } - else - { - error_at (token->location, "invalid loop expression"); - return error_mark_node; - } - expr = cp_parser_binary_expression (parser, false, false, PREC_NOT_OPERATOR, - NULL); - if (expr == error_mark_node) - return expr; - - return build2 (MODIFY_EXPR, void_type_node, name, - build2 (t_code, TREE_TYPE (name), name, expr)); -} - -/* Parses the condition for a for-loop with pragma simd or _Cilk_for - loop. */ - -static tree -cp_parser_cilk_for_condition (cp_parser *parser) -{ - tree lhs, rhs; - enum tree_code code = ERROR_MARK; - - lhs = cp_parser_binary_expression (parser, false, false, - PREC_SHIFT_EXPRESSION, NULL); - switch (cp_lexer_peek_token (parser->lexer)->type) - { - case CPP_NOT_EQ: - code = NE_EXPR; - break; - case CPP_LESS: - code = LT_EXPR; - break; - case CPP_LESS_EQ: - code = LE_EXPR; - break; - case CPP_GREATER_EQ: - code = GE_EXPR; - break; - case CPP_GREATER: - code = GT_EXPR; - break; - case CPP_EQ_EQ: - error_at (cp_lexer_peek_token (parser->lexer)->location, - "equality test not permitted in the Cilk_for loop"); - break; - default: - error_at (cp_lexer_peek_token (parser->lexer)->location, - "missing comparison operator in the loop condition"); - } - cp_lexer_consume_token (parser->lexer); - - rhs = cp_parser_binary_expression (parser, false, false, - PREC_SHIFT_EXPRESSION, NULL); - parser->scope = NULL_TREE; - parser->qualifying_scope = NULL_TREE; - parser->object_scope = NULL_TREE; - - if (code == ERROR_MARK || lhs == error_mark_node || rhs == error_mark_node) - return error_mark_node; - - return build2 (code, boolean_type_node, lhs, rhs); -} - /* Top-level function to parse _Cilk_for and for statements. */ static tree @@ -30552,12 +30414,14 @@ cp_parser_cilk_for (cp_parser *parser, enum rid for_keyword, tree clauses) return error_mark_node; if (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON)) { - error_at (loc, "%s-loop requires a condition", - for_keyword == RID_FOR ? "for" : "_Cilk_for"); + error_at (loc, "missing condition"); cond = error_mark_node; } else - cond = cp_parser_cilk_for_condition (parser); + { + cond = cp_parser_condition (parser); + cond = finish_cilk_for_cond (cond); + } if (cond == error_mark_node) valid = false; @@ -30565,12 +30429,11 @@ cp_parser_cilk_for (cp_parser *parser, enum rid for_keyword, tree clauses) if (cp_lexer_next_token_is (parser->lexer, CPP_CLOSE_PAREN)) { - error_at (loc, "%s-loop requires an increment expression", - for_keyword == RID_FOR ? "for" : "_Cilk_for"); + error_at (loc, "missing increment"); incr_expr = error_mark_node; } else - incr_expr = cp_parser_cilk_for_expression_iterator (parser); + incr_expr = cp_parser_expression (parser, false, NULL); if (incr_expr == error_mark_node) { @@ -30592,10 +30455,8 @@ cp_parser_cilk_for (cp_parser *parser, enum rid for_keyword, tree clauses) if (for_keyword == RID_FOR) { - tree initv, incrv, condv, declv, omp_simd_node, body = NULL_TREE; - parser->in_statement = IN_CILK_P_SIMD_FOR; - body = push_stmt_list (); + tree body = push_stmt_list (); cp_parser_statement (parser, NULL_TREE, false, NULL); body = pop_stmt_list (body); @@ -30604,23 +30465,16 @@ cp_parser_cilk_for (cp_parser *parser, enum rid for_keyword, tree clauses) nodes, just return an error mark node. */ if (!cpp_validate_cilk_plus_loop (body)) return error_mark_node; - - /* Now pass all the information into finish_omp_for. */ - initv = make_tree_vec (1); - condv = make_tree_vec (1); - incrv = make_tree_vec (1); - declv = make_tree_vec (1); - TREE_VEC_ELT (initv, 0) = init; - TREE_VEC_ELT (condv, 0) = cond; - TREE_VEC_ELT (incrv, 0) = incr_expr; - TREE_VEC_ELT (declv, 0) = decl; - omp_simd_node = finish_omp_for (loc, OMP_SIMD, declv, initv, condv, - incrv, body, pre_body, clauses); - return omp_simd_node; + + return c_finish_cilk_simd_loop (loc, decl, init, cond, incr_expr, + body, clauses); } else - /* Fix this when _Cilk_for is added into the mix. */ - return NULL_TREE; + { + /* Handle _Cilk_for here when implemented. */ + gcc_unreachable (); + return NULL_TREE; + } } #include "gt-cp-parser.h" diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 9c5dc74..16e2472 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -6054,6 +6054,13 @@ finish_omp_cancellation_point (tree clauses) finish_expr_stmt (stmt); } +/* Perform any canonicalization of the conditional in a Cilk for loop. */ +tree +finish_cilk_for_cond (tree cond) +{ + return maybe_convert_cond (cond); +} + /* Begin a __transaction_atomic or __transaction_relaxed statement. If PCOMPOUND is non-null, this is for a function-transaction-block, and we should create an extra compound stmt. */ diff --git a/gcc/testsuite/c-c++-common/cilk-plus/PS/for4.c b/gcc/testsuite/c-c++-common/cilk-plus/PS/for4.c new file mode 100644 index 0000000..2da8235 --- /dev/null +++ b/gcc/testsuite/c-c++-common/cilk-plus/PS/for4.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-O3 -fcilkplus" } */ + +int *a, *c; + +void foo() +{ + int i, j; + + // Pointers are OK. + #pragma simd + for (int *i=c; i < c; ++i) + *a = '5'; +} diff --git a/gcc/testsuite/c-c++-common/cilk-plus/PS/for5.c b/gcc/testsuite/c-c++-common/cilk-plus/PS/for5.c new file mode 100644 index 0000000..7075a44 --- /dev/null +++ b/gcc/testsuite/c-c++-common/cilk-plus/PS/for5.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O3 -fcilkplus" } */ + +int *a, *b; + +void foo() +{ +#pragma simd + for (int i=100; i; --i) + a[i] = b[i]; +}