On Tue, 4 Dec 2018 15:02:15 +0100 Jakub Jelinek wrote: > On Tue, Aug 28, 2018 at 03:19:19PM -0400, Julian Brown wrote: > > 2018-08-28 Julian Brown > > Cesar Philippidis > > > > 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 > *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 > > >; > > 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 p > = std::make_pair (unshare_expr (c), > unshare_expr (nextc)); > ctx->decl_data_clause->put (base_addr, p); > or similar? > > > + > > +static std::pair * > > +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. Thanks for review! How's this version? I took the liberty of fixing the patch for Fortran array-descriptor mappings that use a PSET, also, and adding another test for that functionality. Re-tested with offloading to nvptx. OK? Julian 2018-08-28 Julian Brown Cesar Philippidis gcc/ * gimplify.c (oacc_array_mapping_info): New struct. (gimplify_omp_ctx): Add decl_data_clause hash map. (new_omp_context): Zero-initialise above. (delete_omp_context): Delete above if allocated. (gimplify_scan_omp_clauses): Scan for array mappings on data constructs, and record in above map. (gomp_oacc_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 OpenACC 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. * testsuite/libgomp.oacc-fortran/pr70828-6.f90: New test. Reviewed-by: Jakub Jelinek