Gio T wrote:
This patch implements the C and C++ OpenMP 5.2 deprecations
Generic remark - which I forgot to mention in my generic comment in 1/4:
In the OpenMP specification, all 5.x deprecations have been removed
in the OpenMP 6.0 specification. Obviously, compilers try to be backward
compatible - and, hence, it is expected that all deprecated features will
remain accepted by GCC for a long time.
* * *
Subject: [PATCH] [PATCH 2/4] openmp: Bump Version from 4.5 to 5.2
I think it is okay and makes sense to have four patches/commits,
but I am not really happy about the subject line (as then
visible by 'git log --oneline'):
[PATCH 1/4] openmp: Bump Version from 4.5 to 5.2
PATCH [2/4] openmp: Bump Version from 4.5 to 5.2
[PATCH 3/4] openmp: Bump Version from 4.5 to 5.2
[PATCH 3/4] openmp: Bump Version from 4.5 to 5.2
I think it would be better to use:
openmp: Bump Version from 4.5 to 5.2 (1/4)
openmp: Bump Version from 4.5 to 5.2 (2/4)
etc.
Alternatively, e.g.
* openmp: Bump Version from 4.5 to 5.2 and 5.1 deprecations
* openmp: Bump Version from 4.5 to 5.2 - 5.2 deprecations in C/C++
* openmp: Bump Version from 4.5 to 5.2 - 5.2 deprecations in Fortran
* openmp: Bump Version from 4.5 to 5.2 - documentation update
Or something like that.
[I probably would go this time for '(1/4)' also
to match the subject line of the submitted emails.]
* * *
Implements the OpenMP 5.2 C and C++ deprecations.
Generic background info to thread readers:
The 5.2 deprecations are mostly because of conversion to a
generator and splitting off the clauses from the directives
to their own sections - and then realizing that, e.g.
declare target to(...)
and
target update to(...)
have only a common theme but their semantic is quite different.
(Hence, the former takes now an 'enter' clause.)
* * *
Uses the warning
established in patch 1/4, -Wdeprecated-openmp, for said deprecations.
Not implemented is 'uses_allocators', since the base is not yet in
mainline, along with the relaxing of constraints for the interop
construct, since this is not a deprecation.
To expand on this remark:
The latter is "The init clause of interop construct now accepts an
interop_type in any position of the modifier list." - which is not
really a deprecation and nothing one can warn about. (interop is
implemented since GCC 15 and supports most of the 6.0 changes,
including this 5.2 one.)
Also missing is the requires context selector deprecations due to
https://gcc.gnu.org/PR113067 .
* * *
Additionally does not
deprecate 'destroy' with no arguments on depobj construct, since
this was undeprecated in OpenMP 6.0.
Backgound:
The problem was that this change required to specify the
expression twice. The argument version is still the way to go, but
skipping the argument to the directive itself. (Not yet implemented.)
However as stated, the old version is indeed undeprecated in 6.0 and
still in TR14.
* * *
gcc/c/ChangeLog:
* c-parser.cc (c_parser_omp_clause_reduction): Deprecate '-'
[...]
(c_parser_omp_clause_map): Map clause modifiers comma-separated.
Odd wording. Maybe 'Deprecate space-separated modifiers'?
(cp_parser_omp_clause_map): Map clause modifiers
comma-separated.
...
* * *
I might have missed it, but I think there is no 'dg-warning' check for
+ "%<to%> clause with %<declare target%> deprecated since "
+ "OpenMP 5.2, use %<enter%>");
nor for
+ "%<-%> operator for reductions deprecated in OpenMP 5.2");
[The semantic is OpenMP is clear, still it can be mind boggling what
it actually means. I expect real-world code to prefer '+' and handle
any required minus differently.]
* * *
@@ -28001,6 +28037,12 @@ c_parser_omp_declare_target (c_parser *parser)
}
for (tree c = clauses; c; c = OMP_CLAUSE_CHAIN (c))
{
+ if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_TO)
+ {
+ warning_at (OMP_CLAUSE_LOCATION (c), OPT_Wdeprecated_openmp,
+ "%<to%> clause with %<declare target%> deprecated since "
+ "OpenMP 5.2, use %<enter%>");
+ }
If there is only one statement inside an if or else block, GCC style is
to use it directly without adding { }.
[Multiple times]
@@ -20182,6 +20200,9 @@ c_parser_omp_clause_map (c_parser *parser, tree list,
bool declare_mapper_p)
if (tok->type == CPP_COMMA)
{
+ ++num_commas;
+ if (num_commas > num_identifiers)
+ c_parser_error (parser, "illegal comma");
c_parser_consume_token (parser);
continue;
}
I believe the '++num_identifiers;' of this and the next 5 cases can be
moved to before the 'if' in
@@ -20298,6 +20324,12 @@ c_parser_omp_clause_map (c_parser *parser, tree list,
bool declare_mapper_p)
parens.skip_until_found_close (parser);
return list;
}
+ if (num_identifiers - 1 != num_commas)
+ {
+ warning_at (clause_loc, OPT_Wdeprecated_openmp,
+ "%<map%> clause modifiers without comma separation is deprecated "
+ "since OpenMP 5.2");
+ }
}
as all if/else if/else either bumpnum_identifiers – or return early because
of some error diagnostic.
Thus, 'if (num_identifiers++ != num_commas)' should work – or
maybe++num_identifiers;
if (num_identifiers - 1 != num_commas)
is clearer?
And the { } in the if is not needed as there is only one statement in the if
block:
the warning_at.
* * *
@@ -43372,6 +43397,7 @@ cp_parser_omp_clause_map (cp_parser *parser, tree list,
bool declare_mapper_p)
const char *p = IDENTIFIER_POINTER (tok->u.value);
if (strcmp ("always", p) == 0)
{
+ ++num_identifiers;
Likewise.
* * *
LGTM - but adding the two warning checks would be good - as would
be considering the mentioned minor cleanups and subject nit.
Tobias
