[OpenACC] Update OpenACC data clause semantics to the 2.5 behavior - runtime

[Thomas Schwinge]

Hi Jakub!

Any comments on my questions, please?

On Thu, 02 May 2019 16:03:09 +0200, I wrote:
> I'm currently working on other pending OpenACC 'deviceptr' clause patches
> from our backlog, and I noticed the following, which I don't understand.
> You reviewed and approved this patch, could you please help?
> 
> On Tue, 19 Jun 2018 10:01:20 -0700, Cesar Philippidis <cesar@codesourcery.com> wrote:
> > --- a/libgomp/oacc-parallel.c
> > +++ b/libgomp/oacc-parallel.c
> 
> > +/* Handle the mapping pair that are presented when a
> > +   deviceptr clause is used with Fortran.  */
> > +
> > +static void
> > +handle_ftn_pointers (size_t mapnum, void **hostaddrs, size_t *sizes,
> > +		     unsigned short *kinds)
> > +{
> > +  int i;
> > +
> > +  for (i = 0; i < mapnum; i++)
> > +    {
> > +      unsigned short kind1 = kinds[i] & 0xff;
> > +
> > +      /* Handle Fortran deviceptr clause.  */
> > +      if (kind1 == GOMP_MAP_FORCE_DEVICEPTR)
> > +	{
> > +	  unsigned short kind2;
> > +
> > +	  if (i < (signed)mapnum - 1)
> > +	    kind2 = kinds[i + 1] & 0xff;
> > +	  else
> > +	    kind2 = 0xffff;
> > +
> > +	  if (sizes[i] == sizeof (void *))
> > +	    continue;
> > +
> > +	  /* At this point, we're dealing with a Fortran deviceptr.
> > +	     If the next element is not what we're expecting, then
> > +	     this is an instance of where the deviceptr variable was
> > +	     not used within the region and the pointer was removed
> > +	     by the gimplifier.  */
> > +	  if (kind2 == GOMP_MAP_POINTER
> > +	      && sizes[i + 1] == 0
> > +	      && hostaddrs[i] == *(void **)hostaddrs[i + 1])
> > +	    {
> > +	      kinds[i+1] = kinds[i];
> > +	      sizes[i+1] = sizeof (void *);
> > +	    }
> > +
> > +	  /* Invalidate the entry.  */
> > +	  hostaddrs[i] = NULL;
> > +	}
> > +    }
> >  }
> 
> This is used for rewriting the mappings for OpenACC 'parallel'
> etc. constructs:
> 
> > @@ -88,6 +141,8 @@ GOACC_parallel_keyed (int device, void (*fn) (void *),
> >    thr = goacc_thread ();
> >    acc_dev = thr->dev;
> >  
> > +  handle_ftn_pointers (mapnum, hostaddrs, sizes, kinds);
> > +
> >    /* Host fallback if "if" clause is false or if the current device is set to
> >       the host.  */
> >    if (host_fallback)
> 
> ..., and on our OpenACC development branch likewise for OpenACC 'data'
> constructs ('GOACC_data_start').
> 
> What this function seems to be doing, as I understand this, is that when
> there is an 'GOMP_MAP_FORCE_DEVICEPTR' with a size not eqal to pointer
> size (which should never happen, as per the information given
> 'include/gomp-constants.h'?), which is followed by a 'GOMP_MAP_POINTER',
> then preserve the 'GOMP_MAP_FORCE_DEVICEPTR' (by storing it into the slot
> of the 'GOMP_MAP_POINTER'), and unconditionally remove the
> 'GOMP_MAP_POINTER'.  This seems like a strange choice of a GCC/libgomp
> ABI to me -- or am I not understanding this correctly?
> 
> Instead of rewriting the mappings at run time, why isn't (presumably) the
> gimplifier changed to just emit the correct mappings?


Grüße
 Thomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 658 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20190529/04d97e35/attachment.sig>