On Tue, Aug 28, 2018 at 03:19:19PM -0400, Julian Brown wrote:
> 2018-08-28 Julian Brown <jul...@codesourcery.com>
> Cesar Philippidis <ce...@codesourcery.com>
>
> PR middle-end/70828
>
> gcc/
> * gimplify.c (gimplify_omp_ctx): Add decl_data_clause hash map.
> (new_omp_context): Initialise above.
> (delete_omp_context): Delete above.
> (gimplify_scan_omp_clauses): Scan for array mappings on data constructs,
> and record in above map.
> (gomp_needs_data_present): New function.
> (gimplify_adjust_omp_clauses_1): Handle data mappings (e.g. array
> slices) declared in lexically-enclosing data constructs.
> * omp-low.c (lower_omp_target): Allow decl for bias not to be present
> in omp context.
>
> gcc/testsuite/
> * c-c++-common/goacc/acc-data-chain.c: New test.
> * gfortran.dg/goacc/pr70828.f90: New test.
> * gfortran.dg/goacc/pr70828-2.f90: New test.
>
> libgomp/
> * testsuite/libgomp.oacc-c-c++-common/pr70828.c: New test.
> * testsuite/libgomp.oacc-fortran/implicit_copy.f90: New test.
> * testsuite/libgomp.oacc-fortran/pr70828.f90: New test.
> * testsuite/libgomp.oacc-fortran/pr70828-2.f90: New test.
> * testsuite/libgomp.oacc-fortran/pr70828-3.f90: New test.
> * testsuite/libgomp.oacc-fortran/pr70828-5.f90: New test.
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -191,6 +191,7 @@ struct gimplify_omp_ctx
> bool target_map_scalars_firstprivate;
> bool target_map_pointers_as_0len_arrays;
> bool target_firstprivatize_array_bases;
> + hash_map<tree, std::pair<tree, tree> > *decl_data_clause;
> };
>
> static struct gimplify_ctx *gimplify_ctxp;
> @@ -413,6 +414,7 @@ new_omp_context (enum omp_region_type region_type)
> c->default_kind = OMP_CLAUSE_DEFAULT_SHARED;
> else
> c->default_kind = OMP_CLAUSE_DEFAULT_UNSPECIFIED;
> + c->decl_data_clause = new hash_map<tree, std::pair<tree, tree> >;
Not really happy about creating this unconditionally. Can you leave it
NULL by default and only initialize for contexts where it will be needed?
> @@ -7793,8 +7796,21 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq
> *pre_p,
> case OMP_TARGET:
> break;
> case OACC_DATA:
> - if (TREE_CODE (TREE_TYPE (decl)) != ARRAY_TYPE)
> - break;
> + {
> + tree nextc = OMP_CLAUSE_CHAIN (c);
> + if (nextc
> + && OMP_CLAUSE_CODE (nextc) == OMP_CLAUSE_MAP
> + && (OMP_CLAUSE_MAP_KIND (nextc)
> + == GOMP_MAP_FIRSTPRIVATE_POINTER
> + || OMP_CLAUSE_MAP_KIND (nextc) == GOMP_MAP_POINTER))
> + {
> + tree base_addr = OMP_CLAUSE_DECL (nextc);
> + ctx->decl_data_clause->put (base_addr,
> + std::make_pair (unshare_expr (c), unshare_expr (nextc)));
Don't like the wrapping here, can you just split it up:
std::pair<tree, tree> p
= std::make_pair (unshare_expr (c),
unshare_expr (nextc));
ctx->decl_data_clause->put (base_addr, p);
or similar?
> +
> +static std::pair<tree, tree> *
> +gomp_needs_data_present (tree decl)
Would be helpful to have acc/oacc in the function name.
> +{
> + gimplify_omp_ctx *ctx = NULL;
> +
> + if (TREE_CODE (TREE_TYPE (decl)) != ARRAY_TYPE
> + && TREE_CODE (TREE_TYPE (decl)) != POINTER_TYPE
> + && (TREE_CODE (TREE_TYPE (decl)) != POINTER_TYPE
> + || TREE_CODE (TREE_TYPE (TREE_TYPE (decl))) != ARRAY_TYPE))
> + return NULL;
> +
> + if (gimplify_omp_ctxp->region_type != ORT_ACC_PARALLEL
> + && gimplify_omp_ctxp->region_type != ORT_ACC_KERNELS)
> + return NULL;
And move this test to the top.
> --- a/gcc/omp-low.c
> +++ b/gcc/omp-low.c
> @@ -8411,9 +8411,10 @@ lower_omp_target (gimple_stmt_iterator *gsi_p,
> omp_context *ctx)
> x = fold_convert_loc (clause_loc, type, x);
> if (!integer_zerop (OMP_CLAUSE_SIZE (c)))
> {
> - tree bias = OMP_CLAUSE_SIZE (c);
> - if (DECL_P (bias))
> - bias = lookup_decl (bias, ctx);
> + tree bias = OMP_CLAUSE_SIZE (c), remapped_bias;
> + if (DECL_P (bias)
> + && (remapped_bias = maybe_lookup_decl (bias, ctx)))
> + bias = remapped_bias;
> bias = fold_convert_loc (clause_loc, sizetype, bias);
> bias = fold_build1_loc (clause_loc, NEGATE_EXPR, sizetype,
> bias);
This is shared with OpenMP and must be conditionalized for OpenACC only.
Jakub
