On Sun, Sep 08, 2024 at 09:15:23AM -0600, Sandra Loosemore wrote:
> On 8/16/24 06:58, Jakub Jelinek wrote:
> >
> > If this can apply (perhaps with small fuzz) to vanilla trunk, guess it can
> > be committed right now, doesn't need to wait for the rest of the
> > metadirective patch set.
>
> OK. I've tested the slightly cleaned-up version of the patch which is
> attached; I'll push it in the next day or two if there is no further
> objection.
>
> -Sandra
> From 23a82bea26805f2a430354d56b444d5bb7eaed3f Mon Sep 17 00:00:00 2001
> From: Sandra Loosemore <sloosem...@baylibre.com>
> Date: Fri, 6 Sep 2024 20:58:13 +0000
> Subject: [PATCH] OpenMP: Reject other properties with kind(any)
>
> TR13 (pre-6.0) of the OpenMP spec says:
>
> "If trait-property any is specified in the kind trait-selector of the
> device selector set or the target_device selector sets, no other
> trait-property may be specified in the same selector set."
>
> This restriction dates back to OpenMP 5.1 (where it had slightly
> different wording). GCC's implementation was based on the 5.0 spec, and
> several testcases include selectors that are now considered invalid.
> This patch adds a diagnostic and fixes the testcases.
>
> gcc/ChangeLog
> * omp-general.cc (omp_check_context_selector): Reject other
> properties in the same selector set with kind(any).
>
> gcc/testsuite/ChangeLog
> * c-c++-common/gomp/declare-variant-10.c: Fix broken tests.
> * c-c++-common/gomp/declare-variant-3.c: Likewise.
> * c-c++-common/gomp/declare-variant-9.c: Likewise.
> * c-c++-common/gomp/declare-variant-any.c: New.
> * gfortran.dg/gomp/declare-variant-10.f90: Fix broken tests.
> * gfortran.dg/gomp/declare-variant-3.f90: Likewise.
> * gfortran.dg/gomp/declare-variant-9.f90: Likewise.
> * gfortran.dg/gomp/declare-variant-any.f90: New.
> ---
> gcc/omp-general.cc | 35 +++++++++++++++++++
> .../c-c++-common/gomp/declare-variant-10.c | 4 +--
> .../c-c++-common/gomp/declare-variant-3.c | 10 ++----
> .../c-c++-common/gomp/declare-variant-9.c | 4 +--
> .../c-c++-common/gomp/declare-variant-any.c | 10 ++++++
> .../gfortran.dg/gomp/declare-variant-10.f90 | 4 +--
> .../gfortran.dg/gomp/declare-variant-3.f90 | 12 ++-----
> .../gfortran.dg/gomp/declare-variant-9.f90 | 2 +-
> .../gfortran.dg/gomp/declare-variant-any.f90 | 28 +++++++++++++++
> 9 files changed, 86 insertions(+), 23 deletions(-)
> create mode 100644 gcc/testsuite/c-c++-common/gomp/declare-variant-any.c
> create mode 100644 gcc/testsuite/gfortran.dg/gomp/declare-variant-any.f90
>
> diff --git a/gcc/omp-general.cc b/gcc/omp-general.cc
> index 0b61335dba4..aa53c00bec5 100644
> --- a/gcc/omp-general.cc
> +++ b/gcc/omp-general.cc
> @@ -1288,6 +1288,8 @@ omp_check_context_selector (location_t loc, tree ctx)
> for (tree tss = ctx; tss; tss = TREE_CHAIN (tss))
> {
> enum omp_tss_code tss_code = OMP_TSS_CODE (tss);
> + bool saw_any_prop = false;
> + bool saw_other_prop = false;
>
> /* We can parse this, but not handle it yet. */
> if (tss_code == OMP_TRAIT_SET_TARGET_DEVICE)
> @@ -1324,6 +1326,31 @@ omp_check_context_selector (location_t loc, tree ctx)
> else
> ts_seen[ts_code] = true;
>
> +
I think just one empty line is enough, no need for two.
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/gomp/declare-variant-any.c
> @@ -0,0 +1,10 @@
> +extern int f1 (int);
> +extern int f2 (int);
> +extern int f3 (int);
> +extern int f4 (int);
> +
> +#pragma omp declare variant (f1) match (device={kind(any,gpu)}) /* {
> dg-error "no other trait-property may be specified" } */
> +#pragma omp declare variant (f2) match (device={kind(cpu,"any")}) /* {
> dg-error "no other trait-property may be specified" } */
> +#pragma omp declare variant (f3) match (device={kind("any"),arch(x86_64)})
> /* { dg-error "no other trait-property may be specified" } */
> +#pragma omp declare variant (f4) match (device={arch(x86_64),kind(any)}) /*
> { dg-error "no other trait-property may be specified" } */
I think also testing the device={kind(any,any)} and device={kind("any",any)}
and device={kind(any,"any"))} would be useful.
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/gomp/declare-variant-any.f90
> @@ -0,0 +1,28 @@
> +integer function f1 (x)
> + integer, intent(in) :: x
> + f1 = x + 1
> +end function
> +integer function f2 (x)
> + integer, intent(in) :: x
> + f2 = x + 2
> +end function
> +integer function f3 (x)
> + integer, intent(in) :: x
> + f3 = x + 3
> +end function
> +integer function f4 (x)
> + integer, intent(in) :: x
> + f4 = x + 4
> +end function
> +
> +integer function f (x)
> + integer, intent(in) :: x
> +
> + !$omp declare variant (f1) match (device={kind(any,gpu)}) ! { dg-error
> "no other trait-property may be specified" }
> + !$omp declare variant (f2) match (device={kind(cpu,"any")}) ! { dg-error
> "no other trait-property may be specified" }
> + !$omp declare variant (f3) match (device={kind("any"),arch(x86_64)}) ! {
> dg-error "no other trait-property may be specified" }
> + !$omp declare variant (f4) match (device={arch(x86_64),kind(any)}) ! {
> dg-error "no other trait-property may be specified" }
> +
> + f = x
> +end function
> +
Here too. And, please avoid empty line at the end of test.
LGTM with those nits fixed.
Jakub
