Hi Julian!
On 2019-12-17T22:03:47-0800, Julian Brown <jul...@codesourcery.com> wrote:
> This part contains the libgomp runtime support for the GOMP_MAP_ATTACH and
> GOMP_MAP_DETACH mapping kinds
> --- a/libgomp/oacc-mem.c
> +++ b/libgomp/oacc-mem.c
> @@ -1018,6 +1033,33 @@ goacc_exit_data_internal (struct gomp_device_descr
> *acc_dev, size_t mapnum,
> {
> gomp_mutex_lock (&acc_dev->lock);
>
> + /* Handle "detach" before copyback/deletion of mapped data. */
> + for (size_t i = 0; i < mapnum; ++i)
> + {
> + unsigned char kind = kinds[i] & 0xff;
> + switch (kind)
> + {
> + case GOMP_MAP_DETACH:
> + case GOMP_MAP_FORCE_DETACH:
> + {
> + struct splay_tree_key_s cur_node;
> + uintptr_t hostaddr = (uintptr_t) hostaddrs[i];
> + cur_node.host_start = hostaddr;
> + cur_node.host_end = cur_node.host_start + sizeof (void *);
> + splay_tree_key n
> + = splay_tree_lookup (&acc_dev->mem_map, &cur_node);
> +
> + if (n == NULL)
> + gomp_fatal ("struct not mapped for detach operation");
> +
> + gomp_detach_pointer (acc_dev, aq, n, hostaddr, finalize, NULL);
> + }
> + break;
> + default:
> + ;
> + }
> + }
> +
> for (size_t i = 0; i < mapnum; ++i)
> {
> unsigned char kind = kinds[i] & 0xff;
What's the reason that we're not instead emitting any 'detach' mappings
in the expected order (that is, first), which would avoid this
double-traversal here? Given that 'mapnum' typically won't exceed the
dozens, the code we now got doesn't have a big run-time cost, of course,
but it's still a bit ugly, conceptually, for no apparent reason, unless
I'm confused?
> --- a/libgomp/target.c
> +++ b/libgomp/target.c
> @@ -1534,6 +1571,18 @@ gomp_unmap_vars_internal (struct target_mem_desc *tgt,
> bool do_copyfrom,
> }
>
> size_t i;
> +
> + /* We must perform detachments before any copies back to the host. */
> + for (i = 0; i < tgt->list_count; i++)
> + {
> + splay_tree_key k = tgt->list[i].key;
> +
> + if (k != NULL && tgt->list[i].do_detach)
> + gomp_detach_pointer (devicep, aq, k, tgt->list[i].key->host_start
> + + tgt->list[i].offset,
> + k->refcount == 1, NULL);
> + }
> +
> for (i = 0; i < tgt->list_count; i++)
> {
> splay_tree_key k = tgt->list[i].key;
Similarly, isn't it sufficient if any 'detach' for 'tgt->list[i].key' is
done (directly) before the copy-back/unmap of 'tgt->list[i].key', again
to avoid the double-traversal here?
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
