On February 10, 2025, Sandra Loosemore wrote:
[…]
When not using the variant function, having it limited to the TU
means that that there are now warnings like:
warning: ‘f.ompvariant1’ defined but not used [-Wunused-function]
4 | int f(int i) {
| ^
I think that's okay - both in terms of the name (albeit 'f' would be
nicer) and printing this -Wall warning.
Contrary to C++, I don't see a possibility how the function can be
accessed from other translation units.
As mentioned before, the following error is bogus as there is no
requirement to have the base function defined or declared before
the variant function (or at all, or to be available in any TU):
foo.c: In function ‘my.ompvariant1’:
foo.c:4:7: error: no previous declaration of base function
4 | int my(int i) { return 2; }
| ^~
That's something that should be fixed.
* * *
The following is accepted, but feels odd:
int my(int i) {
#pragma omp begin declare variant match(user={condition(1)})
return 3;
#pragma omp end declare variant
return 2;
}
GCC OG14 accepts this code, Clang-20 rejects it; there is
elision involved.
OpenMP 6 has in "9.6.5 begin declare_variant Directive" under
"Semantics": "The delimited code region is a declaration sequence."
and defines the latter:
"declaration sequence – For C/C++, a sequence of base language
declarations, including definitions, that appear in the same
scope. The sequence may include other directives that are
associated with the declarations."
Which sounds as if it invalid as 'return' is not a
(type, variable, function, ...) declaration.
* * *
When compiling, e.g.
int my(int);
#pragma omp begin declare variant match(user={condition(1)})
int my(void) {
return 1;
}
#pragma omp end declare variant
the following (correct) diagnostic is shown:
foo.c:4:5: error: variant function definition does not match previous
declaration of ‘my’
4 | int my(void) {
| ^~
However, I think that after
if (!comptypes (TREE_TYPE (decl), TREE_TYPE (base_decl)))
{
error_at (DECL_SOURCE_LOCATION (decl),
"variant function definition does not match previous "
"declaration of %qE", base_decl);
the following should be added:
inform (DECL_SOURCE_LOCATION (base_decl), "declared here");
* * *
+ #pragma omp begin declare variant (match context-selector) new-line */
I think that should be "match(context-selector)" not "(match
context-selector)"?!?
/* OpenMP 4.0
#pragma omp end declare target
OpenMP 5.1
- #pragma omp end assumes */
+ #pragma omp end assumes
+ #pragma omp end declare variant new-line */
For consistency, I would drop the 'new-line' here, even though it is
required (for all).
* * *
+ if (a.attr_syntax)
+ error_at (loc,
+ "%<begin declare variant%> in attribute syntax "
+ "terminated with "
+ "%<end declare variant%> in pragma syntax");
+ else
+ error_at (loc,
+ "%<begin declare variant%> in pragma syntax "
+ "terminated with "
+ "%<end declare variant%> in attribute syntax");
+ }
I noticed that there is not a single attribute test case for neither C23
nor C++ >= 11.
See testsuite/gcc.dg/gomp/attrs-18.c, testsuite/g++.dg/gomp/attrs-18.C,
and testsuite/g++.dg/gomp/attrs-9.C for 'begin declare target' examples.
Thanks for the patch, which otherwise looks good.
Tobias
