On Mon, Sep 03, 2018 at 08:46:54PM -0400, Julian Brown wrote:
> 2018-09-03 Cesar Philippidis <ce...@codesourcery.com>
>
> gcc/fortran/
> * openmp.c (gfc_match_omp_variable_list): New allow_derived
> argument. (gfc_match_omp_map_clause): Update call to
> gfc_match_omp_variable_list. (gfc_match_omp_clauses): Update
> calls to gfc_match_omp_map_clause. (gfc_match_oacc_update):
> Update call to gfc_match_omp_clauses. (resolve_omp_clauses):
> Permit derived type variables in ACC UPDATE clauses.
> * trans-openmp.c (gfc_trans_omp_clauses_1): Lower derived type
> members.
>
> gcc/
> * gimplify.c (gimplify_scan_omp_clauses): Update handling of ACC
> UPDATE variables.
>
> gcc/testsuite/
> * gfortran.dg/goacc/derived-types.f90: New test.
>
> libgomp/
> * testsuite/libgomp.oacc-fortran/update-2.f90: New test.
> * testsuite/libgomp.oacc-fortran/derived-type-1.f90: New test.
Note, already OpenMP 4.5 allows the %s in map/to/from clauses, I just didn't
get to that yet.
And OpenMP 5.0 allows arbitrary expressions there.
> @@ -4336,9 +4342,12 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses
> *omp_clauses,
> || n->expr->ref == NULL
> || n->expr->ref->next
> || n->expr->ref->type != REF_ARRAY)
> - gfc_error ("%qs in %s clause at %L is not a proper "
> - "array section", n->sym->name, name,
> - &n->where);
> + {
> + if (n->sym->ts.type != BT_DERIVED)
> + 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)
> gfc_error ("Coarrays not supported in %s clause at %L",
> name, &n->where);
I'm worried about this change a little bit. It isn't guarded for OpenACC
only and I wonder if you actually resolve properly the derived expressions
(look inside of those).
> diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
> index f038f4c..95b15e5 100644
> --- a/gcc/fortran/trans-openmp.c
> +++ b/gcc/fortran/trans-openmp.c
> @@ -2108,7 +2108,68 @@ gfc_trans_omp_clauses (stmtblock_t *block,
> gfc_omp_clauses *clauses,
> tree decl = gfc_get_symbol_decl (n->sym);
> if (DECL_P (decl))
> TREE_ADDRESSABLE (decl) = 1;
> - if (n->expr == NULL || n->expr->ref->u.ar.type == AR_FULL)
> + /* Handle derived-typed members for OpenACC Update. */
> + if (n->sym->ts.type == BT_DERIVED
> + && n->expr != NULL && n->expr->ref != NULL
> + && (n->expr->ref->next == NULL
> + || (n->expr->ref->next != NULL
> + && n->expr->ref->next->type == REF_ARRAY
> + && n->expr->ref->next->u.ar.type == AR_FULL))
> + && (n->expr->ref->type == REF_ARRAY
> + && n->expr->ref->u.ar.type != AR_SECTION))
Like here you have all kinds of conditions, but has resolving made sure all
the needed diagnostics is emitted?
Perhaps at least for now this also should be guarded on OpenACC only,
once OpenMP allows %s in map/to/from, part of this will be usable for it,
but e.g.
> + if (context != type)
> + {
> + tree f2 = c->norestrict_decl;
> + if (!f2 || DECL_FIELD_CONTEXT (f2) != type)
> + for (f2 = TYPE_FIELDS (TREE_TYPE (decl)); f2;
> + f2 = DECL_CHAIN (f2))
> + if (TREE_CODE (f2) == FIELD_DECL
> + && DECL_NAME (f2) == DECL_NAME (field))
> + break;
> + gcc_assert (f2);
> + c->norestrict_decl = f2;
> + field = f2;
> + }
the above stuff looks way too OpenACC specific.
Jakub
