On 2018/12/7 1:43 AM, Jakub Jelinek wrote:
On Thu, Dec 06, 2018 at 06:21:16PM +0100, Thomas Schwinge wrote:
On Tue, 25 Sep 2018 21:11:24 +0800, Chung-Lin Tang <chunglin_t...@mentor.com> 
wrote:
Hi Jakub,
This part has changes to 'struct goacc_asyncqueue*' arguments to various
memory copying/mapping functions. To lessen the amount of code changes new 
'gomp_map/unmap_vars_async'
functions names are used (with the non-async original names defined with the 
asyncqueue==NULL).

Is that the way you'd like this to be done, or should instead that
"struct goacc_asyncqueue *aq" parameter be added/passed through all the
existing functions?  (The latter would be my preference, actually.)

I'd prefer not to increase the amount of arguments where possible, because
many of the functions already have more arguments than can be passed in
registers.  Could it be e.g. added into gomp_coalesce_buf which is already
passed around?

Another option would be to use always_inline as C template if the OpenMP and
OpenACC needs diverge too much, then have simply small wrappers that just
call the always_inline function, in one case with the argument NULL or other
constant, in another one with whatever it has been called with.

        Jakub

I have revised the patch to make both gomp_[un]map_vars and 
gomp_[un]map_vars_async
point to gomp_[un]map_vars_internal, which is static always_inline. This should
alleviate that part of the concerns.

Thanks,
Chung-Lin

Index: libgomp/target.c
===================================================================
--- libgomp/target.c    (revision 266973)
+++ libgomp/target.c    (working copy)
@@ -177,6 +177,22 @@ gomp_device_copy (struct gomp_device_descr *device
     }
 }
 
+static inline void
+goacc_device_copy_async (struct gomp_device_descr *devicep,
+                        bool (*copy_func) (int, void *, const void *, size_t,
+                                           struct goacc_asyncqueue *),
+                        const char *dst, void *dstaddr,
+                        const char *src, const void *srcaddr,
+                        size_t size, struct goacc_asyncqueue *aq)
+{
+  if (!copy_func (devicep->target_id, dstaddr, srcaddr, size, aq))
+    {
+      gomp_mutex_unlock (&devicep->lock);
+      gomp_fatal ("Copying of %s object [%p..%p) to %s object [%p..%p) failed",
+                 src, srcaddr, srcaddr + size, dst, dstaddr, dstaddr + size);
+    }
+}
+
 /* Infrastructure for coalescing adjacent or nearly adjacent (in device 
addresses)
    host to device memory transfers.  */
 
@@ -263,8 +279,9 @@ gomp_to_device_kind_p (int kind)
     }
 }
 
-static void
+attribute_hidden void
 gomp_copy_host2dev (struct gomp_device_descr *devicep,
+                   struct goacc_asyncqueue *aq,
                    void *d, const void *h, size_t sz,
                    struct gomp_coalesce_buf *cbuf)
 {
@@ -293,14 +310,23 @@ gomp_copy_host2dev (struct gomp_device_descr *devi
            }
        }
     }
-  gomp_device_copy (devicep, devicep->host2dev_func, "dev", d, "host", h, sz);
+  if (aq)
+    goacc_device_copy_async (devicep, devicep->openacc.async.host2dev_func,
+                            "dev", d, "host", h, sz, aq);
+  else
+    gomp_device_copy (devicep, devicep->host2dev_func, "dev", d, "host", h, 
sz);
 }
 
-static void
+attribute_hidden void
 gomp_copy_dev2host (struct gomp_device_descr *devicep,
+                   struct goacc_asyncqueue *aq,
                    void *h, const void *d, size_t sz)
 {
-  gomp_device_copy (devicep, devicep->dev2host_func, "host", h, "dev", d, sz);
+  if (aq)
+    goacc_device_copy_async (devicep, devicep->openacc.async.dev2host_func,
+                            "host", h, "dev", d, sz, aq);
+  else
+    gomp_device_copy (devicep, devicep->dev2host_func, "host", h, "dev", d, 
sz);
 }
 
 static void
@@ -318,7 +344,8 @@ gomp_free_device_memory (struct gomp_device_descr
    Helper function of gomp_map_vars.  */
 
 static inline void
-gomp_map_vars_existing (struct gomp_device_descr *devicep, splay_tree_key oldn,
+gomp_map_vars_existing (struct gomp_device_descr *devicep,
+                       struct goacc_asyncqueue *aq, splay_tree_key oldn,
                        splay_tree_key newn, struct target_var_desc *tgt_var,
                        unsigned char kind, struct gomp_coalesce_buf *cbuf)
 {
@@ -340,7 +367,7 @@ static inline void
     }
 
   if (GOMP_MAP_ALWAYS_TO_P (kind))
-    gomp_copy_host2dev (devicep,
+    gomp_copy_host2dev (devicep, aq,
                        (void *) (oldn->tgt->tgt_start + oldn->tgt_offset
                                  + newn->host_start - oldn->host_start),
                        (void *) newn->host_start,
@@ -358,8 +385,8 @@ get_kind (bool short_mapkind, void *kinds, int idx
 }
 
 static void
-gomp_map_pointer (struct target_mem_desc *tgt, uintptr_t host_ptr,
-                 uintptr_t target_offset, uintptr_t bias,
+gomp_map_pointer (struct target_mem_desc *tgt, struct goacc_asyncqueue *aq,
+                 uintptr_t host_ptr, uintptr_t target_offset, uintptr_t bias,
                  struct gomp_coalesce_buf *cbuf)
 {
   struct gomp_device_descr *devicep = tgt->device_descr;
@@ -370,7 +397,7 @@ static void
   if (cur_node.host_start == (uintptr_t) NULL)
     {
       cur_node.tgt_offset = (uintptr_t) NULL;
-      gomp_copy_host2dev (devicep,
+      gomp_copy_host2dev (devicep, aq,
                          (void *) (tgt->tgt_start + target_offset),
                          (void *) &cur_node.tgt_offset,
                          sizeof (void *), cbuf);
@@ -392,12 +419,13 @@ static void
      array section.  Now subtract bias to get what we want
      to initialize the pointer with.  */
   cur_node.tgt_offset -= bias;
-  gomp_copy_host2dev (devicep, (void *) (tgt->tgt_start + target_offset),
+  gomp_copy_host2dev (devicep, aq, (void *) (tgt->tgt_start + target_offset),
                      (void *) &cur_node.tgt_offset, sizeof (void *), cbuf);
 }
 
 static void
-gomp_map_fields_existing (struct target_mem_desc *tgt, splay_tree_key n,
+gomp_map_fields_existing (struct target_mem_desc *tgt,
+                         struct goacc_asyncqueue *aq, splay_tree_key n,
                          size_t first, size_t i, void **hostaddrs,
                          size_t *sizes, void *kinds,
                          struct gomp_coalesce_buf *cbuf)
@@ -417,7 +445,7 @@ static void
       && n2->tgt == n->tgt
       && n2->host_start - n->host_start == n2->tgt_offset - n->tgt_offset)
     {
-      gomp_map_vars_existing (devicep, n2, &cur_node,
+      gomp_map_vars_existing (devicep, aq, n2, &cur_node,
                              &tgt->list[i], kind & typemask, cbuf);
       return;
     }
@@ -433,8 +461,8 @@ static void
              && n2->host_start - n->host_start
                 == n2->tgt_offset - n->tgt_offset)
            {
-             gomp_map_vars_existing (devicep, n2, &cur_node, &tgt->list[i],
-                                     kind & typemask, cbuf);
+             gomp_map_vars_existing (devicep, aq, n2, &cur_node,
+                                     &tgt->list[i], kind & typemask, cbuf);
              return;
            }
        }
@@ -445,7 +473,7 @@ static void
          && n2->tgt == n->tgt
          && n2->host_start - n->host_start == n2->tgt_offset - n->tgt_offset)
        {
-         gomp_map_vars_existing (devicep, n2, &cur_node, &tgt->list[i],
+         gomp_map_vars_existing (devicep, aq, n2, &cur_node, &tgt->list[i],
                                  kind & typemask, cbuf);
          return;
        }
@@ -477,10 +505,12 @@ gomp_map_val (struct target_mem_desc *tgt, void **
   return tgt->tgt_start + tgt->list[i].offset;
 }
 
-attribute_hidden struct target_mem_desc *
-gomp_map_vars (struct gomp_device_descr *devicep, size_t mapnum,
-              void **hostaddrs, void **devaddrs, size_t *sizes, void *kinds,
-              bool short_mapkind, enum gomp_map_vars_kind pragma_kind)
+static inline __attribute__((always_inline)) struct target_mem_desc *
+gomp_map_vars_internal (struct gomp_device_descr *devicep,
+                       struct goacc_asyncqueue *aq, size_t mapnum,
+                       void **hostaddrs, void **devaddrs, size_t *sizes,
+                       void *kinds, bool short_mapkind,
+                       enum gomp_map_vars_kind pragma_kind)
 {
   size_t i, tgt_align, tgt_size, not_found_cnt = 0;
   bool has_firstprivate = false;
@@ -594,7 +624,7 @@ gomp_map_val (struct target_mem_desc *tgt, void **
              continue;
            }
          for (i = first; i <= last; i++)
-           gomp_map_fields_existing (tgt, n, first, i, hostaddrs,
+           gomp_map_fields_existing (tgt, aq, n, first, i, hostaddrs,
                                      sizes, kinds, NULL);
          i--;
          continue;
@@ -639,7 +669,7 @@ gomp_map_val (struct target_mem_desc *tgt, void **
       else
        n = splay_tree_lookup (mem_map, &cur_node);
       if (n && n->refcount != REFCOUNT_LINK)
-       gomp_map_vars_existing (devicep, n, &cur_node, &tgt->list[i],
+       gomp_map_vars_existing (devicep, aq, n, &cur_node, &tgt->list[i],
                                kind & typemask, NULL);
       else
        {
@@ -750,7 +780,7 @@ gomp_map_val (struct target_mem_desc *tgt, void **
                tgt_size = (tgt_size + align - 1) & ~(align - 1);
                tgt->list[i].offset = tgt_size;
                len = sizes[i];
-               gomp_copy_host2dev (devicep,
+               gomp_copy_host2dev (devicep, aq,
                                    (void *) (tgt->tgt_start + tgt_size),
                                    (void *) hostaddrs[i], len, cbufp);
                tgt_size += len;
@@ -784,7 +814,7 @@ gomp_map_val (struct target_mem_desc *tgt, void **
                    continue;
                  }
                for (i = first; i <= last; i++)
-                 gomp_map_fields_existing (tgt, n, first, i, hostaddrs,
+                 gomp_map_fields_existing (tgt, aq, n, first, i, hostaddrs,
                                            sizes, kinds, cbufp);
                i--;
                continue;
@@ -804,7 +834,7 @@ gomp_map_val (struct target_mem_desc *tgt, void **
                  cur_node.tgt_offset = gomp_map_val (tgt, hostaddrs, i - 1);
                if (cur_node.tgt_offset)
                  cur_node.tgt_offset -= sizes[i];
-               gomp_copy_host2dev (devicep,
+               gomp_copy_host2dev (devicep, aq,
                                    (void *) (n->tgt->tgt_start
                                              + n->tgt_offset
                                              + cur_node.host_start
@@ -825,7 +855,7 @@ gomp_map_val (struct target_mem_desc *tgt, void **
              k->host_end = k->host_start + sizeof (void *);
            splay_tree_key n = splay_tree_lookup (mem_map, k);
            if (n && n->refcount != REFCOUNT_LINK)
-             gomp_map_vars_existing (devicep, n, k, &tgt->list[i],
+             gomp_map_vars_existing (devicep, aq, n, k, &tgt->list[i],
                                      kind & typemask, cbufp);
            else
              {
@@ -878,7 +908,7 @@ gomp_map_val (struct target_mem_desc *tgt, void **
                  case GOMP_MAP_FORCE_TOFROM:
                  case GOMP_MAP_ALWAYS_TO:
                  case GOMP_MAP_ALWAYS_TOFROM:
-                   gomp_copy_host2dev (devicep,
+                   gomp_copy_host2dev (devicep, aq,
                                        (void *) (tgt->tgt_start
                                                  + k->tgt_offset),
                                        (void *) k->host_start,
@@ -885,11 +915,12 @@ gomp_map_val (struct target_mem_desc *tgt, void **
                                        k->host_end - k->host_start, cbufp);
                    break;
                  case GOMP_MAP_POINTER:
-                   gomp_map_pointer (tgt, (uintptr_t) *(void **) k->host_start,
+                   gomp_map_pointer (tgt, aq,
+                                     (uintptr_t) *(void **) k->host_start,
                                      k->tgt_offset, sizes[i], cbufp);
                    break;
                  case GOMP_MAP_TO_PSET:
-                   gomp_copy_host2dev (devicep,
+                   gomp_copy_host2dev (devicep, aq,
                                        (void *) (tgt->tgt_start
                                                  + k->tgt_offset),
                                        (void *) k->host_start,
@@ -911,7 +942,7 @@ gomp_map_val (struct target_mem_desc *tgt, void **
                          tgt->list[j].always_copy_from = false;
                          if (k->refcount != REFCOUNT_INFINITY)
                            k->refcount++;
-                         gomp_map_pointer (tgt,
+                         gomp_map_pointer (tgt, aq,
                                            (uintptr_t) *(void **) hostaddrs[j],
                                            k->tgt_offset
                                            + ((uintptr_t) hostaddrs[j]
@@ -940,7 +971,7 @@ gomp_map_val (struct target_mem_desc *tgt, void **
                    break;
                  case GOMP_MAP_FORCE_DEVICEPTR:
                    assert (k->host_end - k->host_start == sizeof (void *));
-                   gomp_copy_host2dev (devicep,
+                   gomp_copy_host2dev (devicep, aq,
                                        (void *) (tgt->tgt_start
                                                  + k->tgt_offset),
                                        (void *) k->host_start,
@@ -959,7 +990,7 @@ gomp_map_val (struct target_mem_desc *tgt, void **
                    void *tgt_addr = (void *) (tgt->tgt_start + k->tgt_offset);
                    /* We intentionally do not use coalescing here, as it's not
                       data allocated by the current call to this function.  */
-                   gomp_copy_host2dev (devicep, (void *) n->tgt_offset,
+                   gomp_copy_host2dev (devicep, aq, (void *) n->tgt_offset,
                                        &tgt_addr, sizeof (void *), NULL);
                  }
                array++;
@@ -972,7 +1003,7 @@ gomp_map_val (struct target_mem_desc *tgt, void **
       for (i = 0; i < mapnum; i++)
        {
          cur_node.tgt_offset = gomp_map_val (tgt, hostaddrs, i);
-         gomp_copy_host2dev (devicep,
+         gomp_copy_host2dev (devicep, aq,
                              (void *) (tgt->tgt_start + i * sizeof (void *)),
                              (void *) &cur_node.tgt_offset, sizeof (void *),
                              cbufp);
@@ -983,7 +1014,8 @@ gomp_map_val (struct target_mem_desc *tgt, void **
     {
       long c = 0;
       for (c = 0; c < cbuf.chunk_cnt; ++c)
-       gomp_copy_host2dev (devicep, (void *) (tgt->tgt_start + cbuf.chunks[2 * 
c]),
+       gomp_copy_host2dev (devicep, aq,
+                           (void *) (tgt->tgt_start + cbuf.chunks[2 * c]),
                            (char *) cbuf.buf + (cbuf.chunks[2 * c] - 
cbuf.chunks[0]),
                            cbuf.chunks[2 * c + 1] - cbuf.chunks[2 * c], NULL);
       free (cbuf.buf);
@@ -1002,7 +1034,27 @@ gomp_map_val (struct target_mem_desc *tgt, void **
   return tgt;
 }
 
-static void
+attribute_hidden struct target_mem_desc *
+gomp_map_vars (struct gomp_device_descr *devicep, size_t mapnum,
+              void **hostaddrs, void **devaddrs, size_t *sizes, void *kinds,
+              bool short_mapkind, enum gomp_map_vars_kind pragma_kind)
+{
+  return gomp_map_vars_internal (devicep, NULL, mapnum, hostaddrs, devaddrs,
+                                sizes, kinds, short_mapkind, pragma_kind);
+}
+
+attribute_hidden struct target_mem_desc *
+gomp_map_vars_async (struct gomp_device_descr *devicep,
+                    struct goacc_asyncqueue *aq, size_t mapnum,
+                    void **hostaddrs, void **devaddrs, size_t *sizes,
+                    void *kinds, bool short_mapkind,
+                    enum gomp_map_vars_kind pragma_kind)
+{
+  return gomp_map_vars_internal (devicep, aq, mapnum, hostaddrs, devaddrs,
+                                sizes, kinds, short_mapkind, pragma_kind);
+}
+
+attribute_hidden void
 gomp_unmap_tgt (struct target_mem_desc *tgt)
 {
   /* Deallocate on target the tgt->tgt_start .. tgt->tgt_end region.  */
@@ -1034,8 +1086,9 @@ gomp_remove_var (struct gomp_device_descr *devicep
    variables back from device to host: if it is false, it is assumed that this
    has been done already.  */
 
-attribute_hidden void
-gomp_unmap_vars (struct target_mem_desc *tgt, bool do_copyfrom)
+static inline __attribute__((always_inline)) void
+gomp_unmap_vars_internal (struct target_mem_desc *tgt, bool do_copyfrom,
+                         struct goacc_asyncqueue *aq)
 {
   struct gomp_device_descr *devicep = tgt->device_descr;
 
@@ -1072,7 +1125,7 @@ gomp_remove_var (struct gomp_device_descr *devicep
 
       if ((do_unmap && do_copyfrom && tgt->list[i].copy_from)
          || tgt->list[i].always_copy_from)
-       gomp_copy_dev2host (devicep,
+       gomp_copy_dev2host (devicep, aq,
                            (void *) (k->host_start + tgt->list[i].offset),
                            (void *) (k->tgt->tgt_start + k->tgt_offset
                                      + tgt->list[i].offset),
@@ -1089,6 +1142,19 @@ gomp_remove_var (struct gomp_device_descr *devicep
   gomp_mutex_unlock (&devicep->lock);
 }
 
+attribute_hidden void
+gomp_unmap_vars (struct target_mem_desc *tgt, bool do_copyfrom)
+{
+  gomp_unmap_vars_internal (tgt, do_copyfrom, NULL);
+}
+
+attribute_hidden void
+gomp_unmap_vars_async (struct target_mem_desc *tgt, bool do_copyfrom,
+                      struct goacc_asyncqueue *aq)
+{
+  gomp_unmap_vars_internal (tgt, do_copyfrom, aq);
+}
+
 static void
 gomp_update (struct gomp_device_descr *devicep, size_t mapnum, void 
**hostaddrs,
             size_t *sizes, void *kinds, bool short_mapkind)
@@ -1138,9 +1204,10 @@ gomp_update (struct gomp_device_descr *devicep, si
            size_t size = cur_node.host_end - cur_node.host_start;
 
            if (GOMP_MAP_COPY_TO_P (kind & typemask))
-             gomp_copy_host2dev (devicep, devaddr, hostaddr, size, NULL);
+             gomp_copy_host2dev (devicep, NULL, devaddr, hostaddr, size,
+                                 NULL);
            if (GOMP_MAP_COPY_FROM_P (kind & typemask))
-             gomp_copy_dev2host (devicep, hostaddr, devaddr, size);
+             gomp_copy_dev2host (devicep, NULL, hostaddr, devaddr, size);
          }
       }
   gomp_mutex_unlock (&devicep->lock);
@@ -1433,9 +1500,21 @@ gomp_init_device (struct gomp_device_descr *device
                                   false);
     }
 
+  /* Initialize OpenACC asynchronous queues.  */
+  goacc_init_asyncqueues (devicep);
+
   devicep->state = GOMP_DEVICE_INITIALIZED;
 }
 
+attribute_hidden bool
+gomp_fini_device (struct gomp_device_descr *devicep)
+{
+  bool ret = goacc_fini_asyncqueues (devicep);
+  ret &= devicep->fini_device_func (devicep->target_id);
+  devicep->state = GOMP_DEVICE_FINALIZED;
+  return ret;
+}
+
 attribute_hidden void
 gomp_unload_device (struct gomp_device_descr *devicep)
 {
@@ -1944,7 +2023,7 @@ gomp_exit_data (struct gomp_device_descr *devicep,
 
          if ((kind == GOMP_MAP_FROM && k->refcount == 0)
              || kind == GOMP_MAP_ALWAYS_FROM)
-           gomp_copy_dev2host (devicep, (void *) cur_node.host_start,
+           gomp_copy_dev2host (devicep, NULL, (void *) cur_node.host_start,
                                (void *) (k->tgt->tgt_start + k->tgt_offset
                                          + cur_node.host_start
                                          - k->host_start),
@@ -2626,20 +2705,20 @@ gomp_load_plugin_for_device (struct gomp_device_de
   if (device->capabilities & GOMP_OFFLOAD_CAP_OPENACC_200)
     {
       if (!DLSYM_OPT (openacc.exec, openacc_exec)
-         || !DLSYM_OPT (openacc.register_async_cleanup,
-                        openacc_register_async_cleanup)
-         || !DLSYM_OPT (openacc.async_test, openacc_async_test)
-         || !DLSYM_OPT (openacc.async_test_all, openacc_async_test_all)
-         || !DLSYM_OPT (openacc.async_wait, openacc_async_wait)
-         || !DLSYM_OPT (openacc.async_wait_async, openacc_async_wait_async)
-         || !DLSYM_OPT (openacc.async_wait_all, openacc_async_wait_all)
-         || !DLSYM_OPT (openacc.async_wait_all_async,
-                        openacc_async_wait_all_async)
-         || !DLSYM_OPT (openacc.async_set_async, openacc_async_set_async)
          || !DLSYM_OPT (openacc.create_thread_data,
                         openacc_create_thread_data)
          || !DLSYM_OPT (openacc.destroy_thread_data,
-                        openacc_destroy_thread_data))
+                        openacc_destroy_thread_data)
+         || !DLSYM_OPT (openacc.async.construct, openacc_async_construct)
+         || !DLSYM_OPT (openacc.async.destruct, openacc_async_destruct)
+         || !DLSYM_OPT (openacc.async.test, openacc_async_test)
+         || !DLSYM_OPT (openacc.async.synchronize, openacc_async_synchronize)
+         || !DLSYM_OPT (openacc.async.serialize, openacc_async_serialize)
+         || !DLSYM_OPT (openacc.async.queue_callback,
+                        openacc_async_queue_callback)
+         || !DLSYM_OPT (openacc.async.exec, openacc_async_exec)
+         || !DLSYM_OPT (openacc.async.dev2host, openacc_async_dev2host)
+         || !DLSYM_OPT (openacc.async.host2dev, openacc_async_host2dev))
        {
          /* Require all the OpenACC handlers if we have
             GOMP_OFFLOAD_CAP_OPENACC_200.  */
@@ -2690,10 +2769,7 @@ gomp_target_fini (void)
       struct gomp_device_descr *devicep = &devices[i];
       gomp_mutex_lock (&devicep->lock);
       if (devicep->state == GOMP_DEVICE_INITIALIZED)
-       {
-         ret = devicep->fini_device_func (devicep->target_id);
-         devicep->state = GOMP_DEVICE_FINALIZED;
-       }
+       ret = gomp_fini_device (devicep);
       gomp_mutex_unlock (&devicep->lock);
       if (!ret)
        gomp_fatal ("device finalization failed");

Reply via email to