Hi Jakub! 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
