Hi Chung-Lin!

While we're all waiting for Tom to comment on this ;-) -- here's another
item I realized:

On 2019-09-10T19:41:59+0800, Chung-Lin Tang <chunglin_t...@mentor.com> wrote:
> The libgomp nvptx plugin changes are also quite contained, with lots of
> now unneeded [...] code deleted (since we no longer first cuAlloc a
> buffer for the argument record before cuLaunchKernel)

It would be nice ;-) -- but unless I'm confused, it's not that simple: we
either have to reject (force host-fallback execution) or keep supporting
"old-style" nvptx offloading code: new-libgomp has to continue to work
with nvptx offloading code once generated by old-GCC.  Possibly even a
mixture of old and new nvptx offloading code, if libraries are involved,
huh!

I have not completely thought that through, but I suppose this could be
addressed by adding a flag to the 'struct nvptx_fn' (or similar) that's
synthesized by nvptx 'mkoffload'?

Maybe if fact the 'enum id_map_flag' machinery that I once added for
'Un-parallelized OpenACC kernels constructs with nvptx offloading: "avoid
offloading"'?  (That's part of og8 commit
2d42fbf7e989e4bb76727b32ef11deb5845d5ab1 -- not present on og9, huh?!)
The 'enum id_map_flag' machinery serves the purpose of transporting
information from the offload compiler to libgomp, which seems what's
needed here?  (But please verify.)

For reference, your proposed changes:

> --- libgomp/plugin/plugin-nvptx.c     (revision 275493)
> +++ libgomp/plugin/plugin-nvptx.c     (working copy)
> @@ -696,16 +696,24 @@ link_ptx (CUmodule *module, const struct targ_ptx_
>  
>  static void
>  nvptx_exec (void (*fn), size_t mapnum, void **hostaddrs, void **devaddrs,
> -         unsigned *dims, void *targ_mem_desc,
> -         CUdeviceptr dp, CUstream stream)
> +         unsigned *dims, CUstream stream)
>  {
>    struct targ_fn_descriptor *targ_fn = (struct targ_fn_descriptor *) fn;
>    CUfunction function;
>    int i;
> -  void *kargs[1];
>    struct nvptx_thread *nvthd = nvptx_thread ();
>    int warp_size = nvthd->ptx_dev->warp_size;
> +  void **kernel_args = NULL;
>  
> +  GOMP_PLUGIN_debug (0, "prepare mappings (mapnum: %u)\n", (unsigned) 
> mapnum);
> +
> +  if (mapnum > 0)
> +    {
> +      kernel_args = alloca (mapnum * sizeof (void *));
> +      for (int i = 0; i < mapnum; i++)
> +     kernel_args[i] = (devaddrs[i] ? &devaddrs[i] : &hostaddrs[i]);
> +    }
> +  
>    function = targ_fn->fn;
>  
>    /* Initialize the launch dimensions.  Typically this is constant,
> @@ -937,11 +945,10 @@ nvptx_exec (void (*fn), size_t mapnum, void **host
>                                           api_info);
>      }
>  
> -  kargs[0] = &dp;
>    CUDA_CALL_ASSERT (cuLaunchKernel, function,
>                   dims[GOMP_DIM_GANG], 1, 1,
>                   dims[GOMP_DIM_VECTOR], dims[GOMP_DIM_WORKER], 1,
> -                 0, stream, kargs, 0);
> +                 0, stream, kernel_args, 0);
>  
>    if (profiling_p)
>      {
> @@ -1350,67 +1357,8 @@ GOMP_OFFLOAD_openacc_exec (void (*fn) (void *), si
>                          void **hostaddrs, void **devaddrs,
>                          unsigned *dims, void *targ_mem_desc)
>  {
> -  GOMP_PLUGIN_debug (0, "  %s: prepare mappings\n", __FUNCTION__);
> +  nvptx_exec (fn, mapnum, hostaddrs, devaddrs, dims, NULL);
>  
> -  struct goacc_thread *thr = GOMP_PLUGIN_goacc_thread ();
> -  acc_prof_info *prof_info = thr->prof_info;
> -  acc_event_info data_event_info;
> -  acc_api_info *api_info = thr->api_info;
> -  bool profiling_p = __builtin_expect (prof_info != NULL, false);
> -
> -  void **hp = NULL;
> -  CUdeviceptr dp = 0;
> -
> -  if (mapnum > 0)
> -    {
> -      size_t s = mapnum * sizeof (void *);
> -      hp = alloca (s);
> -      for (int i = 0; i < mapnum; i++)
> -     hp[i] = (devaddrs[i] ? devaddrs[i] : hostaddrs[i]);
> -      CUDA_CALL_ASSERT (cuMemAlloc, &dp, s);
> -      if (profiling_p)
> -     goacc_profiling_acc_ev_alloc (thr, (void *) dp, s);
> -    }
> -
> -  /* Copy the (device) pointers to arguments to the device (dp and hp might 
> in
> -     fact have the same value on a unified-memory system).  */
> -  if (mapnum > 0)
> -    {
> -      if (profiling_p)
> -     {
> -       prof_info->event_type = acc_ev_enqueue_upload_start;
> -
> -       data_event_info.data_event.event_type = prof_info->event_type;
> -       data_event_info.data_event.valid_bytes
> -         = _ACC_DATA_EVENT_INFO_VALID_BYTES;
> -       data_event_info.data_event.parent_construct
> -         = acc_construct_parallel;
> -       data_event_info.data_event.implicit = 1; /* Always implicit.  */
> -       data_event_info.data_event.tool_info = NULL;
> -       data_event_info.data_event.var_name = NULL;
> -       data_event_info.data_event.bytes = mapnum * sizeof (void *);
> -       data_event_info.data_event.host_ptr = hp;
> -       data_event_info.data_event.device_ptr = (const void *) dp;
> -
> -       api_info->device_api = acc_device_api_cuda;
> -
> -       GOMP_PLUGIN_goacc_profiling_dispatch (prof_info, &data_event_info,
> -                                             api_info);
> -     }
> -      CUDA_CALL_ASSERT (cuMemcpyHtoD, dp, (void *) hp,
> -                     mapnum * sizeof (void *));
> -      if (profiling_p)
> -     {
> -       prof_info->event_type = acc_ev_enqueue_upload_end;
> -       data_event_info.data_event.event_type = prof_info->event_type;
> -       GOMP_PLUGIN_goacc_profiling_dispatch (prof_info, &data_event_info,
> -                                             api_info);
> -     }
> -    }
> -
> -  nvptx_exec (fn, mapnum, hostaddrs, devaddrs, dims, targ_mem_desc,
> -           dp, NULL);
> -
>    CUresult r = CUDA_CALL_NOCHECK (cuStreamSynchronize, NULL);
>    const char *maybe_abort_msg = "(perhaps abort was called)";
>    if (r == CUDA_ERROR_LAUNCH_FAILED)
> @@ -1418,20 +1366,8 @@ GOMP_OFFLOAD_openacc_exec (void (*fn) (void *), si
>                      maybe_abort_msg);
>    else if (r != CUDA_SUCCESS)
>      GOMP_PLUGIN_fatal ("cuStreamSynchronize error: %s", cuda_error (r));
> -
> -  CUDA_CALL_ASSERT (cuMemFree, dp);
> -  if (profiling_p)
> -    goacc_profiling_acc_ev_free (thr, (void *) dp);
>  }
>  
> -static void
> -cuda_free_argmem (void *ptr)
> -{
> -  void **block = (void **) ptr;
> -  nvptx_free (block[0], (struct ptx_device *) block[1]);
> -  free (block);
> -}
> -
>  void
>  GOMP_OFFLOAD_openacc_async_exec (void (*fn) (void *), size_t mapnum,
>                                void **hostaddrs, void **devaddrs,
> @@ -1438,78 +1374,7 @@ GOMP_OFFLOAD_openacc_async_exec (void (*fn) (void
>                                unsigned *dims, void *targ_mem_desc,
>                                struct goacc_asyncqueue *aq)
>  {
> -  GOMP_PLUGIN_debug (0, "  %s: prepare mappings\n", __FUNCTION__);
> -
> -  struct goacc_thread *thr = GOMP_PLUGIN_goacc_thread ();
> -  acc_prof_info *prof_info = thr->prof_info;
> -  acc_event_info data_event_info;
> -  acc_api_info *api_info = thr->api_info;
> -  bool profiling_p = __builtin_expect (prof_info != NULL, false);
> -
> -  void **hp = NULL;
> -  CUdeviceptr dp = 0;
> -  void **block = NULL;
> -
> -  if (mapnum > 0)
> -    {
> -      size_t s = mapnum * sizeof (void *);
> -      block = (void **) GOMP_PLUGIN_malloc (2 * sizeof (void *) + s);
> -      hp = block + 2;
> -      for (int i = 0; i < mapnum; i++)
> -     hp[i] = (devaddrs[i] ? devaddrs[i] : hostaddrs[i]);
> -      CUDA_CALL_ASSERT (cuMemAlloc, &dp, s);
> -      if (profiling_p)
> -     goacc_profiling_acc_ev_alloc (thr, (void *) dp, s);
> -    }
> -
> -  /* Copy the (device) pointers to arguments to the device (dp and hp might 
> in
> -     fact have the same value on a unified-memory system).  */
> -  if (mapnum > 0)
> -    {
> -      if (profiling_p)
> -     {
> -       prof_info->event_type = acc_ev_enqueue_upload_start;
> -
> -       data_event_info.data_event.event_type = prof_info->event_type;
> -       data_event_info.data_event.valid_bytes
> -         = _ACC_DATA_EVENT_INFO_VALID_BYTES;
> -       data_event_info.data_event.parent_construct
> -         = acc_construct_parallel;
> -       data_event_info.data_event.implicit = 1; /* Always implicit.  */
> -       data_event_info.data_event.tool_info = NULL;
> -       data_event_info.data_event.var_name = NULL;
> -       data_event_info.data_event.bytes = mapnum * sizeof (void *);
> -       data_event_info.data_event.host_ptr = hp;
> -       data_event_info.data_event.device_ptr = (const void *) dp;
> -
> -       api_info->device_api = acc_device_api_cuda;
> -
> -       GOMP_PLUGIN_goacc_profiling_dispatch (prof_info, &data_event_info,
> -                                             api_info);
> -     }
> -
> -      CUDA_CALL_ASSERT (cuMemcpyHtoDAsync, dp, (void *) hp,
> -                     mapnum * sizeof (void *), aq->cuda_stream);
> -      block[0] = (void *) dp;
> -
> -      struct nvptx_thread *nvthd =
> -     (struct nvptx_thread *) GOMP_PLUGIN_acc_thread ();
> -      block[1] = (void *) nvthd->ptx_dev;
> -
> -      if (profiling_p)
> -     {
> -       prof_info->event_type = acc_ev_enqueue_upload_end;
> -       data_event_info.data_event.event_type = prof_info->event_type;
> -       GOMP_PLUGIN_goacc_profiling_dispatch (prof_info, &data_event_info,
> -                                             api_info);
> -     }
> -    }
> -
> -  nvptx_exec (fn, mapnum, hostaddrs, devaddrs, dims, targ_mem_desc,
> -           dp, aq->cuda_stream);
> -
> -  if (mapnum > 0)
> -    GOMP_OFFLOAD_openacc_async_queue_callback (aq, cuda_free_argmem, block);
> +  nvptx_exec (fn, mapnum, hostaddrs, devaddrs, dims, aq->cuda_stream);
>  }
>  
>  void *


Grüße
 Thomas

Attachment: signature.asc
Description: PGP signature

Reply via email to