[PATCH] OpenACC 2.6 manual deep copy support (attach/detach)

Julian Brown julian@codesourcery.com
Fri Dec 14 19:00:00 GMT 2018


Hi!

Thanks again for review. Here's another iteration of the patch, with
your comments addressed.

One small thing, additionally: I've introduced a function
c_omp_map_clause_name in c-family/c-omp.c to return the name of OpenACC
map clauses -- since many of these use OMP_CLAUSE_MAP and then use
OMP_CLAUSE_MAP_KIND to distinguish between different mapping types,
just emitting 'map' to refer to several different clauses won't be very
meaningful to the user. I didn't see any existing way to get the right
names.

On Thu, 13 Dec 2018 11:57:05 +0100
Jakub Jelinek <jakub@redhat.com> wrote:

> On Mon, Dec 10, 2018 at 07:41:37PM +0000, Julian Brown wrote:
> > @@ -11870,7 +11874,8 @@ c_parser_oacc_wait_list (c_parser *parser,
> > location_t clause_loc, tree list) static tree
> >  c_parser_omp_variable_list (c_parser *parser,
> >  			    location_t clause_loc,
> > -			    enum omp_clause_code kind, tree list)
> > +			    enum omp_clause_code kind, tree list,
> > +			    bool allow_deref)  
> 
> Make it bool allow_deref = false so that you don't have to change all
> callers?

Done.

> > @@ -12579,7 +12597,8 @@ c_parser_omp_clause_lastprivate (c_parser
> > *parser, tree list) }
> >  	}
> >        tree nlist = c_parser_omp_variable_list (parser, loc,
> > -
> > OMP_CLAUSE_LASTPRIVATE, list);
> > +
> > OMP_CLAUSE_LASTPRIVATE, list,
> > +					       false);
> >        c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
> > "expected %<)%>"); if (conditional)
> >  	for (tree c = nlist; c != list; c = OMP_CLAUSE_CHAIN (c))  
> 
> Like these etc.

Those bits reverted.

> > +	  if (ort == C_ORT_ACC && TREE_CODE (t) == MEM_REF)
> > +	    {
> > +	      poly_int64 offset = mem_ref_offset (t).force_shwi ();
> > +	      if (maybe_ne (offset, 0))  
> 
> Just do if (maybe_ne (mem_ref_offset (t), 0)) ?

Fixed.

> > @@ -14432,6 +14491,16 @@ c_finish_omp_clauses (tree clauses, enum
> > c_omp_region_type ort) }
> >  	      if (remove)
> >  		break;
> > +	      if (ort == C_ORT_ACC && TREE_CODE (t) == MEM_REF)
> > +	        {
> > +		  poly_int64 offset = mem_ref_offset
> > (t).force_shwi ();
> > +		  if (maybe_ne (offset, 0))  
> 
> Likewise.

Fixed.

> > @@ -32111,7 +32115,7 @@ check_no_duplicate_clause (tree clauses,
> > enum omp_clause_code code, 
> >  static tree
> >  cp_parser_omp_var_list_no_open (cp_parser *parser, enum
> > omp_clause_code kind,
> > -				tree list, bool *colon)
> > +				tree list, bool *colon, bool
> > allow_deref)  
> 
> See above.

Fixed.

> > @@ -33560,7 +33579,7 @@ cp_parser_omp_clause_reduction (cp_parser
> > *parser, enum omp_clause_code kind, goto resync_fail;
> >  
> >    nlist = cp_parser_omp_var_list_no_open (parser, kind, list,
> > -					  NULL);
> > +					  NULL, false);
> >    for (c = nlist; c != list; c = OMP_CLAUSE_CHAIN (c))
> >      {
> >        OMP_CLAUSE_REDUCTION_CODE (c) = code;  
> 
> See above.

Fixed.

> > --- a/libgomp/target.c
> > +++ b/libgomp/target.c
> > @@ -39,6 +39,9 @@
> >  #include <string.h>
> >  #include <assert.h>
> >  #include <errno.h>
> > +#ifdef RC_CHECKING
> > +#include <stdio.h>
> > +#endif  
> 
> This doesn't belong here.

Oops! This leaked over from the refcount-checking patch via a bad
merge. Fixed.

> > @@ -1089,8 +1274,10 @@ gomp_remove_var (struct gomp_device_descr
> > *devicep, splay_tree_key k) {
> >    bool is_tgt_unmapped = false;
> >    splay_tree_remove (&devicep->mem_map, k);
> > -  if (k->link_key)
> > -    splay_tree_insert (&devicep->mem_map, (splay_tree_node)
> > k->link_key);
> > +  if (k->virtual_refcount == VREFCOUNT_LINK_KEY && k->u.link_key)
> > +    splay_tree_insert (&devicep->mem_map, (splay_tree_node)
> > k->u.link_key);
> > +  if (k->virtual_refcount != VREFCOUNT_LINK_KEY &&
> > k->u.attach_count)
> > +    free (k->u.attach_count);  
> 
> So write
>   if (k->virtual_refcount == VREFCOUNT_LINK_KEY)
>     {
>       if (k->u.link_key)
> 	splay_tree_insert (&devicep->mem_map, (splay_tree_node)
> k->u.link_key); }
>   else if (k->u.attach_count)
>     free (k->u.attach_count);
> ?

I've done so.

Re-tested with offloading to nvptx. OK?

Thanks,

Julian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: attach-detach-4.diff
Type: text/x-patch
Size: 160450 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20181214/a615d575/attachment.bin>


More information about the Gcc-patches mailing list