Hi, this patch reworks some of the way that asynchronous copyouts are implemented for OpenACC in libgomp.
Before this patch, we had a somewhat confusing way of implementing this by having two refcounts for each mapping: refcount and async_refcount, which I never got working again after the last wave of async regressions showed up. So this patch implements what I believe to be a simplification: async_refcount is removed, and instead of trying to queue the async copyouts during unmapping we actually do that during the plugin event handling. This requires a addition of the async stream integer as an argument to the register_async_cleanup plugin hook, but overall I think this should be more elegant than before. This patch fixes the libgomp.oacc-c-c++-common/asyncwait-1.c regression. It also fixed data-[23].c regressions before, but some other recent check-in happened to already fixed those. Tested without regressions, is this okay for trunk? Thanks, Chung-Lin 2015-11-24 Chung-Lin Tang <clt...@codesourcery.com> * oacc-plugin.h (GOMP_PLUGIN_async_unmap_vars): Add int parameter. * oacc-plugin.c (GOMP_PLUGIN_async_unmap_vars): Add 'int async' parameter, use to set async stream around call to gomp_unmap_vars, call gomp_unmap_vars() with 'do_copyfrom' set to true. * plugin/plugin-nvptx.c (struct ptx_event): Add 'int val' field. (event_gc): Adjust event handling loop, collect PTX_EVT_ASYNC_CLEANUP events and call GOMP_PLUGIN_async_unmap_vars() for each of them. (event_add): Add int parameter, initialize 'val' field when adding new ptx_event struct. (nvptx_evec): Adjust event_add() call arguments. (nvptx_host2dev): Likewise. (nvptx_dev2host): Likewise. (nvptx_wait_async): Likewise. (nvptx_wait_all_async): Likewise. (GOMP_OFFLOAD_openacc_register_async_cleanup): Add async parameter, pass to event_add() call. * oacc-host.c (host_openacc_register_async_cleanup): Add 'int async' parameter. * oacc-mem.c (gomp_acc_remove_pointer): Adjust async case to call openacc.register_async_cleanup_func() hook. * oacc-parallel.c (GOACC_parallel_keyed): Likewise. * target.c (gomp_copy_from_async): Delete function. (gomp_map_vars): Remove async_refcount. (gomp_unmap_vars): Likewise. (gomp_load_image_to_device): Likewise. (omp_target_associate_ptr): Likewise. * libgomp.h (struct splay_tree_key_s): Remove async_refcount. (acc_dispatch_t.register_async_cleanup_func): Add int parameter. (gomp_copy_from_async): Remove.
Index: plugin/plugin-nvptx.c =================================================================== --- plugin/plugin-nvptx.c (revision 230796) +++ plugin/plugin-nvptx.c (working copy) @@ -310,6 +310,7 @@ struct ptx_event int type; void *addr; int ord; + int val; struct ptx_event *next; }; @@ -786,6 +787,7 @@ static void event_gc (bool memmap_lockable) { struct ptx_event *ptx_event = ptx_events; + struct ptx_event *async_cleanups = NULL; struct nvptx_thread *nvthd = nvptx_thread (); pthread_mutex_lock (&ptx_event_lock); @@ -803,6 +805,7 @@ event_gc (bool memmap_lockable) r = cuEventQuery (*e->evt); if (r == CUDA_SUCCESS) { + bool append_async = false; CUevent *te; te = e->evt; @@ -827,7 +830,7 @@ event_gc (bool memmap_lockable) if (!memmap_lockable) continue; - GOMP_PLUGIN_async_unmap_vars (e->addr); + append_async = true; } break; } @@ -835,6 +838,7 @@ event_gc (bool memmap_lockable) cuEventDestroy (*te); free ((void *)te); + /* Unlink 'e' from ptx_events list. */ if (ptx_events == e) ptx_events = ptx_events->next; else @@ -845,15 +849,31 @@ event_gc (bool memmap_lockable) e_->next = e_->next->next; } - free (e); + if (append_async) + { + e->next = async_cleanups; + async_cleanups = e; + } + else + free (e); } } pthread_mutex_unlock (&ptx_event_lock); + + /* We have to do these here, after ptx_event_lock is released. */ + while (async_cleanups) + { + struct ptx_event *e = async_cleanups; + async_cleanups = async_cleanups->next; + + GOMP_PLUGIN_async_unmap_vars (e->addr, e->val); + free (e); + } } static void -event_add (enum ptx_event_type type, CUevent *e, void *h) +event_add (enum ptx_event_type type, CUevent *e, void *h, int val) { struct ptx_event *ptx_event; struct nvptx_thread *nvthd = nvptx_thread (); @@ -866,6 +886,7 @@ static void ptx_event->evt = e; ptx_event->addr = h; ptx_event->ord = nvthd->ptx_dev->ord; + ptx_event->val = val; pthread_mutex_lock (&ptx_event_lock); @@ -966,7 +987,7 @@ nvptx_exec (void (*fn), size_t mapnum, void **host if (r != CUDA_SUCCESS) GOMP_PLUGIN_fatal ("cuEventRecord error: %s", cuda_error (r)); - event_add (PTX_EVT_KNL, e, (void *)dev_str); + event_add (PTX_EVT_KNL, e, (void *)dev_str, 0); } #else r = cuCtxSynchronize (); @@ -1073,7 +1094,7 @@ nvptx_host2dev (void *d, const void *h, size_t s) if (r != CUDA_SUCCESS) GOMP_PLUGIN_fatal ("cuEventRecord error: %s", cuda_error (r)); - event_add (PTX_EVT_MEM, e, (void *)h); + event_add (PTX_EVT_MEM, e, (void *)h, 0); } else #endif @@ -1138,7 +1159,7 @@ nvptx_dev2host (void *h, const void *d, size_t s) if (r != CUDA_SUCCESS) GOMP_PLUGIN_fatal ("cuEventRecord error: %s", cuda_error (r)); - event_add (PTX_EVT_MEM, e, (void *)h); + event_add (PTX_EVT_MEM, e, (void *)h, 0); } else #endif @@ -1264,7 +1285,7 @@ nvptx_wait_async (int async1, int async2) if (r != CUDA_SUCCESS) GOMP_PLUGIN_fatal ("cuEventRecord error: %s", cuda_error (r)); - event_add (PTX_EVT_SYNC, e, NULL); + event_add (PTX_EVT_SYNC, e, NULL, 0); r = cuStreamWaitEvent (s2->stream, *e, 0); if (r != CUDA_SUCCESS) @@ -1346,7 +1367,7 @@ nvptx_wait_all_async (int async) if (r != CUDA_SUCCESS) GOMP_PLUGIN_fatal ("cuEventRecord error: %s", cuda_error (r)); - event_add (PTX_EVT_SYNC, e, NULL); + event_add (PTX_EVT_SYNC, e, NULL, 0); r = cuStreamWaitEvent (waiting_stream->stream, *e, 0); if (r != CUDA_SUCCESS) @@ -1658,7 +1679,7 @@ GOMP_OFFLOAD_openacc_parallel (void (*fn) (void *) } void -GOMP_OFFLOAD_openacc_register_async_cleanup (void *targ_mem_desc) +GOMP_OFFLOAD_openacc_register_async_cleanup (void *targ_mem_desc, int async) { CUevent *e; CUresult r; @@ -1674,7 +1695,7 @@ void if (r != CUDA_SUCCESS) GOMP_PLUGIN_fatal ("cuEventRecord error: %s", cuda_error (r)); - event_add (PTX_EVT_ASYNC_CLEANUP, e, targ_mem_desc); + event_add (PTX_EVT_ASYNC_CLEANUP, e, targ_mem_desc, async); } int Index: oacc-mem.c =================================================================== --- oacc-mem.c (revision 230796) +++ oacc-mem.c (working copy) @@ -659,10 +659,7 @@ gomp_acc_remove_pointer (void *h, bool force_copyf if (async < acc_async_noval) gomp_unmap_vars (t, true); else - { - gomp_copy_from_async (t); - acc_dev->openacc.register_async_cleanup_func (t); - } + t->device_descr->openacc.register_async_cleanup_func (t, async); gomp_debug (0, " %s: mappings restored\n", __FUNCTION__); } Index: libgomp.h =================================================================== --- libgomp.h (revision 230796) +++ libgomp.h (working copy) @@ -829,8 +829,6 @@ struct splay_tree_key_s { uintptr_t tgt_offset; /* Reference count. */ uintptr_t refcount; - /* Asynchronous reference count. */ - uintptr_t async_refcount; }; /* The comparison function. */ @@ -864,7 +862,7 @@ typedef struct acc_dispatch_t unsigned *, void *); /* Async cleanup callback registration. */ - void (*register_async_cleanup_func) (void *); + void (*register_async_cleanup_func) (void *, int); /* Asynchronous routines. */ int (*async_test_func) (int); @@ -958,7 +956,6 @@ extern struct target_mem_desc *gomp_map_vars (stru size_t, void **, void **, size_t *, void *, bool, enum gomp_map_vars_kind); -extern void gomp_copy_from_async (struct target_mem_desc *); extern void gomp_unmap_vars (struct target_mem_desc *, bool); extern void gomp_init_device (struct gomp_device_descr *); extern void gomp_free_memmap (struct splay_tree_s *); Index: oacc-plugin.c =================================================================== --- oacc-plugin.c (revision 230796) +++ oacc-plugin.c (working copy) @@ -31,11 +31,14 @@ #include "oacc-int.h" void -GOMP_PLUGIN_async_unmap_vars (void *ptr) +GOMP_PLUGIN_async_unmap_vars (void *ptr, int async) { struct target_mem_desc *tgt = ptr; + struct gomp_device_descr *devicep = tgt->device_descr; - gomp_unmap_vars (tgt, false); + devicep->openacc.async_set_async_func (async); + gomp_unmap_vars (tgt, true); + devicep->openacc.async_set_async_func (acc_async_sync); } /* Return the target-specific part of the TLS data for the current thread. */ Index: oacc-plugin.h =================================================================== --- oacc-plugin.h (revision 230796) +++ oacc-plugin.h (working copy) @@ -27,7 +27,7 @@ #ifndef OACC_PLUGIN_H #define OACC_PLUGIN_H 1 -extern void GOMP_PLUGIN_async_unmap_vars (void *); +extern void GOMP_PLUGIN_async_unmap_vars (void *, int); extern void *GOMP_PLUGIN_acc_thread (void); #endif Index: oacc-host.c =================================================================== --- oacc-host.c (revision 230796) +++ oacc-host.c (working copy) @@ -143,7 +143,8 @@ host_openacc_exec (void (*fn) (void *), } static void -host_openacc_register_async_cleanup (void *targ_mem_desc __attribute__ ((unused))) +host_openacc_register_async_cleanup (void *targ_mem_desc __attribute__ ((unused)), + int async __attribute__ ((unused))) { } Index: target.c =================================================================== --- target.c (revision 230796) +++ target.c (working copy) @@ -644,7 +644,6 @@ gomp_map_vars (struct gomp_device_descr *devicep, tgt->list[i].offset = 0; tgt->list[i].length = k->host_end - k->host_start; k->refcount = 1; - k->async_refcount = 0; tgt->refcount++; array->left = NULL; array->right = NULL; @@ -784,40 +783,6 @@ gomp_unmap_tgt (struct target_mem_desc *tgt) free (tgt); } -/* Decrease the refcount for a set of mapped variables, and queue asychronous - copies from the device back to the host after any work that has been issued. - Because the regions are still "live", increment an asynchronous reference - count to indicate that they should not be unmapped from host-side data - structures until the asynchronous copy has completed. */ - -attribute_hidden void -gomp_copy_from_async (struct target_mem_desc *tgt) -{ - struct gomp_device_descr *devicep = tgt->device_descr; - size_t i; - - gomp_mutex_lock (&devicep->lock); - - for (i = 0; i < tgt->list_count; i++) - if (tgt->list[i].key == NULL) - ; - else if (tgt->list[i].key->refcount > 1) - { - tgt->list[i].key->refcount--; - tgt->list[i].key->async_refcount++; - } - else - { - splay_tree_key k = tgt->list[i].key; - if (tgt->list[i].copy_from) - devicep->dev2host_func (devicep->target_id, (void *) k->host_start, - (void *) (k->tgt->tgt_start + k->tgt_offset), - k->host_end - k->host_start); - } - - gomp_mutex_unlock (&devicep->lock); -} - /* Unmap variables described by TGT. If DO_COPYFROM is true, copy relevant variables back from device to host: if it is false, it is assumed that this has been done already, i.e. by gomp_copy_from_async above. */ @@ -847,13 +812,8 @@ gomp_unmap_vars (struct target_mem_desc *tgt, bool k->refcount--; else if (k->refcount == 1) { - if (k->async_refcount > 0) - k->async_refcount--; - else - { - k->refcount--; - do_unmap = true; - } + k->refcount--; + do_unmap = true; } if ((do_unmap && do_copyfrom && tgt->list[i].copy_from) @@ -995,7 +955,6 @@ gomp_load_image_to_device (struct gomp_device_desc k->tgt = tgt; k->tgt_offset = target_table[i].start; k->refcount = REFCOUNT_INFINITY; - k->async_refcount = 0; array->left = NULL; array->right = NULL; splay_tree_insert (&devicep->mem_map, array); @@ -1020,7 +979,6 @@ gomp_load_image_to_device (struct gomp_device_desc k->tgt = tgt; k->tgt_offset = target_var->start; k->refcount = REFCOUNT_INFINITY; - k->async_refcount = 0; array->left = NULL; array->right = NULL; splay_tree_insert (&devicep->mem_map, array); @@ -2120,7 +2078,6 @@ omp_target_associate_ptr (void *host_ptr, void *de k->tgt = tgt; k->tgt_offset = (uintptr_t) device_ptr + device_offset; k->refcount = REFCOUNT_INFINITY; - k->async_refcount = 0; array->left = NULL; array->right = NULL; splay_tree_insert (&devicep->mem_map, array); Index: oacc-parallel.c =================================================================== --- oacc-parallel.c (revision 230796) +++ oacc-parallel.c (working copy) @@ -182,10 +182,7 @@ GOACC_parallel_keyed (int device, void (*fn) (void if (async < acc_async_noval) gomp_unmap_vars (tgt, true); else - { - gomp_copy_from_async (tgt); - acc_dev->openacc.register_async_cleanup_func (tgt); - } + tgt->device_descr->openacc.register_async_cleanup_func (tgt, async); acc_dev->openacc.async_set_async_func (acc_async_sync); }