| Message ID | 559D6530.2080100@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
Hi Jim! On Wed, 8 Jul 2015 13:00:16 -0500, James Norris <jnorris@codesourcery.com> wrote: > This patch adds handling of the deviceptr clause when > used within a Fortran program. Please motivate such non-obvious code changes by a test case. At least to me, it's not at all obvious what's going on here... Is that the occasion where you asked me about how to add a test case to the libgomp testsuite, that'd consist of both a C and a Fortran part, so you'd need to run both compilers? (Unfortunately, I have not yet thought about a solution for this.) But even then, please at least post such test cases with patch submissions. > Committed to gomp-4_0-branch > + * oacc-parallel.c (GOACC_parallel GOACC_data_start): Handle Fortran > + deviceptr clause. > --- a/libgomp/oacc-parallel.c > +++ b/libgomp/oacc-parallel.c > @@ -211,6 +211,21 @@ GOACC_parallel (int device, void (*fn) (void *), > thr = goacc_thread (); > acc_dev = thr->dev; > > + for (i = 0; i < mapnum; i++) > + { > + unsigned short kind1 = kinds[i] & 0xff; > + unsigned short kind2 = kinds[i+1] & 0xff; > + > + if ((kind1 == GOMP_MAP_FORCE_DEVICEPTR && kind2 == GOMP_MAP_POINTER) > + && (sizes[i + 1] == 0) > + && (hostaddrs[i] == *(void **)hostaddrs[i + 1])) > + { > + kinds[i+1] = kinds[i]; > + sizes[i+1] = sizeof (void *); > + hostaddrs[i] = NULL; > + } > + } Ugh. That loop should be bounded by mapnum - 1 to avoid out-of-bounds array accesses. And, such "voodoo" code constructs do need a comment, please. Why does this processing need to happen at run-time, in libgomp? Should something else be done during OMP lowering, for example? > @@ -263,8 +278,13 @@ GOACC_parallel (int device, void (*fn) (void *), > > devaddrs = gomp_alloca (sizeof (void *) * mapnum); > for (i = 0; i < mapnum; i++) > - devaddrs[i] = (void *) (tgt->list[i]->tgt->tgt_start > - + tgt->list[i]->tgt_offset); > + { > + if (tgt->list[i] != NULL) > + devaddrs[i] = (void *) (tgt->list[i]->tgt->tgt_start > + + tgt->list[i]->tgt_offset); > + else > + devaddrs[i] = NULL; > + } > > acc_dev->openacc.exec_func (tgt_fn, mapnum, hostaddrs, devaddrs, sizes, kinds, > num_gangs, num_workers, vector_length, async, > @@ -305,6 +326,21 @@ GOACC_data_start (int device, size_t mapnum, > struct goacc_thread *thr = goacc_thread (); > struct gomp_device_descr *acc_dev = thr->dev; > > + for (i = 0; i < mapnum; i++) > + { > + unsigned short kind1 = kinds[i] & 0xff; > + unsigned short kind2 = kinds[i+1] & 0xff; > + > + if ((kind1 == GOMP_MAP_FORCE_DEVICEPTR && kind2 == GOMP_MAP_POINTER) > + && (sizes[i + 1] == 0) > + && (hostaddrs[i] == *(void **)hostaddrs[i + 1])) > + { > + kinds[i+1] = kinds[i]; > + sizes[i+1] = sizeof (void *); > + hostaddrs[i] = NULL; > + } > + } The same code being repeated here for the OpenACC data construct -- should this be moved to a common place? Grüße, Thomas
diff --git a/libgomp/ChangeLog.gomp b/libgomp/ChangeLog.gomp index 418474d..1949d78 100644 --- a/libgomp/ChangeLog.gomp +++ b/libgomp/ChangeLog.gomp @@ -1,3 +1,8 @@ +2015-07-08 James Norris <jnorris@codesourcery.com> + + * oacc-parallel.c (GOACC_parallel GOACC_data_start): Handle Fortran + deviceptr clause. + 2015-07-07 Thomas Schwinge <thomas@codesourcery.com> * testsuite/libgomp.oacc-c++/c++.exp (check_effective_target_c): diff --git a/libgomp/oacc-parallel.c b/libgomp/oacc-parallel.c index 70758bc..eeb08c4 100644 --- a/libgomp/oacc-parallel.c +++ b/libgomp/oacc-parallel.c @@ -211,6 +211,21 @@ GOACC_parallel (int device, void (*fn) (void *), thr = goacc_thread (); acc_dev = thr->dev; + for (i = 0; i < mapnum; i++) + { + unsigned short kind1 = kinds[i] & 0xff; + unsigned short kind2 = kinds[i+1] & 0xff; + + if ((kind1 == GOMP_MAP_FORCE_DEVICEPTR && kind2 == GOMP_MAP_POINTER) + && (sizes[i + 1] == 0) + && (hostaddrs[i] == *(void **)hostaddrs[i + 1])) + { + kinds[i+1] = kinds[i]; + sizes[i+1] = sizeof (void *); + hostaddrs[i] = NULL; + } + } + /* Host fallback if "if" clause is false or if the current device is set to the host. */ if (host_fallback) @@ -263,8 +278,13 @@ GOACC_parallel (int device, void (*fn) (void *), devaddrs = gomp_alloca (sizeof (void *) * mapnum); for (i = 0; i < mapnum; i++) - devaddrs[i] = (void *) (tgt->list[i]->tgt->tgt_start - + tgt->list[i]->tgt_offset); + { + if (tgt->list[i] != NULL) + devaddrs[i] = (void *) (tgt->list[i]->tgt->tgt_start + + tgt->list[i]->tgt_offset); + else + devaddrs[i] = NULL; + } acc_dev->openacc.exec_func (tgt_fn, mapnum, hostaddrs, devaddrs, sizes, kinds, num_gangs, num_workers, vector_length, async, @@ -291,6 +311,7 @@ GOACC_data_start (int device, size_t mapnum, { bool host_fallback = device == GOMP_DEVICE_HOST_FALLBACK; struct target_mem_desc *tgt; + int i; #ifdef HAVE_INTTYPES_H gomp_debug (0, "%s: mapnum=%"PRIu64", hostaddrs=%p, sizes=%p, kinds=%p\n", @@ -305,6 +326,21 @@ GOACC_data_start (int device, size_t mapnum, struct goacc_thread *thr = goacc_thread (); struct gomp_device_descr *acc_dev = thr->dev; + for (i = 0; i < mapnum; i++) + { + unsigned short kind1 = kinds[i] & 0xff; + unsigned short kind2 = kinds[i+1] & 0xff; + + if ((kind1 == GOMP_MAP_FORCE_DEVICEPTR && kind2 == GOMP_MAP_POINTER) + && (sizes[i + 1] == 0) + && (hostaddrs[i] == *(void **)hostaddrs[i + 1])) + { + kinds[i+1] = kinds[i]; + sizes[i+1] = sizeof (void *); + hostaddrs[i] = NULL; + } + } + /* Host fallback or 'do nothing'. */ if ((acc_dev->capabilities & GOMP_OFFLOAD_CAP_SHARED_MEM) || host_fallback) @@ -673,8 +709,6 @@ GOACC_register_static (void *addr, int size, unsigned int mask) oacc_statics = s; } -#include <stdio.h> - void GOACC_declare (int device, size_t mapnum, void **hostaddrs, size_t *sizes, unsigned short *kinds)