On Wed, Jun 09, 2021 at 02:18:43PM +0200, Tobias Burnus wrote:
> This patch add's OpenMP 5.1's  defaultmap extensions to Fortran.
> 
> There is one odd thing,
>   integer :: ii, it
>   target :: it
> both count as nonallocatable, nonpointer scalars (i.e. category 'scalar').
> But with implicit mapping (and 'defaultmap(default)'), 'it' is mapped
> tofrom due to the TARGET attribute (cf. quote in the PR).

The list in 5.1 2.21.7 is ordered, so if defaultmap is present and is
not default, it takes precedence over TARGET attribute.
So, it above with defaultmap(firstprivate:scalar) will result in
firstprivate(it, ii), while no defaultmap or default for it will result in
map(tofrom: it) firstprivate (ii) (same for ALLOCATABLE/POINTER).

> +       switch ((enum gfc_omp_defaultmap) i)
> +         {
> +           case OMP_DFLTMAP_CAT_SCALAR: dfltmap = "SCALAR"; break;
> +           case OMP_DFLTMAP_CAT_AGGREGATE: dfltmap = "AGGREGATE"; break;
> +           case OMP_DFLTMAP_CAT_ALLOCATABLE: dfltmap = "ALLOCATABLE"; break;
> +           case OMP_DFLTMAP_CAT_POINTER: dfltmap = "POINTER"; break;
> +           default: gcc_unreachable ();

Formatting.  case/default should be indented the same as {.

> --- a/gcc/fortran/gfortran.h
> +++ b/gcc/fortran/gfortran.h
> @@ -1241,6 +1241,29 @@ enum gfc_omp_map_op
>    OMP_MAP_ALWAYS_TOFROM
>  };
>  
> +enum gfc_omp_defaultmap
> +{
> +  OMP_DFLTMAP_UNSET,
> +  OMP_DFLTMAP_ALLOC,
> +  OMP_DFLTMAP_TO,
> +  OMP_DFLTMAP_FROM,
> +  OMP_DFLTMAP_TOFROM,
> +  OMP_DFLTMAP_FIRSTPRIVATE,
> +  OMP_DFLTMAP_NONE,
> +  OMP_DFLTMAP_DEFAULT,
> +  OMP_DFLTMAP_PRESENT

Any reason not to use full OMP_DEFAULTMAP_ ?  The extra 3 chars
will improve readability I think.
> +};
> +
> +enum gfc_omp_dfltmpap_category

Was this meant to be dfltmap rather than mpap?
I think I'd prefer omp_defaultmap_category

> +{
> +  OMP_DFLTMAP_CAT_UNCATEGORIZED,
> +  OMP_DFLTMAP_CAT_SCALAR,
> +  OMP_DFLTMAP_CAT_AGGREGATE,
> +  OMP_DFLTMAP_CAT_ALLOCATABLE,
> +  OMP_DFLTMAP_CAT_POINTER,
> +  OMP_DFLTMAP_CAT_NUM

And same as above.

> +      switch(clauses->defaultmap[i])

Missing space after switch.

> diff --git a/gcc/testsuite/gfortran.dg/gomp/defaultmap-1.f90 
> b/gcc/testsuite/gfortran.dg/gomp/defaultmap-1.f90
> new file mode 100644
> index 00000000000..299d971f23c

As for the testsuite, I miss the integer, target :: it case
in the gfortran.dg/gomp/defaultmap-*.f90 tests, it is only in the runtime
case if I'm not blind.

        Jakub

Reply via email to