diff mbox series

[1/2] openacc: Helper functions for enter/exit data using single mapping

Message ID e06bfb2f85d5f8429a09539ff2410daa9b7180cf.1594338631.git.julian@codesourcery.com
State New
Headers show
Series openacc: Refactor enter/exit data code paths, adjust dynamic refcount semantics | expand

Commit Message

Julian Brown July 10, 2020, 12:06 a.m. UTC 
This patch factors out the parts of goacc_enter_datum and
goacc_exit_datum that can be shared with goacc_enter_data_internal
and goacc_exit_data_internal respectively (in the next patch),
without overloading function return values or complicating code paths
unnecessarily.

OK?

Thanks,

Julian

2020-07-10  Julian Brown  <julian@codesourcery.com>

libgomp/
	* oacc-mem.c (goacc_map_var_existing): New function.
	(goacc_enter_datum): Use above function.
	(goacc_exit_datum_1): New function.
	(goacc_exit_datum): Use above function.
---
 libgomp/oacc-mem.c | 127 ++++++++++++++++++++++++++-------------------
 1 file changed, 74 insertions(+), 53 deletions(-)

Comments

Thomas Schwinge July 10, 2020, 2:22 p.m. UTC | #1 
Hi Julian!

On 2020-07-09T17:06:58-0700, Julian Brown <julian@codesourcery.com> wrote:
> This patch factors out the parts of goacc_enter_datum and
> goacc_exit_datum that can be shared with goacc_enter_data_internal
> and goacc_exit_data_internal respectively (in the next patch),
> without overloading function return values or complicating code paths
> unnecessarily.

Thanks!

> OK?

Just one comment (incremental patch attached; don't need to re-test, I've
done that).  With that, OK for master and releases/gcc-10 branches.

> --- a/libgomp/oacc-mem.c
> +++ b/libgomp/oacc-mem.c

> +/* Helper function to map a single dynamic data item, represented by a single
> +   mapping.  The acc_dev->lock should be held on entry, and remains locked on
> +   exit.  */
> +
> +static void *
> +goacc_map_var_existing (struct gomp_device_descr *acc_dev, void *hostaddr,
> +                     size_t size, goacc_aq aq, splay_tree_key n)

Given that this deals only with existing mappings, for which we're never
going to do any host/device data copying etc., this cannot ever make any
use of 'aq', so we shouldn't include it here (see incremental patch
attached).

> +{
> +  assert (n);
> +
> +  /* Present. */
> +  void *d = (void *) (n->tgt->tgt_start + n->tgt_offset + hostaddr
> +         - n->host_start);
> +
> +  if (hostaddr + size > (void *) n->host_end)
> +    {
> +      gomp_mutex_unlock (&acc_dev->lock);
> +      gomp_fatal ("[%p,+%d] not mapped", hostaddr, (int) size);
> +    }
> +
> +  assert (n->refcount != REFCOUNT_LINK);
> +  if (n->refcount != REFCOUNT_INFINITY)
> +    {
> +      n->refcount++;
> +      n->virtual_refcount++;
> +    }
> +
> +  return d;
> +}


Grüße
 Thomas


-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
diff mbox series

Patch

diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c
index 247ca135804..6f9043bf5b1 100644
--- a/libgomp/oacc-mem.c
+++ b/libgomp/oacc-mem.c
@@ -498,6 +498,36 @@  acc_unmap_data (void *h)
 }
 
 
+/* Helper function to map a single dynamic data item, represented by a single
+   mapping.  The acc_dev->lock should be held on entry, and remains locked on
+   exit.  */
+
+static void *
+goacc_map_var_existing (struct gomp_device_descr *acc_dev, void *hostaddr,
+			size_t size, goacc_aq aq, splay_tree_key n)
+{
+  assert (n);
+
+  /* Present. */
+  void *d = (void *) (n->tgt->tgt_start + n->tgt_offset + hostaddr
+	    - n->host_start);
+
+  if (hostaddr + size > (void *) n->host_end)
+    {
+      gomp_mutex_unlock (&acc_dev->lock);
+      gomp_fatal ("[%p,+%d] not mapped", hostaddr, (int) size);
+    }
+
+  assert (n->refcount != REFCOUNT_LINK);
+  if (n->refcount != REFCOUNT_INFINITY)
+    {
+      n->refcount++;
+      n->virtual_refcount++;
+    }
+
+  return d;
+}
+
 /* Enter dynamic mapping for a single datum.  Return the device pointer.  */
 
 static void *
@@ -529,27 +559,10 @@  goacc_enter_datum (void **hostaddrs, size_t *sizes, void *kinds, int async)
   gomp_mutex_lock (&acc_dev->lock);
 
   n = lookup_host (acc_dev, hostaddrs[0], sizes[0]);
+  goacc_aq aq = get_goacc_asyncqueue (async);
   if (n)
     {
-      void *h = hostaddrs[0];
-      size_t s = sizes[0];
-
-      /* Present. */
-      d = (void *) (n->tgt->tgt_start + n->tgt_offset + h - n->host_start);
-
-      if ((h + s) > (void *)n->host_end)
-	{
-	  gomp_mutex_unlock (&acc_dev->lock);
-	  gomp_fatal ("[%p,+%d] not mapped", (void *)h, (int)s);
-	}
-
-      assert (n->refcount != REFCOUNT_LINK);
-      if (n->refcount != REFCOUNT_INFINITY)
-	{
-	  n->refcount++;
-	  n->virtual_refcount++;
-	}
-
+      d = goacc_map_var_existing (acc_dev, hostaddrs[0], sizes[0], aq, n);
       gomp_mutex_unlock (&acc_dev->lock);
     }
   else
@@ -558,8 +571,6 @@  goacc_enter_datum (void **hostaddrs, size_t *sizes, void *kinds, int async)
 
       gomp_mutex_unlock (&acc_dev->lock);
 
-      goacc_aq aq = get_goacc_asyncqueue (async);
-
       struct target_mem_desc *tgt
 	= gomp_map_vars_async (acc_dev, aq, mapnum, hostaddrs, NULL, sizes,
 			       kinds, true, GOMP_MAP_VARS_OPENACC_ENTER_DATA);
@@ -649,38 +660,13 @@  acc_pcopyin (void *h, size_t s)
 #endif
 
 
-/* Exit a dynamic mapping for a single variable.  */
+/* Helper function to unmap a single data item.  Device lock should be held on
+   entry, and remains locked on exit.  */
 
 static void
-goacc_exit_datum (void *h, size_t s, unsigned short kind, int async)
+goacc_exit_datum_1 (struct gomp_device_descr *acc_dev, void *h, size_t s,
+		    unsigned short kind, splay_tree_key n, goacc_aq aq)
 {
-  /* No need to call lazy open, as the data must already have been
-     mapped.  */
-
-  kind &= 0xff;
-
-  struct goacc_thread *thr = goacc_thread ();
-  struct gomp_device_descr *acc_dev = thr->dev;
-
-  if (acc_dev->capabilities & GOMP_OFFLOAD_CAP_SHARED_MEM)
-    return;
-
-  acc_prof_info prof_info;
-  acc_api_info api_info;
-  bool profiling_p = GOACC_PROFILING_SETUP_P (thr, &prof_info, &api_info);
-  if (profiling_p)
-    {
-      prof_info.async = async;
-      prof_info.async_queue = prof_info.async;
-    }
-
-  gomp_mutex_lock (&acc_dev->lock);
-
-  splay_tree_key n = lookup_host (acc_dev, h, s);
-  if (!n)
-    /* PR92726, RP92970, PR92984: no-op.  */
-    goto out;
-
   if ((uintptr_t) h < n->host_start || (uintptr_t) h + s > n->host_end)
     {
       size_t host_size = n->host_end - n->host_start;
@@ -691,6 +677,7 @@  goacc_exit_datum (void *h, size_t s, unsigned short kind, int async)
 
   bool finalize = (kind == GOMP_MAP_DELETE
 		   || kind == GOMP_MAP_FORCE_FROM);
+
   if (finalize)
     {
       if (n->refcount != REFCOUNT_INFINITY)
@@ -709,8 +696,6 @@  goacc_exit_datum (void *h, size_t s, unsigned short kind, int async)
 
   if (n->refcount == 0)
     {
-      goacc_aq aq = get_goacc_asyncqueue (async);
-
       bool copyout = (kind == GOMP_MAP_FROM
 		      || kind == GOMP_MAP_FORCE_FROM);
       if (copyout)
@@ -740,8 +725,44 @@  goacc_exit_datum (void *h, size_t s, unsigned short kind, int async)
 	  assert (is_tgt_unmapped || num_mappings > 1);
 	}
     }
+}
+
+
+/* Exit a dynamic mapping for a single variable.  */
+
+static void
+goacc_exit_datum (void *h, size_t s, unsigned short kind, int async)
+{
+  /* No need to call lazy open, as the data must already have been
+     mapped.  */
+
+  kind &= 0xff;
+
+  struct goacc_thread *thr = goacc_thread ();
+  struct gomp_device_descr *acc_dev = thr->dev;
+
+  if (acc_dev->capabilities & GOMP_OFFLOAD_CAP_SHARED_MEM)
+    return;
+
+  acc_prof_info prof_info;
+  acc_api_info api_info;
+  bool profiling_p = GOACC_PROFILING_SETUP_P (thr, &prof_info, &api_info);
+  if (profiling_p)
+    {
+      prof_info.async = async;
+      prof_info.async_queue = prof_info.async;
+    }
+
+  gomp_mutex_lock (&acc_dev->lock);
+
+  splay_tree_key n = lookup_host (acc_dev, h, s);
+  /* Non-present data is a no-op: PR92726, RP92970, PR92984.  */
+  if (n)
+    {
+      goacc_aq aq = get_goacc_asyncqueue (async);
+      goacc_exit_datum_1 (acc_dev, h, s, kind, n, aq);
+    }
 
- out:
   gomp_mutex_unlock (&acc_dev->lock);
 
   if (profiling_p)