This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: OpenACC wait clause
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Cesar Philippidis <cesar at codesourcery dot com>
- Cc: gcc-patches at gcc dot gnu dot org, Fortran List <fortran at gcc dot gnu dot org>, Thomas Schwinge <thomas_schwinge at mentor dot com>
- Date: Tue, 28 Jun 2016 09:08:22 +0200
- Subject: Re: OpenACC wait clause
- Authentication-results: sourceware.org; auth=none
- References: <20160607111307 dot GQ7387 at tucnak dot redhat dot com> <5756E1B6 dot 605 at codesourcery dot com> <20160607150252 dot GS7387 at tucnak dot redhat dot com> <57636CF5 dot 40400 at codesourcery dot com> <20160617143438 dot GB7387 at tucnak dot redhat dot com> <576D54F9 dot 9030008 at codesourcery dot com> <20160624155322 dot GD7387 at tucnak dot redhat dot com> <5771722A dot 6050404 at codesourcery dot com> <20160627192332 dot GT7387 at tucnak dot redhat dot com> <5771B519 dot 90106 at codesourcery dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Mon, Jun 27, 2016 at 04:22:01PM -0700, Cesar Philippidis wrote:
> + while (ret == MATCH_YES)
> {
> - if (cp->gang_static)
> - return MATCH_ERROR;
> + if (gfc_match (" static :") == MATCH_YES)
> + {
> + if (cp->gang_static)
> + return MATCH_ERROR;
It might be useful to gfc_error before this (i.e. follow a rule,
if you return MATCH_ERROR where MATCH_ERROR has not been returned before,
you emit gfc_error, if it has been returned from nested calls, you don't),
but it is not a big deal, worse case you get the cryptic generic error.
Let's keep it as is for now.
> + /* The 'num' arugment is optional. */
Typo, argument.
> + if (gfc_match (" num :") == MATCH_ERROR)
> + return MATCH_ERROR;
This is unnecessary (I mean the == MATCH_ERROR check), gfc_match for
no % operands in it will never return MATCH_ERROR (why would it?),
just MATCH_YES or MATCH_NO.
> + else if (gwv == GOMP_DIM_VECTOR)
> + {
> + /* The 'length' arugment is optional. */
See above.
> + if (gfc_match (" length :") == MATCH_ERROR)
And once more.
> @@ -1275,8 +1308,8 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, uint64_t mask,
> continue;
> if ((mask & OMP_CLAUSE_TILE)
> && !c->tile_list
> - && match_oacc_expr_list ("tile (", &c->tile_list,
> - true) == MATCH_YES)
> + && match_oacc_expr_list ("tile (", &c->tile_list, true)
> + == MATCH_YES)
Why this hunk? I think it is better to put the line break before the last
argument here, you don't have to decide if it shouldn't be surrounded by
&& (match_oacc... (...)
== MATCH_YES))
Otherwise LGTM.
Jakub