On 8/11/25 11:03, Tobias Burnus wrote:
[snip]
As you are adding comments, I think you also need to add one for
the 'not construct={simd}'.
That those are skipped is part of commit r10-3744-g94e7f906ca5c73,
i.e. pretty old.
I bet is has something to do with the modifications done via
c_omp_declare_simd_clauses_to_numbers but I actually do not really
see a strong relatation between 'construct={simd}' and
'declare simd', unless the compiler adds something automatically
and hopefully does some argument checking elsewhere ...
* * *
In any case, it seems as we should but don't handle
adjust_args/append_args with declare simd/construct={simd} functions.
Hmmm. I need some clarification of the OpenMP spec and expected
behavior, here.
There is an existing test case c-c++-common/gomp/declare-variant-5.c
that is doing
typedef float __v4sf __attribute__((vector_size (16)));
typedef int __v4si __attribute__((vector_size (16)));
typedef float __v8sf __attribute__((vector_size (32)));
typedef int __v8si __attribute__((vector_size (32)));
__v4si f1 (__v4sf, __v4sf, float *);
__v8si f2 (__v8sf, __v8sf, float *);
__v4si f3 (__v4si, int, __v4si);
#pragma omp declare variant (f1) match
(construct={parallel,for,simd(simdlen(4),notinbranch,uniform(z),aligned(z:4
* sizeof (*z)))})
#pragma omp declare variant (f2) match
(construct={for,simd(uniform(z),simdlen(8),notinbranch)})
int f4 (float x, float y, float *z);
#pragma omp declare variant (f3) match
(construct={simd(simdlen(4),inbranch,linear(y:1))})
int f5 (int x, int y);
I can't find anything in the OpenMP spec that suggests that "declare
variant" replacement is supposed to map scalar -> vector arguments when
the construct selector includes "simd". It seems to me that the intent
is that the construct selectors are essentially assertions that the
variant function is *only* invoked in those contexts, and that it might
be a candidate for an implicit "declare simd", which would allow a
two-step optimization: first variant replacement at the call site with a
scalar function, then simd clone replacement with a vectorized version
the same way as for "declare simd". (I have to say, though, that I
don't presently have any knowledge of how simd clone replacement
actually works.)
Anyway, on mainline this example currently generates invalid calls
passing the regular scalar arguments to the variants expecting vector
parameters. Since we completely reworked the matching and variant
replacement code for GCC 15, I tried some older releases going back as
far as GCC 10 to see if it ever had previously worked, but instead it
totally failed to do any variant replacement at all.
Besides this code, also the following code in C++'s decl.cc implies
this:
if (simd)
{
TREE_VALUE (simd)
= c_omp_declare_simd_clauses_to_numbers (DECL_ARGUMENTS (decl),
OMP_TS_PROPERTIES
(simd));
/* FIXME, adjusting simd args unimplemented. */
return true;
}
I don't think c_omp_declare_simd_clauses_to_numbers is an issue, or at
least it's not the biggest issue. Unless somebody can point me at a
reference that says "declare variant" substitution maps scalar to vector
arguments or inserts additional arguments when "simd" is present in the
context selector, I think the test case mentioned above is invalid and
has never worked.
The other issues you raised are more cosmetic and I don't anticipate
problems fixing those, but it's hard for me to add a comment explaining
why the code for dealing with the "simd" selector is the way it is when
AFAICT it's just broken and always has been. :-(
-Sandra
