Re: [PATCH v6 05/11] openmp, fortran: Add support for iterators in OpenMP 'target update' constructs (Fortran)

CC fortran as it is a Fortran patch. Cf. for the overview
https://gcc.gnu.org/pipermail/gcc-patches/2026-June/719430.html
and for this patch
https://gcc.gnu.org/pipermail/gcc-patches/2025-July/689064.html

@Sandra - please fix the u3 issue before committing the 4/11 and this 5/11
patch, cf. below.

Sandra Loosemore wrote:

> This adds Fortran support for iterators in 'to' and 'from' clauses in the
> 'target update' OpenMP directive.
>
> --- a/gcc/fortran/dump-parse-tree.cc
> +++ b/gcc/fortran/dump-parse-tree.cc
> @@ -1486,7 +1486,8 @@ show_omp_namelist (int list_type, gfc_omp_namelist *n)
>       {
>         gfc_current_ns = ns_curr;
>         if (list_type == OMP_LIST_AFFINITY || list_type == OMP_LIST_DEPEND
> -	  || list_type == OMP_LIST_MAP)
> +	  || list_type == OMP_LIST_MAP
> +	  || list_type == OMP_LIST_TO || list_type == OMP_LIST_FROM)
>   	{
...
> --- a/gcc/fortran/match.cc
> +++ b/gcc/fortran/match.cc
> @@ -6342,7 +6342,8 @@ void
>   gfc_free_omp_namelist (gfc_omp_namelist *name, enum gfc_omp_list_type list)
>   {
>     bool free_ns = (list == OMP_LIST_AFFINITY || list == OMP_LIST_DEPEND
> -		  || list == OMP_LIST_MAP);
> +		  || 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);
>    bool free_mapper = (list == OMP_LIST_MAP);

This part actually clashes with 'declare mapper' for the not
merged bits. (I have a pending patch (snipped of Julian's patch set),
that I kept wanting to review this week. The problem is:

       if (free_ns)
         gfc_free_namespace (name->u2.ns);
...
       else if (free_mapper && name->u2.udm)
         free (name->u2.udm);

and gfc_omp_namelist's

   union
     {
       struct gfc_omp_namelist_udr *udr;
       struct gfc_omp_namelist_udm *udm; /// <<<<<<<<<<<<<<<
       gfc_namespace *ns; /// <<<<<<<<<<<<<<<
       gfc_expr *allocator;
       struct gfc_symbol *traits_sym;
       struct gfc_omp_namelist *duplicate_of;
       char *init_interop;
     } u2;

which actually also affects patch 4/5 that added:

--- a/gcc/fortran/match.cc
+++ b/gcc/fortran/match.cc
@@ -6341,7 +6341,8 @@ gfc_free_namelist (gfc_namelist *name)
  void
  gfc_free_omp_namelist (gfc_omp_namelist *name, enum gfc_omp_list_type list)
  {
-  bool free_ns = (list == OMP_LIST_AFFINITY || list == OMP_LIST_DEPEND);
+  bool free_ns = (list == OMP_LIST_AFFINITY || list == OMP_LIST_DEPEND
+                 || list == OMP_LIST_MAP);

and that I missed there :-/

I am not sure what's the best way - by possibly to do what Kwok
did in commit (here: OG14):
   2afb6281a5a  openmp, fortran: Move udm field of gfc_omp_namelist into a new union

* * *

Thus: Could you do so? On mainline, it currently only affects:

gcc/fortran/gfortran.h:      struct gfc_omp_namelist_udm udm;
gcc/fortran/match.cc:   { } /* For now, u2.udm is not a pointer.  */
gcc/fortran/openmp.cc:                    n->u2.udm.udm = udm;

As Kwok's change reduced to those three cases is trivial,
please just commit it.

Once done, I re-base my next almost ready bit of the Fortran
declare-mapper patches.

* * *

> --- a/gcc/fortran/trans-openmp.cc
> +++ b/gcc/fortran/trans-openmp.cc
> @@ -5270,12 +5270,40 @@ gfc_trans_omp_clauses (stmtblock_t *block, gfc_omp_clauses *clauses,
>   	case OMP_LIST_TO:
>   	case OMP_LIST_FROM:
>   	case OMP_LIST_CACHE:
> +	  iterator = NULL_TREE;
> +	  prev = NULL;
> +	  prev_clauses = omp_clauses;
>   	  for (; n != NULL; n = n->next)
>   	    {
>   	      if (!n->sym->attr.referenced
>   		  && n->sym->attr.flavor != FL_PARAMETER)
>   		continue;

Just looking at the interdiff, I think the last condition (not
touched by this patch) looks wrong. I think it should be:

!n->sym->attr.referenced || n->sym->attr.flavor == FL_PARAMETER

It seems to be wrong that parameters are still processed, when they
shouldn't :-/

That's

r16-2213-g451b6dbf475959 Fortran/OpenACC: Permit PARAMETER as 'var' in clauses (+ ignore)

Side remark: There is also some more recent OpenMP wording regarding
const/constexpr which go in the same direction for mapping.

I guess it is on me to fix this ...

* * *

LGTM - after the 'u3' fix.

Thanks for the patch!

Tobias

PS: And I shall not forget to look at this condition issue ...