From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 74989 invoked by alias); 30 Oct 2015 21:22:02 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 74975 invoked by uid 89); 30 Oct 2015 21:22:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 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; Fri, 30 Oct 2015 21:21:59 +0000 Received: from svr-orw-fem-06.mgc.mentorg.com ([147.34.97.120]) by relay1.mentorg.com with esmtp id 1ZsH7T-0005gv-N8 from Cesar_Philippidis@mentor.com ; Fri, 30 Oct 2015 14:21:55 -0700 Received: from [127.0.0.1] (147.34.91.1) by SVR-ORW-FEM-06.mgc.mentorg.com (147.34.97.120) with Microsoft SMTP Server id 14.3.224.2; Fri, 30 Oct 2015 14:21:55 -0700 Subject: Re: [OpenACC] num_gangs, num_workers and vector_length in c++ To: Jakub Jelinek References: <5632A573.80002@codesourcery.com> <20151030133753.GA478@tucnak.redhat.com> <563381DF.7030005@codesourcery.com> <20151030170555.GE478@tucnak.redhat.com> CC: Jason Merrill , "gcc-patches@gcc.gnu.org" , Nathan Sidwell From: Cesar Philippidis Message-ID: <5633DF72.4070006@codesourcery.com> Date: Fri, 30 Oct 2015 21:28:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <20151030170555.GE478@tucnak.redhat.com> Content-Type: multipart/mixed; boundary="------------080603070503000705080504" X-SW-Source: 2015-10/txt/msg03456.txt.bz2 --------------080603070503000705080504 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Content-length: 1841 On 10/30/2015 10:05 AM, Jakub Jelinek wrote: > On Fri, Oct 30, 2015 at 07:42:39AM -0700, Cesar Philippidis wrote: >>> Another thing is what Jason as C++ maintainer wants, it is nice to get rid >>> of some code redundancies, on the other side the fact that there is one >>> function per non-terminal in the grammar is also quite nice property. >>> I know I've violated this a few times too. > >> That name had some legacy from the c FE in gomp-4_0-branch which the >> function was inherited from. On one hand, it doesn't make sense to allow >> negative integer values for those clauses, but at the same time, those >> values aren't checked during scanning. Maybe it should be renamed >> cp_parser_oacc_single_int_clause instead? > > That is better. > >> If you like, I could make a more general >> cp_parser_omp_generic_expression that has a scan_list argument so that >> it can be used for both general expressions and assignment-expressions. >> That way it can be used for both omp and oacc clauses of the form 'foo ( >> expression )'. > > No, that will only confuse readers of the parser. After all, the code to > parse an expression argument of a clause is not that large. > So, either cp_parser_oacc_single_int_clause or just keeping the old separate > parsing functions, just remove the cruft from those (testing the type, > using cp_parser_condition instead of cp_parser_assignment_expression) is ok > with me. Please ping Jason on what he prefers from those two. Jason, what's your preference here? Should I create a single function to parser num_gangs, num_workers and vector_length since they all accept the same type of argument or should I just correct the existing functions as I did in the attached patch? Either one would be specific to openacc. This patch has been bootstrapped and regression tested on trunk. Cesar --------------080603070503000705080504 Content-Type: text/x-patch; name="cxx-ng-nw-vl.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="cxx-ng-nw-vl.diff" Content-length: 4971 2015-10-30 Cesar Philippidis gcc/cp/ * parser.c (cp_parser_oacc_clause_vector_length): Parse the clause argument as an assignment expression. Bail out early on error. (cp_parser_omp_clause_num_gangs): Likewise. (cp_parser_omp_clause_num_workers): Likewise. diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index c8f8b3d..a0d3f3b 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -29732,37 +29732,29 @@ cp_parser_oacc_shape_clause (cp_parser *parser, omp_clause_code kind, static tree cp_parser_oacc_clause_vector_length (cp_parser *parser, tree list) { - tree t, c; - location_t location = cp_lexer_peek_token (parser->lexer)->location; - bool error = false; + location_t loc = cp_lexer_peek_token (parser->lexer)->location; if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN)) return list; - t = cp_parser_condition (parser); - if (t == error_mark_node || !INTEGRAL_TYPE_P (TREE_TYPE (t))) - { - error_at (location, "expected positive integer expression"); - error = true; - } + tree t = cp_parser_assignment_expression (parser, NULL, false, false); - if (error || !cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)) + if (t == error_mark_node + || !cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)) { cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, - /*or_comma=*/false, - /*consume_paren=*/true); + /*or_comma=*/false, + /*consume_paren=*/true); return list; } check_no_duplicate_clause (list, OMP_CLAUSE_VECTOR_LENGTH, "vector_length", - location); + loc); - c = build_omp_clause (location, OMP_CLAUSE_VECTOR_LENGTH); - OMP_CLAUSE_VECTOR_LENGTH_EXPR (c) = t; + tree c = build_omp_clause (loc, OMP_CLAUSE_VECTOR_LENGTH); + OMP_CLAUSE_OPERAND (c, 0) = t; OMP_CLAUSE_CHAIN (c) = list; - list = c; - - return list; + return c; } /* OpenACC 2.0 @@ -30149,34 +30141,28 @@ cp_parser_omp_clause_nowait (cp_parser * /*parser*/, static tree cp_parser_omp_clause_num_gangs (cp_parser *parser, tree list) { - tree t, c; - location_t location = cp_lexer_peek_token (parser->lexer)->location; + location_t loc = cp_lexer_peek_token (parser->lexer)->location; if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN)) return list; - t = cp_parser_condition (parser); + tree t = cp_parser_assignment_expression (parser, NULL, false, false); if (t == error_mark_node || !cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)) - cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, - /*or_comma=*/false, - /*consume_paren=*/true); - - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) { - error_at (location, "expected positive integer expression"); + cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, + /*or_comma=*/false, + /*consume_paren=*/true); return list; } - check_no_duplicate_clause (list, OMP_CLAUSE_NUM_GANGS, "num_gangs", location); + check_no_duplicate_clause (list, OMP_CLAUSE_NUM_GANGS, "num_gangs", loc); - c = build_omp_clause (location, OMP_CLAUSE_NUM_GANGS); - OMP_CLAUSE_NUM_GANGS_EXPR (c) = t; + tree c = build_omp_clause (loc, OMP_CLAUSE_NUM_GANGS); + OMP_CLAUSE_OPERAND (c, 0) = t; OMP_CLAUSE_CHAIN (c) = list; - list = c; - - return list; + return c; } /* OpenMP 2.5: @@ -30393,35 +30379,28 @@ cp_parser_omp_clause_defaultmap (cp_parser *parser, tree list, static tree cp_parser_omp_clause_num_workers (cp_parser *parser, tree list) { - tree t, c; - location_t location = cp_lexer_peek_token (parser->lexer)->location; + location_t loc = cp_lexer_peek_token (parser->lexer)->location; if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN)) return list; - t = cp_parser_condition (parser); + tree t = cp_parser_assignment_expression (parser, NULL, false, false); if (t == error_mark_node || !cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)) - cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, - /*or_comma=*/false, - /*consume_paren=*/true); - - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) { - error_at (location, "expected positive integer expression"); + cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, + /*or_comma=*/false, + /*consume_paren=*/true); return list; } - check_no_duplicate_clause (list, OMP_CLAUSE_NUM_WORKERS, "num_gangs", - location); + check_no_duplicate_clause (list, OMP_CLAUSE_NUM_WORKERS, "num_workers", loc); - c = build_omp_clause (location, OMP_CLAUSE_NUM_WORKERS); - OMP_CLAUSE_NUM_WORKERS_EXPR (c) = t; + tree c = build_omp_clause (loc, OMP_CLAUSE_NUM_WORKERS); + OMP_CLAUSE_OPERAND (c, 0) = t; OMP_CLAUSE_CHAIN (c) = list; - list = c; - - return list; + return c; } /* OpenMP 2.5: --------------080603070503000705080504--