Re: [PATCH] OpenACC: Further attach/detach clause fixes for Fortran [PR109622]

[Tobias Burnus <tobias@codesourcery.com>]

On 29.04.23 12:57, Julian Brown wrote:
> This patch moves several tests introduced by the following patch:
>
>    https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616939.html

I believe you intent this as git log entry. Can you add
   ... commit r14-325-gcacf65d74463600815773255e8b82b4043432bd7
as this makes looking at the git history easier.

> into the proper location for OpenACC testing (thanks to Thomas for
> spotting my mistake!), and also fixes a few additional problems --
> missing diagnostics for non-pointer attaches, and a case where a pointer
> was incorrectly dereferenced. Tests are also adjusted for vector-length
> warnings on nvidia accelerators.
>
> Tested with offloading to nvptx. OK?
>
> 2023-04-29  Julian Brown  <julian@codesourcery.com>
>
>       PR fortran/109622
>
> gcc/fortran/
>       * trans-openmp.cc (gfc_trans_omp_clauses): Add diagnostic for
>       non-pointer/non-allocatable attach/detach.  Remove dereference for
>       pointer-to-scalar derived type component attach/detach.

In general, we prefer resolution-time diagnostic to tree-translation diagnostic,
unless there is a good reason to do the latter.
At a glance, it should be even sufficient to have a single diagnostic
instead of two when placed into openmp.cc.

Search for lastref in resolve_omp_clauses; I think it should do,
but otherwise something like:
  symbol_attr attr = gfc_expr_attr(e);
  if (attr.pointer || attr.allocatable)
should work.

You currently have:

> @@ -3430,6 +3432,13 @@ gfc_trans_omp_clauses (stmtblock_t *block, gfc_omp_clauses *clauses,
>                           = TYPE_SIZE_UNIT (gfc_charlen_type_node);
>                       }
>                   }
> +               else if (openacc
> +                        && (n->u.map_op == OMP_MAP_ATTACH
> +                            || n->u.map_op == OMP_MAP_DETACH))
> +                 gfc_error ("%qs clause argument not pointer or "
> +                            "allocatable at %L",
> +                            (n->u.map_op == OMP_MAP_ATTACH)
> +                            ? "attach" : "detach", &where);

Additionally, I think we we usually have wording like: 'must be ALLOCATABLE or a POINTER'.
(Which avoids also the question whether 'neither' instead of 'not should be used
and/besides to 'nor' instead of 'or'.)

Additionally, I think there should be a also an error for:

integer :: a
!$acc enter data attach(a)
end

(Well, in that case, just looking at n->expr won't work, but also n->sym needs to be
handled for list == OMP_LIST_MAP && n->u.map_op == OMP_MAP_(DE)ATTACH.

The other changes look fine. Thanks!

Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955