Fix potential race condition in OpenACC "exit data" operations

PR libgomp/92881

	libgomp/
	* libgomp.h (gomp_remove_var_async): Add prototype.
	* oacc-mem.c (delete_copyout): Call gomp_remove_var_async instead of
	gomp_remove_var.
	* target.c (gomp_unref_tgt): Change return type to bool, indicating
	whether target_mem_desc was unmapped.
	(gomp_unref_tgt_void): New.
	(gomp_remove_var): Reimplement in terms of...
	(gomp_remove_var_internal): ...this new helper function.
	(gomp_remove_var_async): New, implemented using above helper function.
	(gomp_unmap_vars_internal): Use gomp_unref_tgt_void instead of
	gomp_unref_tgt.

Reviewed-by: Thomas Schwinge <thomas@codesourcery.com>

From-SVN: r279388

libgomp/ChangeLog

@@ -1,3 +1,19 @@
+2019-12-13  Julian Brown  <julian@codesourcery.com>
+
+	PR libgomp/92881
+
+	* libgomp.h (gomp_remove_var_async): Add prototype.
+	* oacc-mem.c (delete_copyout): Call gomp_remove_var_async instead of
+	gomp_remove_var.
+	* target.c (gomp_unref_tgt): Change return type to bool, indicating
+	whether target_mem_desc was unmapped.
+	(gomp_unref_tgt_void): New.
+	(gomp_remove_var): Reimplement in terms of...
+	(gomp_remove_var_internal): ...this new helper function.
+	(gomp_remove_var_async): New, implemented using above helper function.
+	(gomp_unmap_vars_internal): Use gomp_unref_tgt_void instead of
+	gomp_unref_tgt.
+
 2019-12-13  Andrew Stubbs  <ams@codesourcery.com>
 
 	* testsuite/libgomp.oacc-c-c++-common/acc_prof-init-1.c: Handle gcn.

libgomp/libgomp.h

@@ -1166,6 +1166,8 @@ extern bool gomp_fini_device (struct gomp_device_descr *);
 extern void gomp_free_memmap (struct splay_tree_s *);
 extern void gomp_unload_device (struct gomp_device_descr *);
 extern bool gomp_remove_var (struct gomp_device_descr *, splay_tree_key);
+extern void gomp_remove_var_async (struct gomp_device_descr *, splay_tree_key,
+				   struct goacc_asyncqueue *);
 
 /* work.c */
 

libgomp/oacc-mem.c

@@ -660,7 +660,6 @@ static void
 delete_copyout (unsigned f, void *h, size_t s, int async, const char *libfnname)
 {
   splay_tree_key n;
-  void *d;
   struct goacc_thread *thr = goacc_thread ();
   struct gomp_device_descr *acc_dev = thr->dev;
 
@@ -689,9 +688,6 @@ delete_copyout (unsigned f, void *h, size_t s, int async, const char *libfnname)
       gomp_fatal ("[%p,%d] is not mapped", (void *)h, (int)s);
     }
 
-  d = (void *) (n->tgt->tgt_start + n->tgt_offset
-		+ (uintptr_t) h - n->host_start);
-
   if ((uintptr_t) h < n->host_start || (uintptr_t) h + s > n->host_end)
     {
       size_t host_size = n->host_end - n->host_start;
@@ -723,12 +719,15 @@ delete_copyout (unsigned f, void *h, size_t s, int async, const char *libfnname)
 
   if (n->refcount == 0)
     {
+      goacc_aq aq = get_goacc_asyncqueue (async);
+
       if (f & FLAG_COPYOUT)
 	{
-	  goacc_aq aq = get_goacc_asyncqueue (async);
+	  void *d = (void *) (n->tgt->tgt_start + n->tgt_offset
+			      + (uintptr_t) h - n->host_start);
 	  gomp_copy_dev2host (acc_dev, aq, h, d, s);
 	}
-      gomp_remove_var (acc_dev, n);
+      gomp_remove_var_async (acc_dev, n, aq);
     }
 
   gomp_mutex_unlock (&acc_dev->lock);

libgomp/target.c

@@ -1116,32 +1116,63 @@ gomp_unmap_tgt (struct target_mem_desc *tgt)
   free (tgt);
 }
 
-attribute_hidden bool
-gomp_remove_var (struct gomp_device_descr *devicep, splay_tree_key k)
-{
-  bool is_tgt_unmapped = false;
-  splay_tree_remove (&devicep->mem_map, k);
-  if (k->link_key)
-    splay_tree_insert (&devicep->mem_map, (splay_tree_node) k->link_key);
-  if (k->tgt->refcount > 1)
-    k->tgt->refcount--;
-  else
-    {
-      is_tgt_unmapped = true;
-      gomp_unmap_tgt (k->tgt);
-    }
-  return is_tgt_unmapped;
-}
-
-static void
+static bool
 gomp_unref_tgt (void *ptr)
 {
+  bool is_tgt_unmapped = false;
+
   struct target_mem_desc *tgt = (struct target_mem_desc *) ptr;
 
   if (tgt->refcount > 1)
     tgt->refcount--;
   else
-    gomp_unmap_tgt (tgt);
+    {
+      gomp_unmap_tgt (tgt);
+      is_tgt_unmapped = true;
+    }
+
+  return is_tgt_unmapped;
+}
+
+static void
+gomp_unref_tgt_void (void *ptr)
+{
+  (void) gomp_unref_tgt (ptr);
+}
+
+static inline __attribute__((always_inline)) bool
+gomp_remove_var_internal (struct gomp_device_descr *devicep, splay_tree_key k,
+			  struct goacc_asyncqueue *aq)
+{
+  bool is_tgt_unmapped = false;
+  splay_tree_remove (&devicep->mem_map, k);
+  if (k->link_key)
+    splay_tree_insert (&devicep->mem_map, (splay_tree_node) k->link_key);
+  if (aq)
+    devicep->openacc.async.queue_callback_func (aq, gomp_unref_tgt_void,
+						(void *) k->tgt);
+  else
+    is_tgt_unmapped = gomp_unref_tgt ((void *) k->tgt);
+  return is_tgt_unmapped;
+}
+
+attribute_hidden bool
+gomp_remove_var (struct gomp_device_descr *devicep, splay_tree_key k)
+{
+  return gomp_remove_var_internal (devicep, k, NULL);
+}
+
+/* Remove a variable asynchronously.  This actually removes the variable
+   mapping immediately, but retains the linked target_mem_desc until the
+   asynchronous operation has completed (as it may still refer to target
+   memory).  The device lock must be held before entry, and remains locked on
+   exit.  */
+
+attribute_hidden void
+gomp_remove_var_async (struct gomp_device_descr *devicep, splay_tree_key k,
+		       struct goacc_asyncqueue *aq)
+{
+  (void) gomp_remove_var_internal (devicep, k, aq);
 }
 
 /* Unmap variables described by TGT.  If DO_COPYFROM is true, copy relevant
@@ -1197,7 +1228,7 @@ gomp_unmap_vars_internal (struct target_mem_desc *tgt, bool do_copyfrom,
     }
 
   if (aq)
-    devicep->openacc.async.queue_callback_func (aq, gomp_unref_tgt,
+    devicep->openacc.async.queue_callback_func (aq, gomp_unref_tgt_void,
 						(void *) tgt);
   else
     gomp_unref_tgt ((void *) tgt);