[gcc/devel/omp/gcc-13] In 'libgomp/target.c:gomp_unmap_vars_internal', defer 'gomp_remove_var'

[gcc/devel/omp/gcc-13] In 'libgomp/target.c:gomp_unmap_vars_internal', defer 'gomp_remove_var'

[Kwok Yeung]

https://gcc.gnu.org/g:d8235b106422d661880940ef02a5d272403db79c

commit d8235b106422d661880940ef02a5d272403db79c
Author: Thomas Schwinge <thomas@codesourcery.com>
Date:   Tue Mar 14 19:42:12 2023 +0100

    In 'libgomp/target.c:gomp_unmap_vars_internal', defer 'gomp_remove_var'
    
    An upcoming change requires that 'gomp_remove_var' be deferred until after all
    'gomp_copy_dev2host' calls have been handled.
    
    Do this likewise to how commit 275c736e732d29934e4d22e8f030d5aae8c12a52
    "libgomp: Structure element mapping for OpenMP 5.0" changed 'gomp_exit_data'.
    
            libgomp/
            * target.c (gomp_unmap_vars_internal): Queue splay-tree keys for
            removal after main loop.

Diff:
---
 libgomp/ChangeLog.omp |  3 +++
 libgomp/target.c      | 34 +++++++++++++++++++---------------
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/libgomp/ChangeLog.omp b/libgomp/ChangeLog.omp
index 9748854d786..8cc30c8c493 100644
--- a/libgomp/ChangeLog.omp
+++ b/libgomp/ChangeLog.omp
@@ -1,5 +1,8 @@
 2023-03-24  Thomas Schwinge  <thomas@codesourcery.com>
 
+	* target.c (gomp_unmap_vars_internal): Queue splay-tree keys for
+	removal after main loop.
+
 	PR other/76739
 	* oacc-parallel.c (GOACC_parallel_keyed): Given OpenACC 'async',
 	defer 'free' of non-contiguous array support data structures.
diff --git a/libgomp/target.c b/libgomp/target.c
index 898e9ccf84c..6925f9e8328 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -2185,6 +2185,9 @@ gomp_unmap_vars_internal (struct target_mem_desc *tgt, bool do_copyfrom,
 			     false, NULL);
     }
 
+  size_t nrmvars = 0;
+  splay_tree_key remove_vars[tgt->list_count];
+
   for (i = 0; i < tgt->list_count; i++)
     {
       splay_tree_key k = tgt->list[i].key;
@@ -2206,16 +2209,21 @@ gomp_unmap_vars_internal (struct target_mem_desc *tgt, bool do_copyfrom,
 			    (void *) (k->tgt->tgt_start + k->tgt_offset
 				      + tgt->list[i].offset),
 			    tgt->list[i].length);
+      /* Queue all removals together for processing below.
+	 See also 'gomp_exit_data'.  */
       if (do_remove)
-	{
-	  struct target_mem_desc *k_tgt = k->tgt;
-	  bool is_tgt_unmapped = gomp_remove_var (devicep, k);
-	  /* It would be bad if TGT got unmapped while we're still iterating
-	     over its LIST_COUNT, and also expect to use it in the following
-	     code.  */
-	  assert (!is_tgt_unmapped
-		  || k_tgt != tgt);
-	}
+	remove_vars[nrmvars++] = k;
+    }
+
+  for (i = 0; i < nrmvars; i++)
+    {
+      splay_tree_key k = remove_vars[i];
+      struct target_mem_desc *k_tgt = k->tgt;
+      bool is_tgt_unmapped = gomp_remove_var (devicep, k);
+      /* It would be bad if TGT got unmapped while we're still iterating over
+	 its LIST_COUNT, and also expect to use it in the following code.  */
+      assert (!is_tgt_unmapped
+	      || k_tgt != tgt);
     }
 
   if (aq)
@@ -4162,7 +4170,7 @@ gomp_exit_data (struct gomp_device_descr *devicep, size_t mapnum,
 			       false, NULL);
       }
 
-  int nrmvars = 0;
+  size_t nrmvars = 0;
   splay_tree_key remove_vars[mapnum];
 
   for (i = 0; i < mapnum; i++)
@@ -4225,10 +4233,6 @@ gomp_exit_data (struct gomp_device_descr *devicep, size_t mapnum,
 	     errors if we still have following element siblings to copy back.
 	     While we're at it, it also seems more disciplined to simply
 	     queue all removals together for processing below.
-
-	     Structured block unmapping (i.e. gomp_unmap_vars_internal) should
-	     not have this problem, since they maintain an additional
-	     tgt->refcount = 1 reference to the target_mem_desc to start with.
 	  */
 	  if (do_remove)
 	    remove_vars[nrmvars++] = k;
@@ -4243,7 +4247,7 @@ gomp_exit_data (struct gomp_device_descr *devicep, size_t mapnum,
 	}
     }
 
-  for (int i = 0; i < nrmvars; i++)
+  for (i = 0; i < nrmvars; i++)
     gomp_remove_var (devicep, remove_vars[i]);
 
   gomp_mutex_unlock (&devicep->lock);