From: Cesar Philippidis <cesar@codesourcery.com>
To: Jakub Jelinek <jakub@redhat.com>
Cc: Jason Merrill <jason@redhat.com>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
Nathan Sidwell <nathan@codesourcery.com>
Subject: Re: [OpenACC] num_gangs, num_workers and vector_length in c++
Date: Fri, 30 Oct 2015 21:28:00 -0000 [thread overview]
Message-ID: <5633DF72.4070006@codesourcery.com> (raw)
In-Reply-To: <20151030170555.GE478@tucnak.redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1841 bytes --]
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
[-- Attachment #2: cxx-ng-nw-vl.diff --]
[-- Type: text/x-patch, Size: 4971 bytes --]
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:
next prev parent reply other threads:[~2015-10-30 21:22 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-30 0:03 Cesar Philippidis
2015-10-30 13:41 ` Jakub Jelinek
2015-10-30 14:43 ` Cesar Philippidis
2015-10-30 17:07 ` Jakub Jelinek
2015-10-30 21:28 ` Cesar Philippidis [this message]
2015-11-04 18:09 ` Jason Merrill
2015-11-04 22:10 ` Cesar Philippidis
2017-05-09 21:27 ` OpenACC C front end maintenance: c_parser_oacc_single_int_clause (was: [OpenACC] num_gangs, num_workers and vector_length in c++) Thomas Schwinge
2017-05-10 16:36 ` Jakub Jelinek
2017-05-12 9:38 ` OpenACC C front end maintenance: c_parser_oacc_single_int_clause Thomas Schwinge
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5633DF72.4070006@codesourcery.com \
--to=cesar@codesourcery.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=jason@redhat.com \
--cc=nathan@codesourcery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).