On 12/18/19 7:04 AM, Julian Brown wrote:
This part contains the Fortran front-end support for parsing the new
attach and detach clauses, as well as derived-type members on other
data-movement clauses (copyin, copyout, etc.).
I browsed the patch and it looks mostly fine to me. However, I do have
comments related to the array refs.
@@ -3890,9 +3922,6 @@ check_array_not_assumed (gfc_symbol *sym, locus loc,
const char *name)
static void
resolve_oacc_data_clauses (gfc_symbol *sym, locus loc, const char *name)
{
+ /* Disallow duplicate bare variable references and multiple
+ subarrays of the same array here, but allow multiple components of
+ the same (e.g. derived-type) variable. For the latter, duplicate
+ components are detected elsewhere. */
Do we have a test case for "the latter"?
@@ -4470,23 +4514,43 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses
*omp_clauses,
+ gfc_ref *array_ref = NULL;
+ bool resolved = false;
if (n->expr)
{
- if (!gfc_resolve_expr (n->expr)
+ array_ref = n->expr->ref;
+ resolved = gfc_resolve_expr (n->expr);
+
+ /* Look through component refs to find last array
+ reference. */
+ if (openacc)
+ while (resolved
I would move the "resolved" into the "if" condition as it doesn't change
in the while loop.
+ && array_ref
+ && (array_ref->type == REF_COMPONENT
+ || (array_ref->type == REF_ARRAY
+ && array_ref->next
+ && (array_ref->next->type
+ == REF_COMPONENT))))
+ array_ref = array_ref->next;
+ }
+ if (array_ref
+ || (n->expr
+ && (!resolved || n->expr->expr_type != EXPR_VARIABLE)))
+ {
+ if (!resolved
|| n->expr->expr_type != EXPR_VARIABLE
- || n->expr->ref == NULL
- || n->expr->ref->next
- || n->expr->ref->type != REF_ARRAY)
+ || array_ref->next
+ || array_ref->type != REF_ARRAY)
gfc_error ("%qs in %s clause at %L is not a proper "
"array section", n->sym->name, name,
&n->where);
- else if (n->expr->ref->u.ar.codimen)
+ else if (array_ref->u.ar.codimen)
gfc_error ("Coarrays not supported in %s clause at %L",
name, &n->where);
First, I believe the error message is wrong – coarrays are permitted but
only if their local data is accessed; this check checks whether a
coindex is present, i.e. whether the variable is accessed on a remote
process ("image"). Hence, the error should use something like "Entry
shall not be coindexed in %s clause at %L" or something like that.
Secondly, a coarray can exist at different places, e.g.
type t
integer :: i
end type t
type t2
integer, allocatable :: i[:]
type(t), allocatable :: x[:]
end type t2
type(t), allocatable :: A[:], B[:]
type(t) :: D[*]
type(t2) :: C
!$acc data copy(D[2]%i, A[4], B[4]%i, C%i[2], C%x[4]%i)
Here, C is not a coarray. But all those list items in the clause are
coindexed – but your new check will only detect those where the ultimate
component is coindexed. The quickest check for this is "gfc_is_coindexed
(expr)".
Thirdly, I am not sure whether the following will work with your code:
type t
integer :: i(5), j(17), k
end type t
type(t) :: x(10)
!$acc data copy (x(:)%k, x(:)%j(3))
This data is strided; I don't quickly see whether that's rejected. (I also
didn't check whether it is valid, but I think it is not.)
+/* Transparently dereference VAR if it is a pointer, reference, etc.
+ according to Fortran semantics. */
+
+tree
+gfc_auto_dereference_var (gfc_symbol *sym, tree var, bool descriptor_only_p,
+ bool is_classarray)
I have to admit that 'transparently deference' and 'auto' puzzles me,
naming/description wise, but I don't have a good solution; but I like
the 'Dereference the expression, where needed' more
+ /* Dereference the expression, where needed. */
+ se->expr = gfc_auto_dereference_var (sym, se->expr, se->descriptor_only,
+ is_classarray);
+++ b/gcc/fortran/trans-openmp.c
+ if (element)
+ {
+ gfc_conv_expr_reference (&se, n->expr);
+ gfc_add_block_to_block (block, &se.pre);
+ ptr = se.expr;
+ OMP_CLAUSE_SIZE (node)
+ = TYPE_SIZE_UNIT (TREE_TYPE (ptr));
This fits nicely on a single line; I think the ';' is in column 64!
Cheers,
Tobias
