From patchwork Tue Nov 29 12:27:04 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chung-Lin Tang X-Patchwork-Id: 84791 Delivered-To: patch@linaro.org Received: by 10.140.20.101 with SMTP id 92csp1572102qgi; Tue, 29 Nov 2016 04:27:53 -0800 (PST) X-Received: by 10.98.82.65 with SMTP id g62mr27339980pfb.119.1480422473141; Tue, 29 Nov 2016 04:27:53 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id x61si31124255plb.328.2016.11.29.04.27.52 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 29 Nov 2016 04:27:53 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-442887-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-442887-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-442887-patch=linaro.org@gcc.gnu.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=E8la0wcr6yPSF0UJo QPnlGbbZ6uFoXINZe0Nc2dxRqrmrcJBuGw56B/V1Q3CREp+H9WsjM6bqqbypVYPB 3P+SS5Aqw5gQaHaW6qmJo18E7uroVLYZOSayqGWBdx9VOPixxSQm0KbUCtyxBDpW nZahSy6TxFsXImZbIE3oftAAMM= 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 :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=cxUvDjd3pLc8HkPOLQvn8ym X8d8=; b=p9Ey6DRjcYrRnf2JTXFWgBQwifaWnMhWpbqLQf7dMt5AQEPHA8xHa13 AjHw7aFEFYeD6nVgUgHFn0QdI5DU8OzdoRlm5oN2ESdoEtCi9efcQl5M1nGf5qVF 5c0H0Fxwz0f3Z/DM0kWmEfp0RJc7IfiU14pW8J5D1JautmAhvf1g= Received: (qmail 86936 invoked by alias); 29 Nov 2016 12:27:19 -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 86871 invoked by uid 89); 29 Nov 2016 12:27:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.1 required=5.0 tests=AWL, BAYES_00, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=no version=3.3.2 spammy=Collapse, mutually, statements X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 29 Nov 2016 12:27:15 +0000 Received: from svr-orw-mbx-04.mgc.mentorg.com ([147.34.90.204]) by relay1.mentorg.com with esmtp id 1cBhVC-0007Ay-93 from ChungLin_Tang@mentor.com ; Tue, 29 Nov 2016 04:27:14 -0800 Received: from svr-orw-mbx-02.mgc.mentorg.com (147.34.90.202) by SVR-ORW-MBX-04.mgc.mentorg.com (147.34.90.204) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Tue, 29 Nov 2016 04:27:11 -0800 Received: from [0.0.0.0] (147.34.91.1) by svr-orw-mbx-02.mgc.mentorg.com (147.34.90.202) with Microsoft SMTP Server (TLS) id 15.0.1210.3 via Frontend Transport; Tue, 29 Nov 2016 04:27:08 -0800 Subject: Re: [Patch 3/5] OpenACC tile clause support, C/C++ front-end parts To: Jakub Jelinek References: <61e744b3-3ae8-1709-df7b-52d65bbfb676@mentor.com> <20161111103038.GX3541@tucnak.redhat.com> <56973ea6-3f8c-0035-8443-a14aa7b88437@mentor.com> <20161118112303.GV3541@tucnak.redhat.com> CC: gcc-patches , Nathan Sidwell , Cesar Philippidis From: Chung-Lin Tang Message-ID: Date: Tue, 29 Nov 2016 20:27:04 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Thunderbird/45.5.0 MIME-Version: 1.0 In-Reply-To: <20161118112303.GV3541@tucnak.redhat.com> On 2016/11/18 7:23 PM, Jakub Jelinek wrote: > On Thu, Nov 17, 2016 at 05:34:34PM +0800, Chung-Lin Tang wrote: >> Updated C/C++ front-end patches, adjusted as reviewed. > > Jason is right, finish_omp_clauses will verify the tile operands > when !processing_template_decl are non-negative host INTEGER_CSTs, > so can't you just tsubst it like OMP_CLAUSE_COLLAPSE? If the operand > is not a constant expression, presumably it will not be INTEGER_CST. Yeah, it appears that way will work. Updated C/C++ FE patch as attached. > On the other side, OMP_CLAUSE_TILE has now 3 operands instead of just 1, > don't you need to do something during instantiation for the other 2 > operands? > > Jakub The other two operands are used only in omp-low, they're not programmer defined operands. Thanks, Chung-Lin Index: c/c-parser.c =================================================================== --- c/c-parser.c (revision 241809) +++ c/c-parser.c (working copy) @@ -11010,6 +11010,7 @@ c_parser_omp_clause_collapse (c_parser *parser, tr location_t loc; check_no_duplicate_clause (list, OMP_CLAUSE_COLLAPSE, "collapse"); + check_no_duplicate_clause (list, OMP_CLAUSE_TILE, "tile"); loc = c_parser_peek_token (parser)->location; if (c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) @@ -11920,10 +11921,11 @@ static tree c_parser_oacc_clause_tile (c_parser *parser, tree list) { tree c, expr = error_mark_node; - location_t loc, expr_loc; + location_t loc; tree tile = NULL_TREE; check_no_duplicate_clause (list, OMP_CLAUSE_TILE, "tile"); + check_no_duplicate_clause (list, OMP_CLAUSE_COLLAPSE, "collapse"); loc = c_parser_peek_token (parser)->location; if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) @@ -11931,16 +11933,19 @@ c_parser_oacc_clause_tile (c_parser *parser, tree do { + if (tile && !c_parser_require (parser, CPP_COMMA, "expected %<,%>")) + return list; + if (c_parser_next_token_is (parser, CPP_MULT) && (c_parser_peek_2nd_token (parser)->type == CPP_COMMA || c_parser_peek_2nd_token (parser)->type == CPP_CLOSE_PAREN)) { c_parser_consume_token (parser); - expr = integer_minus_one_node; + expr = integer_zero_node; } else { - expr_loc = c_parser_peek_token (parser)->location; + location_t expr_loc = c_parser_peek_token (parser)->location; c_expr cexpr = c_parser_expr_no_commas (parser, NULL); cexpr = convert_lvalue_to_rvalue (expr_loc, cexpr, false, true); expr = cexpr.value; @@ -11952,28 +11957,19 @@ c_parser_oacc_clause_tile (c_parser *parser, tree return list; } - if (!INTEGRAL_TYPE_P (TREE_TYPE (expr))) - { - c_parser_error (parser, "% value must be integral"); - return list; - } - expr = c_fully_fold (expr, false, NULL); - /* Attempt to statically determine when expr isn't positive. */ - c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, expr, - build_int_cst (TREE_TYPE (expr), 0)); - protected_set_expr_location (c, expr_loc); - if (c == boolean_true_node) + if (!INTEGRAL_TYPE_P (TREE_TYPE (expr)) + || !tree_fits_shwi_p (expr) + || tree_to_shwi (expr) <= 0) { - warning_at (expr_loc, 0,"% value must be positive"); - expr = integer_one_node; + error_at (expr_loc, "% argument needs positive" + " integral constant"); + expr = integer_zero_node; } } tile = tree_cons (NULL_TREE, expr, tile); - if (c_parser_next_token_is (parser, CPP_COMMA)) - c_parser_consume_token (parser); } while (c_parser_next_token_is_not (parser, CPP_CLOSE_PAREN)); @@ -14899,11 +14895,17 @@ c_parser_omp_for_loop (location_t loc, c_parser *p bool fail = false, open_brace_parsed = false; int i, collapse = 1, ordered = 0, count, nbraces = 0; location_t for_loc; + bool tiling = false; vec *for_block = make_tree_vector (); for (cl = clauses; cl; cl = OMP_CLAUSE_CHAIN (cl)) if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_COLLAPSE) collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (cl)); + else if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_TILE) + { + tiling = true; + collapse = list_length (OMP_CLAUSE_TILE_LIST (cl)); + } else if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_ORDERED && OMP_CLAUSE_ORDERED_EXPR (cl)) { @@ -14933,7 +14935,7 @@ c_parser_omp_for_loop (location_t loc, c_parser *p pc = &OMP_CLAUSE_CHAIN (*pc); } - gcc_assert (collapse >= 1 && ordered >= 0); + gcc_assert (tiling || (collapse >= 1 && ordered >= 0)); count = ordered ? ordered : collapse; declv = make_tree_vec (count); Index: cp/pt.c =================================================================== --- cp/pt.c (revision 241809) +++ cp/pt.c (working copy) @@ -14742,6 +14742,7 @@ tsubst_omp_clauses (tree clauses, enum c_omp_regio = tsubst_omp_clause_decl (OMP_CLAUSE_DECL (oc), args, complain, in_decl); break; + case OMP_CLAUSE_TILE: case OMP_CLAUSE_IF: case OMP_CLAUSE_NUM_THREADS: case OMP_CLAUSE_SCHEDULE: @@ -14836,19 +14837,6 @@ tsubst_omp_clauses (tree clauses, enum c_omp_regio case OMP_CLAUSE_AUTO: case OMP_CLAUSE_SEQ: break; - case OMP_CLAUSE_TILE: - { - tree lnc, loc; - for (lnc = OMP_CLAUSE_TILE_LIST (nc), - loc = OMP_CLAUSE_TILE_LIST (oc); - loc; - loc = TREE_CHAIN (loc), lnc = TREE_CHAIN (lnc)) - { - TREE_VALUE (lnc) = tsubst_expr (TREE_VALUE (loc), args, - complain, in_decl, false); - } - } - break; default: gcc_unreachable (); } Index: cp/parser.c =================================================================== --- cp/parser.c (revision 241809) +++ cp/parser.c (working copy) @@ -30885,30 +30885,33 @@ cp_parser_oacc_clause_tile (cp_parser *parser, loc tree c, expr = error_mark_node; tree tile = NULL_TREE; + /* Collapse and tile are mutually exclusive. (The spec doesn't say + so, but the spec authors never considered such a case and have + differing opinions on what it might mean, including 'not + allowed'.) */ check_no_duplicate_clause (list, OMP_CLAUSE_TILE, "tile", clause_loc); + check_no_duplicate_clause (list, OMP_CLAUSE_COLLAPSE, "collapse", + clause_loc); if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN)) return list; do { + if (tile && !cp_parser_require (parser, CPP_COMMA, RT_COMMA)) + return list; + if (cp_lexer_next_token_is (parser->lexer, CPP_MULT) && (cp_lexer_nth_token_is (parser->lexer, 2, CPP_COMMA) || cp_lexer_nth_token_is (parser->lexer, 2, CPP_CLOSE_PAREN))) { cp_lexer_consume_token (parser->lexer); - expr = integer_minus_one_node; + expr = integer_zero_node; } else - expr = cp_parser_assignment_expression (parser, NULL, false, false); + expr = cp_parser_constant_expression (parser); - if (expr == error_mark_node) - return list; - tile = tree_cons (NULL_TREE, expr, tile); - - if (cp_lexer_next_token_is (parser->lexer, CPP_COMMA)) - cp_lexer_consume_token (parser->lexer); } while (cp_lexer_next_token_is_not (parser->lexer, CPP_CLOSE_PAREN)); @@ -31021,6 +31024,7 @@ cp_parser_omp_clause_collapse (cp_parser *parser, } check_no_duplicate_clause (list, OMP_CLAUSE_COLLAPSE, "collapse", location); + check_no_duplicate_clause (list, OMP_CLAUSE_TILE, "tile", location); c = build_omp_clause (loc, OMP_CLAUSE_COLLAPSE); OMP_CLAUSE_CHAIN (c) = list; OMP_CLAUSE_COLLAPSE_EXPR (c) = num; @@ -34027,10 +34031,16 @@ cp_parser_omp_for_loop (cp_parser *parser, enum tr int i, collapse = 1, ordered = 0, count, nbraces = 0; vec *for_block = make_tree_vector (); auto_vec orig_inits; + bool tiling = false; for (cl = clauses; cl; cl = OMP_CLAUSE_CHAIN (cl)) if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_COLLAPSE) collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (cl)); + else if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_TILE) + { + tiling = true; + collapse = list_length (OMP_CLAUSE_TILE_LIST (cl)); + } else if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_ORDERED && OMP_CLAUSE_ORDERED_EXPR (cl)) { @@ -34060,7 +34070,7 @@ cp_parser_omp_for_loop (cp_parser *parser, enum tr pc = &OMP_CLAUSE_CHAIN (*pc); } - gcc_assert (collapse >= 1 && ordered >= 0); + gcc_assert (tiling || (collapse >= 1 && ordered >= 0)); count = ordered ? ordered : collapse; declv = make_tree_vec (count); @@ -34079,13 +34089,15 @@ cp_parser_omp_for_loop (cp_parser *parser, enum tr if (code != CILK_FOR && !cp_lexer_next_token_is_keyword (parser->lexer, RID_FOR)) { - cp_parser_error (parser, "for statement expected"); + if (!collapse_err) + cp_parser_error (parser, "for statement expected"); return NULL; } if (code == CILK_FOR && !cp_lexer_next_token_is_keyword (parser->lexer, RID_CILK_FOR)) { - cp_parser_error (parser, "_Cilk_for statement expected"); + if (!collapse_err) + cp_parser_error (parser, "_Cilk_for statement expected"); return NULL; } loc = cp_lexer_consume_token (parser->lexer)->location; @@ -34245,7 +34257,7 @@ cp_parser_omp_for_loop (cp_parser *parser, enum tr nested. Hopefully the final version clarifies this. For now handle (multiple) {'s and empty statements. */ cp_parser_parse_tentatively (parser); - do + for (;;) { if (cp_lexer_next_token_is_keyword (parser->lexer, RID_FOR)) break; @@ -34260,14 +34272,13 @@ cp_parser_omp_for_loop (cp_parser *parser, enum tr else { loc = cp_lexer_peek_token (parser->lexer)->location; - error_at (loc, "not enough collapsed for loops"); + error_at (loc, "not enough for loops to collapse"); collapse_err = true; cp_parser_abort_tentative_parse (parser); declv = NULL_TREE; break; } } - while (1); if (declv) { Index: cp/semantics.c =================================================================== --- cp/semantics.c (revision 241809) +++ cp/semantics.c (working copy) @@ -7086,7 +7086,8 @@ finish_omp_clauses (tree clauses, enum c_omp_regio else if (!type_dependent_expression_p (t) && !INTEGRAL_TYPE_P (TREE_TYPE (t))) { - error ("% value must be integral"); + error_at (OMP_CLAUSE_LOCATION (c), + "% argument needs integral type"); remove = true; } else @@ -7094,14 +7095,16 @@ finish_omp_clauses (tree clauses, enum c_omp_regio t = mark_rvalue_use (t); if (!processing_template_decl) { + /* Zero is used to indicate '*', we permit you + to get there via an ICE of value zero. */ t = maybe_constant_value (t); - if (TREE_CODE (t) == INTEGER_CST - && tree_int_cst_sgn (t) != 1 - && t != integer_minus_one_node) + if (!tree_fits_shwi_p (t) + || tree_to_shwi (t) < 0) { - warning_at (OMP_CLAUSE_LOCATION (c), 0, - "% value must be positive"); - t = integer_one_node; + error_at (OMP_CLAUSE_LOCATION (c), + "% argument needs positive " + "integral constant"); + remove = true; } } t = fold_build_cleanup_point_expr (TREE_TYPE (t), t); @@ -8000,11 +8003,19 @@ finish_omp_for (location_t locus, enum tree_code c gcc_assert (TREE_VEC_LENGTH (declv) == TREE_VEC_LENGTH (incrv)); if (TREE_VEC_LENGTH (declv) > 1) { - tree c = find_omp_clause (clauses, OMP_CLAUSE_COLLAPSE); + tree c; + + c = find_omp_clause (clauses, OMP_CLAUSE_TILE); if (c) - collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (c)); - if (collapse != TREE_VEC_LENGTH (declv)) - ordered = TREE_VEC_LENGTH (declv); + collapse = list_length (OMP_CLAUSE_TILE_LIST (c)); + else + { + c = find_omp_clause (clauses, OMP_CLAUSE_COLLAPSE); + if (c) + collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (c)); + if (collapse != TREE_VEC_LENGTH (declv)) + ordered = TREE_VEC_LENGTH (declv); + } } for (i = 0; i < TREE_VEC_LENGTH (declv); i++) {