This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [gomp4] Fix Fortran deviceptr
- From: James Norris <jnorris at codesourcery dot com>
- To: Cesar Philippidis <cesar at codesourcery dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Cc: <fortran at gcc dot gnu dot org>, Thomas Schwinge <Thomas_Schwinge at mentor dot com>
- Date: Wed, 9 Dec 2015 10:34:12 -0600
- Subject: Re: [gomp4] Fix Fortran deviceptr
- Authentication-results: sourceware.org; auth=none
- References: <56644BBC dot 1050602 at codesourcery dot com> <5665AC01 dot 2020008 at codesourcery dot com> <566703BC dot 7030202 at codesourcery dot com> <56670F21 dot 8090802 at codesourcery dot com>
Cesar,
On 12/08/2015 11:10 AM, Cesar Philippidis wrote:
On 12/08/2015 08:22 AM, James Norris wrote:
2. It appears that deviceptr code in GOACC_parallel_keyed is mostly
identical to GOACC_data_start. Can you put that duplicate code into
a function? That would be easier to maintain in the long run.
Fixed.
Where? I don't see a patch.
Oppssss... Now attached.
Since you're working on fortran, can you take a look at how
gfc_match_omp_clauses is handling OMP_CLAUSE_DEVICEPTR. It seems overly
confusing to me. Could it be done in a similar way as OMP_CLAUSE_COPYIN,
i.e., using gfc_match_omp_map_clause?
Confusion removed and replaced with code similar to how
the other clauses are handled.
Patch to be committed after testing completes.
Thanks!
Jim
diff --git a/gcc/fortran/ChangeLog.gomp b/gcc/fortran/ChangeLog.gomp
index 00e5746..d05326d 100644
--- a/gcc/fortran/ChangeLog.gomp
+++ b/gcc/fortran/ChangeLog.gomp
@@ -1,3 +1,8 @@
+2015-12-09 James Norris <jnorris@codesourcery.com>
+
+ * openmp.c (gfc_match_omp_clauses): Re-write handling of the
+ deviceptr clause.
+
2015-12-08 Thomas Schwinge <thomas@codesourcery.com>
* gfortran.h (symbol_attribute): Add oacc_function_nohost member.
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index b59528be..78d2e1e 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -818,19 +818,10 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, uint64_t mask,
OMP_MAP_ALLOC))
continue;
if ((mask & OMP_CLAUSE_DEVICEPTR)
- && gfc_match ("deviceptr ( ") == MATCH_YES)
- {
- gfc_omp_namelist **list = &c->lists[OMP_LIST_MAP];
- gfc_omp_namelist **head = NULL;
- if (gfc_match_omp_variable_list ("", list, true, NULL, &head, false)
- == MATCH_YES)
- {
- gfc_omp_namelist *n;
- for (n = *head; n; n = n->next)
- n->u.map_op = OMP_MAP_FORCE_DEVICEPTR;
- continue;
- }
- }
+ && gfc_match ("deviceptr ( ") == MATCH_YES
+ && gfc_match_omp_map_clause (&c->lists[OMP_LIST_MAP],
+ OMP_MAP_FORCE_DEVICEPTR))
+ continue;
if ((mask & OMP_CLAUSE_BIND) && c->routine_bind == NULL
&& gfc_match ("bind ( %s )", &c->routine_bind) == MATCH_YES)
{
diff --git a/libgomp/ChangeLog.gomp b/libgomp/ChangeLog.gomp
index d88cb94..ec133a7 100644
--- a/libgomp/ChangeLog.gomp
+++ b/libgomp/ChangeLog.gomp
@@ -1,3 +1,11 @@
+2015-12-09 James Norris <jnorris@codesourcery.com>
+
+ * oacc-parallel.c (handle_ftn_pointers): New function.
+ (GOACC_parallel_keyed, GOACC_data_start): Factor out Fortran pointer
+ handling.
+ * testsuite/libgomp.oacc-fortran/declare-1.f90: Add comment.
+ * testsuite/libgomp.oacc-fortran/deviceptr-1.f90: Fix comment.
+
2015-12-08 James Norris <jnorris@codesourcery.com>
* testsuite/libgomp.oacc-fortran/kernels-map-1.f90: Add new test.
diff --git a/libgomp/oacc-parallel.c b/libgomp/oacc-parallel.c
index a606152..f68db78 100644
--- a/libgomp/oacc-parallel.c
+++ b/libgomp/oacc-parallel.c
@@ -57,6 +57,51 @@ find_pointer (int pos, size_t mapnum, unsigned short *kinds)
return 0;
}
+/* 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;
+ }
+ }
+}
+
static void goacc_wait (int async, int num_waits, va_list *ap);
@@ -99,40 +144,7 @@ GOACC_parallel_keyed (int device, void (*fn) (void *),
thr = goacc_thread ();
acc_dev = thr->dev;
- 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;
- }
- }
+ handle_ftn_pointers (mapnum, hostaddrs, sizes, kinds);
/* Host fallback if "if" clause is false or if the current device is set to
the host. */
@@ -258,7 +270,6 @@ 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",
@@ -273,41 +284,7 @@ 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;
-
- /* 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 the size is right, skip it. */
- 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;
- }
- }
+ handle_ftn_pointers (mapnum, hostaddrs, sizes, kinds);
/* Host fallback or 'do nothing'. */
if ((acc_dev->capabilities & GOMP_OFFLOAD_CAP_SHARED_MEM)
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/declare-1.f90 b/libgomp/testsuite/libgomp.oacc-fortran/declare-1.f90
index e781878..2d4b707 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/declare-1.f90
+++ b/libgomp/testsuite/libgomp.oacc-fortran/declare-1.f90
@@ -1,5 +1,16 @@
! { dg-do run { target openacc_nvidia_accel_selected } }
+! Tests to exercise the declare directive along with
+! the clauses: copy
+! copyin
+! copyout
+! create
+! present
+! present_or_copy
+! present_or_copyin
+! present_or_copyout
+! present_or_create
+
module vars
implicit none
integer z
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/deviceptr-1.f90 b/libgomp/testsuite/libgomp.oacc-fortran/deviceptr-1.f90
index 879cbf1..276a172 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/deviceptr-1.f90
+++ b/libgomp/testsuite/libgomp.oacc-fortran/deviceptr-1.f90
@@ -1,8 +1,8 @@
! { dg-do run }
-!! Test the deviceptr clause with various directives
-!! and in combination with other directives where
-!! the deviceptr variable is implied.
+! Test the deviceptr clause with various directives
+! and in combination with other directives where
+! the deviceptr variable is implied.
subroutine subr1 (a, b)
implicit none