https://gcc.gnu.org/g:3ea1f835c3775431634ffe1ad44b0c742664a3dd

commit 3ea1f835c3775431634ffe1ad44b0c742664a3dd
Author: Julian Brown <jul...@codesourcery.com>
Date:   Wed Apr 23 00:01:43 2025 +0000

    OpenMP: Look up 'declare mapper' definitions at resolution time not parse 
time
    
    This patch moves 'declare mapper' lookup for OpenMP clauses from parse
    time to resolution time for Fortran, and adds diagnostics for missing
    named mappers.  This changes clause lookup in a particular case -- where
    several 'declare mapper's are defined in a context, mappers declared
    earlier may now instantiate mappers declared later, whereas previously
    they would not.  I think the new behaviour makes more sense -- at an
    invocation site, all mappers are visible no matter the declaration order
    in some particular block.  I've adjusted tests to account for this.
    
    I think the new arrangement better matches the Fortran FE's usual way of
    doing things -- mapper lookup is a semantic concept, not a syntactical
    one, so shouldn't be handled in the syntax-handling code.
    
    The patch also fixes a case where the user explicitly writes 'default'
    as the name on the mapper modifier for a clause.
    
    2023-08-10  Julian Brown  <jul...@codesourcery.com>
    
    gcc/fortran/
            * gfortran.h (gfc_omp_namelist_udm): Add MAPPER_ID field to store 
the
            mapper name to use for lookup during resolution.
            * match.cc (gfc_free_omp_namelist): Handle OMP_LIST_TO and
            OMP_LIST_FROM when freeing mapper references.
            * module.cc (load_omp_udms, write_omp_udm): Handle MAPPER_ID field.
            * openmp.cc (gfc_match_omp_clauses): Handle explicitly-specified
            'default' name.  Don't do mapper lookup here, but record mapper 
name if
            the user specifies one.
            (resolve_omp_clauses): Do mapper lookup here instead.  Report error 
for
            missing named mapper.
    
    gcc/testsuite/
            * gfortran.dg/gomp/declare-mapper-31.f90: New test.
    
    libgomp/
            * testsuite/libgomp.fortran/declare-mapper-30.f90: New test.
            * testsuite/libgomp.fortran/declare-mapper-4.f90: Adjust test for 
new
            lookup behaviour.

Diff:
---
 gcc/fortran/gfortran.h                             |  3 ++
 gcc/fortran/match.cc                               |  4 +-
 gcc/fortran/module.cc                              |  6 +++
 gcc/fortran/openmp.cc                              | 46 ++++++++++++++++------
 .../gfortran.dg/gomp/declare-mapper-31.f90         | 34 ++++++++++++++++
 .../libgomp.fortran/declare-mapper-30.f90          | 24 +++++++++++
 .../testsuite/libgomp.fortran/declare-mapper-4.f90 | 18 +++++----
 7 files changed, 116 insertions(+), 19 deletions(-)

diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 3be828c960a7..67cf255fbd93 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -1844,6 +1844,9 @@ gfc_omp_udm;
 
 typedef struct gfc_omp_namelist_udm
 {
+  /* Used to store mapper_id before resolution.  */
+  const char *mapper_id;
+
   bool multiple_elems_p;
   struct gfc_omp_udm *udm;
 }
diff --git a/gcc/fortran/match.cc b/gcc/fortran/match.cc
index aa902f935e08..d1648fdf913c 100644
--- a/gcc/fortran/match.cc
+++ b/gcc/fortran/match.cc
@@ -5882,7 +5882,9 @@ void
 gfc_free_omp_namelist (gfc_omp_namelist *name, int list)
 {
   bool free_ns = (list == OMP_LIST_AFFINITY || list == OMP_LIST_DEPEND);
-  bool free_mapper = (list == OMP_LIST_MAP);
+  bool free_mapper = (list == OMP_LIST_MAP
+                     || list == OMP_LIST_TO
+                     || list == OMP_LIST_FROM);
   bool free_align_allocator = (list == OMP_LIST_ALLOCATE);
   bool free_mem_traits_space = (list == OMP_LIST_USES_ALLOCATORS);
   bool free_init = (list == OMP_LIST_INIT);
diff --git a/gcc/fortran/module.cc b/gcc/fortran/module.cc
index bad9b6948a2d..bf43ed59eb47 100644
--- a/gcc/fortran/module.cc
+++ b/gcc/fortran/module.cc
@@ -5562,6 +5562,11 @@ load_omp_udms (void)
          if (peek_atom () != ATOM_RPAREN)
            {
              n->u2.udm = gfc_get_omp_namelist_udm ();
+             mio_pool_string (&n->u2.udm->mapper_id);
+
+             if (n->u2.udm->mapper_id == NULL)
+               n->u2.udm->mapper_id = gfc_get_string ("%s", "");
+
              n->u2.udm->multiple_elems_p = mio_name (0, omp_map_cardinality);
              mio_pointer_ref (&n->u2.udm->udm);
            }
@@ -6638,6 +6643,7 @@ write_omp_udm (gfc_omp_udm *udm)
 
       if (n->u2.udm)
        {
+         mio_pool_string (&n->u2.udm->mapper_id);
          mio_name (n->u2.udm->multiple_elems_p, omp_map_cardinality);
          mio_pointer_ref (&n->u2.udm->udm);
        }
diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc
index bcb54512943f..ea4e75edd700 100644
--- a/gcc/fortran/openmp.cc
+++ b/gcc/fortran/openmp.cc
@@ -3645,6 +3645,8 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const 
omp_mask mask,
                      m = gfc_match (" %n ) ", mapper_id);
                      if (m != MATCH_YES)
                        goto error;
+                     if (strcmp (mapper_id, "default") == 0)
+                       mapper_id[0] = '\0';
                    }
                  else
                    break;
@@ -3719,19 +3721,11 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const 
omp_mask mask,
                  for (n = *head; n; n = n->next)
                    {
                      n->u.map.op = map_op;
-
-                     gfc_typespec *ts;
-                     if (n->expr)
-                       ts = &n->expr->ts;
-                     else
-                       ts = &n->sym->ts;
-
-                     gfc_omp_udm *udm
-                       = gfc_find_omp_udm (gfc_current_ns, mapper_id, ts);
-                     if (udm)
+                     if (mapper_id[0] != '\0')
                        {
                          n->u2.udm = gfc_get_omp_namelist_udm ();
-                         n->u2.udm->udm = udm;
+                         n->u2.udm->mapper_id
+                           = gfc_get_string ("%s", mapper_id);
                        }
                    }
                  continue;
@@ -10039,6 +10033,36 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses 
*omp_clauses,
                if (!omp_verify_map_motion_clauses (code, list, name, n,
                                                    openacc))
                  break;
+               if (list == OMP_LIST_MAP
+                   || list == OMP_LIST_TO
+                   || list == OMP_LIST_FROM)
+                 {
+                   gfc_typespec *ts;
+
+                   if (n->expr)
+                     ts = &n->expr->ts;
+                   else
+                     ts = &n->sym->ts;
+
+                   const char *mapper_id
+                     = n->u2.udm ? n->u2.udm->mapper_id : "";
+
+                   gfc_omp_udm *udm = gfc_find_omp_udm (gfc_current_ns,
+                                                        mapper_id, ts);
+                   if (mapper_id[0] != '\0' && !udm)
+                     gfc_error ("User-defined mapper %qs not found at %L",
+                                mapper_id, &n->where);
+                   else if (udm)
+                     {
+                       if (!n->u2.udm)
+                         {
+                           n->u2.udm = gfc_get_omp_namelist_udm ();
+                           gcc_assert (mapper_id[0] == '\0');
+                           n->u2.udm->mapper_id = mapper_id;
+                         }
+                       n->u2.udm->udm = udm;
+                     }
+                 }
              }
 
            if (list != OMP_LIST_DEPEND)
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-mapper-31.f90 
b/gcc/testsuite/gfortran.dg/gomp/declare-mapper-31.f90
new file mode 100644
index 000000000000..7145d51c1625
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-mapper-31.f90
@@ -0,0 +1,34 @@
+! { dg-do compile }
+
+type t
+integer :: x, y
+integer, allocatable :: arr(:)
+end type t
+
+type(t) :: var
+
+allocate(var%arr(1:20))
+
+var%arr = 0
+
+! If we ask for a named mapper that hasn't been defined, an error should be
+! raised.  This isn't a *syntax* error, so the !$omp target..!$omp end target
+! block should still be parsed correctly.
+!$omp target map(mapper(arraymapper), tofrom: var)
+! { dg-error "User-defined mapper .arraymapper. not found" "" { target *-*-* } 
.-1 }
+var%arr(5) = 5
+!$omp end target
+
+! OTOH, this is a syntax error, and the offload block is not recognized.
+!$omp target map(
+! { dg-error "Syntax error in OpenMP variable list" "" { target *-*-* } .-1 }
+var%arr(6) = 6
+!$omp end target
+! { dg-error "Unexpected !.OMP END TARGET statement" "" { target *-*-* } .-1 }
+
+! ...but not for the specific name 'default'.
+!$omp target map(mapper(default), tofrom: var)
+var%arr(5) = 5
+!$omp end target
+
+end
diff --git a/libgomp/testsuite/libgomp.fortran/declare-mapper-30.f90 
b/libgomp/testsuite/libgomp.fortran/declare-mapper-30.f90
new file mode 100644
index 000000000000..bfac28cd45cf
--- /dev/null
+++ b/libgomp/testsuite/libgomp.fortran/declare-mapper-30.f90
@@ -0,0 +1,24 @@
+! { dg-do run }
+
+type t
+integer :: x, y
+integer, allocatable :: arr(:)
+end type t
+
+!$omp declare mapper (t :: x) map(x%arr)
+
+type(t) :: var
+
+allocate(var%arr(1:20))
+
+var%arr = 0
+
+! The mapper named literally 'default' should be the default mapper, i.e.
+! the same as the unnamed mapper defined above.
+!$omp target map(mapper(default), tofrom: var)
+var%arr(5) = 5
+!$omp end target
+
+if (var%arr(5).ne.5) stop 1
+
+end
diff --git a/libgomp/testsuite/libgomp.fortran/declare-mapper-4.f90 
b/libgomp/testsuite/libgomp.fortran/declare-mapper-4.f90
index e95dbbd6f966..266845f35c7a 100644
--- a/libgomp/testsuite/libgomp.fortran/declare-mapper-4.f90
+++ b/libgomp/testsuite/libgomp.fortran/declare-mapper-4.f90
@@ -3,7 +3,7 @@
 program myprog
 type s
   integer :: c
-  integer :: d(99)
+  integer, allocatable :: d(:)
 end type s
 
 type t
@@ -16,21 +16,25 @@ end type u
 
 type(u) :: myu
 
-! Here, the mappers are declared out of order, so later ones are not 'seen' by
-! earlier ones.  Is that right?
+! Here, the mappers are declared out of order, but earlier ones can still
+! trigger mappers defined later.  Implementation-wise, this happens during
+! resolution, but from the user perspective it appears to happen at
+! instantiation time -- at which point all mappers are visible.  I think
+! that makes sense.
 !$omp declare mapper (u :: x) map(tofrom: x%myt)
 !$omp declare mapper (t :: x) map(tofrom: x%mys)
 !$omp declare mapper (s :: x) map(tofrom: x%c, x%d(1:x%c))
 
+allocate(myu%myt%mys%d(1:20))
+
 myu%myt%mys%c = 1
 myu%myt%mys%d = 0
 
 !$omp target map(tofrom: myu)
-myu%myt%mys%d(5) = myu%myt%mys%d(5) + 1
+myu%myt%mys%d(1) = myu%myt%mys%d(1) + 1
 !$omp end target
 
-! Note: we used the default mapper, not the 's' mapper, so we mapped the
-! whole array 'd'.
-if (myu%myt%mys%d(5).ne.1) stop 1
+! Note: we only mapped the first element of the array 'd'.
+if (myu%myt%mys%d(1).ne.1) stop 1
 
 end program myprog

Reply via email to