Hi Sandra, hello world,
Sandra Loosemore wrote:
gcc/fortran/ChangeLog
PR middle-end/107067
* parse.cc (parse_omp_do): Diagnose missing "OMP END METADIRECTIVE"
after loop.
(parse_omp_structured_block): Likewise for strictly structured block.
(parse_omp_metadirective_body): Use better test for variants ending
at different places. Issue a user diagnostic at the end if any
were inconsistent, instead of calling gcc_assert.
gcc/testsuite/ChangeLog
PR middle-end/107067
* gfortran.dg/gomp/metadirective-11.f90: Remove the dg-ice, update
for current behavior, and add more tests to exercise the new error
code.
First, a generic comment/rant: The OpenMP specification is vague enough to
make it rather unclear what exactly is supposed to be valid or not as it
both acts like a processor (one version wins and is then applied) and also
not (dynamic statements etc.).
(Note that 'omp atomic' with 'capture' consists of two statements: the
capture and the atomic update, which usually there is only one
statement/construct
following a metadirective - or a delimited metadirective is required.)
For the following code,
--- a/gcc/testsuite/gfortran.dg/gomp/metadirective-11.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/metadirective-11.f90
...
+ ! The "nothing" directive in a non-begin/end metadirective only applies to a
+ ! a single statement or block, while "atomic capture" permits multiple
+ ! assignment statements.
+ !$OMP metadirective &
+ !$OMP when ( user = { condition ( UseDevice ) } &
+ !$OMP : nothing ) &
+ !$OMP default (atomic capture) ! { dg-error "Variants in a metadirective at
.1. have different associations" }
+ n = n + 1; v = n
[IMHO when glancing at the code, it is not obvious why the compiler chokes
on it. And while the error message makes sense, it is not necessarily
clear enough for a struggling.]
It is rather unclear whether that is supposed to valid or not. If one just
applies 'nothing' that's clearly valid – but on the other hand, assuming
the normal block handling, it is not.
I think choosing not to handle it and print an error is fine. Albeit, as
mentioned, I find
+ if (saw_error)
+ gfc_error_now ("Variants in a metadirective at %L have "
+ "different associations", &body_locus);
+
... not that helpful as error message and wonder whether GCC should give
a hint to the user how to solve the issue.
I could imagine adding the suggestion to the error message (to the
gfc_error_now message) - or it could be separate (cf. below).
Using BLOCK should always work - whether the OpenMP directive is
associated with a block or not. If it is associated with a block
also the delimited form of metadirectives works.
How to do it inside gfc_error_now is obvious (and preexisting in
several gfortran error message). To add the hint after the actual
error, something like the following could be used:
inform (gfc_get_location(&body_locus),
"Consider enclosing in a BLOCK construct or using the "
"BEGIN/END METADIRECTIVE construct");
* * *
To conclude: The patch LGTM but consider giving the user some hint
how to solve it, e.g. using one of the ideas above.
Thanks for the patch!
Tobias
