diff mbox series

[7/7,OpenACC] Stricter dynamic data unmapping testing (WIP)

Message ID a952e04dc4eebf38ad47aa05558795b32d816f1b.1590182783.git.julian@codesourcery.com
State New
Headers show
Series Dynamic reference counts for mapped data | expand

Commit Message

Julian Brown May 22, 2020, 10:21 p.m. UTC 
Using the ability to distinguish structural from dynamic mappings'
target_mem_descs, we can adjust how the assertions in goacc_exit_datum
and goacc_exit_data_internal work.  This is possibly a slightly stronger
test than the one introduced earlier in this patch series -- though
actually I haven't quite convinced myself of that.

Anyway, this passes a regression run, with the refcount self-checking
code enabled also.

OK, or any comments?

Julian

ChangeLog

	libgomp/
	* oacc-mem.c (goacc_exit_datum): Adjust self-test code.
	(goacc_exit_data_internal): Likewise.
	* target.c (gomp_unmap_vars_internal): Clear target_mem_desc variable
	list keys on unmapping.
---
 libgomp/oacc-mem.c | 43 ++++++++++++++++++++++++-------------------
 libgomp/target.c   |  8 +++++++-
 2 files changed, 31 insertions(+), 20 deletions(-)
diff mbox series

Patch

diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c
index c8ec3c9a7dd..d7a1d87c9ef 100644
--- a/libgomp/oacc-mem.c
+++ b/libgomp/oacc-mem.c
@@ -755,16 +755,19 @@  goacc_exit_datum (void *h, size_t s, unsigned short kind, int async)
 	gomp_remove_var_async (acc_dev, n, aq);
       else
 	{
-	  int num_mappings = 0;
-	  /* If the target_mem_desc represents a single data mapping, we can
-	     check that it is freed when this splay tree key's refcount
-	     reaches zero.  Otherwise (e.g. for a struct mapping with multiple
-	     members), fall back to skipping the test.  */
-	  for (int i = 0; i < n->tgt->list_count; i++)
-	    if (n->tgt->list[i].key)
-	      num_mappings++;
+	  int remaining_mappings = 0;
+	  bool dynamic = goacc_marked_dynamic_p (n->tgt);
+	  if (dynamic)
+	    {
+	      /* For dynamic mappings, we may have more than one live splay
+		 tree in the target_mem_desc's variable list.  That's not an
+		 error.  */
+	      for (int i = 0; i < n->tgt->list_count; i++)
+		if (n->tgt->list[i].key)
+		  remaining_mappings++;
+	    }
 	  bool is_tgt_unmapped = gomp_remove_var (acc_dev, n);
-	  assert (num_mappings > 1 || is_tgt_unmapped);
+	  assert ((dynamic && remaining_mappings > 0) || is_tgt_unmapped);
 	}
     }
 
@@ -1283,17 +1286,19 @@  goacc_exit_data_internal (struct gomp_device_descr *acc_dev, size_t mapnum,
 			gomp_fatal ("synchronize failed");
 		      }
 		  }
-		int num_mappings = 0;
-		/* If the target_mem_desc represents a single data mapping, we
-		   can check that it is freed when this splay tree key's
-		   refcount reaches zero.  Otherwise (e.g. for a struct
-		   mapping with multiple members), fall back to skipping the
-		   test.  */
-		for (int j = 0; j < n->tgt->list_count; j++)
-		  if (n->tgt->list[j].key)
-		    num_mappings++;
+		int remaining_mappings = 0;
+		bool dynamic = goacc_marked_dynamic_p (n->tgt);
+		if (dynamic)
+		  {
+		   /* For dynamic mappings, we may have more than one live
+		      splay tree in the target_mem_desc's variable list.
+		      That's not an error.  */
+		    for (int j = 0; j < n->tgt->list_count; j++)
+		      if (n->tgt->list[j].key)
+			remaining_mappings++;
+		  }
 		bool is_tgt_unmapped = gomp_remove_var (acc_dev, n);
-		assert (num_mappings > 1 || is_tgt_unmapped);
+		assert ((dynamic && remaining_mappings > 0) || is_tgt_unmapped);
 	      }
 	  }
 	  break;
diff --git a/libgomp/target.c b/libgomp/target.c
index 9a51e1c70f6..f072e050cc1 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -1630,6 +1630,7 @@  gomp_unmap_vars_internal (struct target_mem_desc *tgt, bool do_copyfrom,
       if (k == NULL)
 	continue;
 
+      bool clear_mapping = true;
       bool do_unmap = false;
       if (k->refcount > 1 && k->refcount != REFCOUNT_INFINITY)
 	{
@@ -1637,7 +1638,10 @@  gomp_unmap_vars_internal (struct target_mem_desc *tgt, bool do_copyfrom,
 	  /* If we only have dynamic references left, mark the tgt_mem_desc
 	     appropriately.  */
 	  if (k->refcount == k->dynamic_refcount)
-	    goacc_mark_dynamic (k->tgt);
+	    {
+	      goacc_mark_dynamic (k->tgt);
+	      clear_mapping = false;
+	    }
 	}
       else if (k->refcount == 1)
 	{
@@ -1662,6 +1666,8 @@  gomp_unmap_vars_internal (struct target_mem_desc *tgt, bool do_copyfrom,
 	  assert (!is_tgt_unmapped
 		  || k_tgt != tgt);
 	}
+      if (clear_mapping)
+	tgt->list[i].key = NULL;
     }
 
   if (aq)