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 > <ce...@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
signature.asc
Description: PGP signature