[PATCH, OpenACC] (1/2) Fix implicit mapping for array slices on lexically-enclosing data constructs (PR70828)

Julian Brown julian@codesourcery.com
Wed Dec 5 21:49:00 GMT 2018


On Tue, 4 Dec 2018 15:02:15 +0100
Jakub Jelinek <jakub@redhat.com> wrote:

> On Tue, Aug 28, 2018 at 03:19:19PM -0400, Julian Brown wrote:
> > 2018-08-28  Julian Brown  <julian@codesourcery.com>
> > 	    Cesar Philippidis  <cesar@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.

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  <julian@codesourcery.com>                        
            Cesar Philippidis  <cesar@codesourcery.com>                        

        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  <jakub@redhat.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: inheritance-of-array-sections-of-data-constructs-3.diff
Type: text/x-patch
Size: 18466 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/fortran/attachments/20181205/e9820ad2/attachment.bin>


More information about the Fortran mailing list