diff mbox series

[OpenACC,'exit,data'] Simplify 'GOMP_MAP_STRUCT' handling (was: [PATCH 07/13] OpenACC 2.6 deep copy: libgomp parts)

Message ID 87y2p1qy5q.fsf@euler.schwinge.homeip.net
State New
Headers show
Series [OpenACC,'exit,data'] Simplify 'GOMP_MAP_STRUCT' handling (was: [PATCH 07/13] OpenACC 2.6 deep copy: libgomp parts) | expand

Commit Message

Thomas Schwinge June 5, 2020, 4:23 p.m. UTC 
Hi!

On 2020-05-20T11:37:35+0200, I wrote:
> Moving this over, from the "Fix component mappings with derived types for
> OpenACC" thread,
> <http://mid.mail-archive.com/20200110014945.5643ace5@squid.athome>, where
> you propose to change this 'GOMP_MAP_STRUCT' handling code:
>
> On 2019-12-17T22:03:47-0800, Julian Brown <julian@codesourcery.com> wrote:
>> --- a/libgomp/oacc-mem.c
>> +++ b/libgomp/oacc-mem.c
>
>> @@ -1075,6 +1119,39 @@ goacc_exit_data_internal (struct gomp_device_descr *acc_dev, size_t mapnum,
>>            gomp_remove_var_async (acc_dev, n, aq);
>>        }
>>        break;
>> +
>> +    case GOMP_MAP_STRUCT:
>> +      {
>> +        int elems = sizes[i];
>> +        for (int j = 1; j <= elems; j++)
>> +          {
>> +            struct splay_tree_key_s k;
>> +            k.host_start = (uintptr_t) hostaddrs[i + j];
>> +            k.host_end = k.host_start + sizes[i + j];
>> +            splay_tree_key str;
>> +            str = splay_tree_lookup (&acc_dev->mem_map, &k);
>> +            if (str)
>> +              {
>> +                if (finalize)
>> +                  {
>> +                    str->refcount -= str->virtual_refcount;
>> +                    str->virtual_refcount = 0;
>> +                  }
>> +                if (str->virtual_refcount > 0)
>> +                  {
>> +                    str->refcount--;
>> +                    str->virtual_refcount--;
>> +                  }
>> +                else if (str->refcount > 0)
>> +                  str->refcount--;
>> +                if (str->refcount == 0)
>> +                  gomp_remove_var_async (acc_dev, str, aq);
>> +              }
>> +          }
>> +        i += elems;
>> +      }
>> +      break;
>> +
>>      default:
>>        gomp_fatal (">>>> goacc_exit_data_internal UNHANDLED kind 0x%.2x",
>>                        kind);
>
> ... into an "empty 'case GOMP_MAP_STRUCT:' as a no-op [...]

> I suppose we still need to unmap the "'GOMP_MAP_STRUCT' components", but
> can do that individually, outside of the 'GOMP_MAP_STRUCT' context.

You've answered my question with the
<http://mid.mail-archive.com/6538c388d22e016bd01737e8c2f80437f432c70f.1591276990.git.julian@codesourcery.com>
patch submission; I've now pushed "[OpenACC 'exit data'] Simplify
'GOMP_MAP_STRUCT' handling" to master branch in commit
1809628fcff6f512206efd0ae03a3faccc4096f2, and releases/gcc-10 branch in
commit 96d8d068f3d6f3efebdca65f0a7b86e0609ee66f, see attached.


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

From 96d8d068f3d6f3efebdca65f0a7b86e0609ee66f Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Wed, 20 May 2020 10:53:33 +0200
Subject: [PATCH] [OpenACC 'exit data'] Simplify 'GOMP_MAP_STRUCT' handling

	libgomp/
	* oacc-mem.c (goacc_exit_data_internal) <GOMP_MAP_STRUCT>:
	Simplify.

Co-Authored-By: Julian Brown <julian@codesourcery.com>
(cherry picked from commit 1809628fcff6f512206efd0ae03a3faccc4096f2)
---
 libgomp/oacc-mem.c | 83 ++--------------------------------------------
 1 file changed, 3 insertions(+), 80 deletions(-)

diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c
index 11419e692aa2..1e3685a073da 100644
--- a/libgomp/oacc-mem.c
+++ b/libgomp/oacc-mem.c
@@ -1180,86 +1180,9 @@  goacc_exit_data_internal (struct gomp_device_descr *acc_dev, size_t mapnum,
 	  break;
 
 	case GOMP_MAP_STRUCT:
-	  {
-	    int elems = sizes[i];
-	    for (int j = 1; j <= elems; j++)
-	      {
-		assert (i + j < mapnum);
-
-		kind = kinds[i + j] & 0xff;
-
-		finalize = false;
-		if (kind == GOMP_MAP_FORCE_FROM
-		    || kind == GOMP_MAP_DELETE
-		    || kind == GOMP_MAP_FORCE_DETACH)
-		  finalize = true;
-
-		copyfrom = false;
-		if (kind == GOMP_MAP_FROM
-		    || kind == GOMP_MAP_FORCE_FROM
-		    || kind == GOMP_MAP_ALWAYS_FROM)
-		  copyfrom = true;
-
-		struct splay_tree_key_s k;
-		k.host_start = (uintptr_t) hostaddrs[i + j];
-		k.host_end = k.host_start + sizes[i + j];
-		splay_tree_key str;
-		str = splay_tree_lookup (&acc_dev->mem_map, &k);
-		if (str)
-		  {
-		    if (finalize)
-		      {
-			if (str->refcount != REFCOUNT_INFINITY)
-			  str->refcount -= str->virtual_refcount;
-			str->virtual_refcount = 0;
-		      }
-		    if (str->virtual_refcount > 0)
-		      {
-			if (str->refcount != REFCOUNT_INFINITY)
-			  str->refcount--;
-			str->virtual_refcount--;
-		      }
-		    else if (str->refcount > 0
-			     && str->refcount != REFCOUNT_INFINITY)
-		      str->refcount--;
-
-		    if (copyfrom
-			&& (kind != GOMP_MAP_FROM || str->refcount == 0))
-		      gomp_copy_dev2host (acc_dev, aq, (void *) k.host_start,
-					  (void *) (str->tgt->tgt_start
-						    + str->tgt_offset
-						    + k.host_start
-						    - str->host_start),
-					  k.host_end - k.host_start);
-
-		    if (str->refcount == 0)
-		      {
-			if (aq)
-			  /* TODO We can't do the 'is_tgt_unmapped' checking --
-			     see the 'gomp_unref_tgt' comment in
-			     <http://mid.mail-archive.com/878snl36eu.fsf@euler.schwinge.homeip.net>;
-			     PR92881.  */
-			  gomp_remove_var_async (acc_dev, str, aq);
-			else
-			  {
-			    size_t 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 'GOMP_MAP_STRUCT' mapping
-			       with multiple members), fall back to skipping
-			       the test.  */
-			    for (size_t l_i = 0; l_i < str->tgt->list_count; ++l_i)
-			      if (str->tgt->list[l_i].key)
-				++num_mappings;
-			    bool is_tgt_unmapped = gomp_remove_var (acc_dev, str);
-			    assert (is_tgt_unmapped || num_mappings > 1);
-			  }
-		      }
-		  }
-	      }
-	    i += elems;
-	  }
+	  /* Skip the 'GOMP_MAP_STRUCT' itself, and use the regular processing
+	     for all its entries.  TODO: don't generate these no-op
+	     'GOMP_MAP_STRUCT's.  */
 	  break;
 
 	default:
-- 
2.26.2