This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [OpenACC] num_gangs, num_workers and vector_length in c++
- From: Cesar Philippidis <cesar at codesourcery dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Jason Merrill <jason at redhat dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Nathan Sidwell <nathan at codesourcery dot com>
- Date: Fri, 30 Oct 2015 14:21:54 -0700
- Subject: Re: [OpenACC] num_gangs, num_workers and vector_length in c++
- Authentication-results: sourceware.org; auth=none
- References: <5632A573 dot 80002 at codesourcery dot com> <20151030133753 dot GA478 at tucnak dot redhat dot com> <563381DF dot 7030005 at codesourcery dot com> <20151030170555 dot GE478 at tucnak dot redhat dot com>
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
2015-10-30 Cesar Philippidis <cesar@codesourcery.com>
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: