On Fri, Jun 29, 2018 at 12:04:58PM -0700, Cesar Philippidis wrote:
> 2018-06-29 Cesar Philippidis <ce...@codesourcery.com>
> James Norris <jnor...@codesourcery.com>
>
> gcc/fortran/
> * openmp.c (gfc_match_omp_map_clause): Re-write handling of the
> deviceptr clause. Add new common_blocks argument. Propagate it to
> gfc_match_omp_variable_list.
> (gfc_match_omp_clauses): Update calls to gfc_match_omp_map_clauses.
> (resolve_positive_int_expr): Promote the warning to an error.
> (check_array_not_assumed): Remove pointer check.
> (resolve_oacc_nested_loops): Error on do concurrent loops.
> * trans-openmp.c (gfc_omp_finish_clause): Don't create pointer data
> mappings for deviceptr clauses.
> (gfc_trans_omp_clauses): Likewise.
>
> gcc/
> * gimplify.c (enum gimplify_omp_var_data): Add GOVD_DEVICETPR.
> (oacc_default_clause): Privatize fortran common blocks.
> (omp_notice_variable): Add GOVD_DEVICEPTR attribute when appropriate.
> Defer the expansion of DECL_VALUE_EXPR for common block decls.
> (gimplify_scan_omp_clauses): Add GOVD_DEVICEPTR attribute when
> appropriate.
> (gimplify_adjust_omp_clauses_1): Set GOMP_MAP_FORCE_DEVICEPTR for
> implicit deviceptr mappings.
>
> gcc/testsuite/
> * c-c++-common/goacc/deviceptr-4.c: Update.
> * gfortran.dg/goacc/common-block-1.f90: New test.
> * gfortran.dg/goacc/common-block-2.f90: New test.
> * gfortran.dg/goacc/loop-2.f95: Update.
> * gfortran.dg/goacc/loop-3-2.f95: Update.
> * gfortran.dg/goacc/loop-3.f95: Update.
> * gfortran.dg/goacc/loop-5.f95: Update.
> * gfortran.dg/goacc/pr72715.f90: New test.
> * gfortran.dg/goacc/sie.f95: Update.
> * gfortran.dg/goacc/tile-1.f90: Update.
> * gfortran.dg/gomp/pr77516.f90: Update.
>
> libgomp/
> * oacc-parallel.c (GOACC_parallel_keyed): Handle Fortran deviceptr
> clause.
> (GOACC_data_start): Likewise.
> * testsuite/libgomp.oacc-fortran/common-block-1.f90: New test.
> * testsuite/libgomp.oacc-fortran/common-block-2.f90: New test.
> * testsuite/libgomp.oacc-fortran/common-block-3.f90: New test.
> * testsuite/libgomp.oacc-fortran/deviceptr-1.f90: New test.
> --- a/gcc/fortran/openmp.c
> +++ b/gcc/fortran/openmp.c
> @@ -914,10 +914,11 @@ omp_inv_mask::omp_inv_mask (const omp_mask &m) :
> omp_mask (m)
> mapping. */
>
> static bool
> -gfc_match_omp_map_clause (gfc_omp_namelist **list, gfc_omp_map_op map_op)
> +gfc_match_omp_map_clause (gfc_omp_namelist **list, gfc_omp_map_op map_op,
> + bool common_blocks)
> {
> gfc_omp_namelist **head = NULL;
> - if (gfc_match_omp_variable_list ("", list, false, NULL, &head, true)
> + if (gfc_match_omp_variable_list ("", list, common_blocks, NULL, &head,
> true)
> == MATCH_YES)
> {
> gfc_omp_namelist *n;
> @@ -1039,7 +1040,7 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const
> omp_mask mask,
> if ((mask & OMP_CLAUSE_COPY)
> && gfc_match ("copy ( ") == MATCH_YES
> && gfc_match_omp_map_clause (&c->lists[OMP_LIST_MAP],
> - OMP_MAP_TOFROM))
> + OMP_MAP_TOFROM, openacc))
Why? OpenMP doesn't have a copy clause, so I'd expect true here.
> continue;
> if (mask & OMP_CLAUSE_COPYIN)
> {
> @@ -1047,7 +1048,7 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const
> omp_mask mask,
> {
> if (gfc_match ("copyin ( ") == MATCH_YES
> && gfc_match_omp_map_clause (&c->lists[OMP_LIST_MAP],
> - OMP_MAP_TO))
> + OMP_MAP_TO, true))
> continue;
OpenMP does have a copyin clause, but it is handled below, so this one is ok.
> @@ -1156,12 +1157,12 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const
> omp_mask mask,
> && openacc
> && gfc_match ("device ( ") == MATCH_YES
> && gfc_match_omp_map_clause (&c->lists[OMP_LIST_MAP],
> - OMP_MAP_FORCE_TO))
> + OMP_MAP_FORCE_TO, false))
> continue;
> if ((mask & OMP_CLAUSE_DEVICEPTR)
> && gfc_match ("deviceptr ( ") == MATCH_YES
> && gfc_match_omp_map_clause (&c->lists[OMP_LIST_MAP],
> - OMP_MAP_FORCE_DEVICEPTR))
> + OMP_MAP_FORCE_DEVICEPTR, false))
> continue;
Not sure about these, your call. deviceptr is OpenACC specific clause,
device is in both, but means something different in OpenMP. In any case,
haven't looked if OpenACC allows common blocks in these clauses.
> @@ -3718,8 +3719,8 @@ resolve_positive_int_expr (gfc_expr *expr, const char
> *clause)
> if (expr->expr_type == EXPR_CONSTANT
> && expr->ts.type == BT_INTEGER
> && mpz_sgn (expr->value.integer) <= 0)
> - gfc_warning (0, "INTEGER expression of %s clause at %L must be positive",
> - clause, &expr->where);
> + gfc_error ("INTEGER expression of %s clause at %L must be positive",
> + clause, &expr->where);
> }
This affects OpenMP too and makes it inconsistent with C/C++. Why?
If you need it for OpenACC clauses, then we need two routines.
> static void
> @@ -3777,10 +3778,6 @@ check_array_not_assumed (gfc_symbol *sym, locus loc,
> const char *name)
> if (sym->as && sym->as->type == AS_ASSUMED_RANK)
> gfc_error ("Assumed rank array %qs in %s clause at %L",
> sym->name, name, &loc);
> - if (sym->as && sym->as->type == AS_DEFERRED && sym->attr.pointer
> - && !sym->attr.contiguous)
> - gfc_error ("Noncontiguous deferred shape array %qs in %s clause at %L",
> - sym->name, name, &loc);
> }
Perhaps it would help to mention oacc in the name of this routine to make
it clearer that it is OpenACC only. The change is ok if that is what
OpenACC now allows.
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -105,6 +105,9 @@ enum gimplify_omp_var_data
> /* Flag for GOVD_MAP: must be present already. */
> GOVD_MAP_FORCE_PRESENT = 524288,
>
> + /* Flag for OpenACC deviceptrs. */
> + GOVD_DEVICEPTR = (1<<21),
Please use the same style of constants as in the rest (and, you need
to adjust anyway for current trunk).
Jakub
