Hi Julian!

On 2020-05-22T15:16:06-0700, Julian Brown <jul...@codesourcery.com> wrote:
> Since goacc_enter_datum only maps a single data item now, there is no
> need to pass "kinds" as an array.  Passing as a scalar allows for some
> simplification in the function's callers.

You'd hope (didn't verify) that the compiler can do the same
transformation/optimization.  ;-)

But, au contraire: in my opinion (but please tell if you disagree), we
should instead get (back) to the state where the runtime API and the
pragma variants of the respective OpenACC functionality map to the same
libgomp implementation.

That's what we had a while ago: 'acc_create' calling the same
'goacc_enter_data' as 'GOACC_enter_exit_data' did for OpenACC 'enter
data' with 'create' clause, etc.  You then removed/changed that in
2019-12-20 commit 378da98fcc907d05002bcd3d6ff7951f0cf485e5 "OpenACC
reference count overhaul", reason unknown.

The idea is (a) to match in the libgomp implementation what the OpenACC
specification states ("The 'acc_create' routines are equivalent to the
'enter data' directive with a 'create' clause", etc.), and (b) to reduce
code duplication and thus potential for bugs -- like we've seen in the
case of 'attach'/'detach', where one variant didn't do reference counting
(runtime API variant; correct), and the other variant did (pragma
variant; incorrect).

As it must be able to handle the very same things (and more), my
understanding/expectation is that 'goacc_enter_data_internal' must offer
a superset of 'goacc_enter_datum' functionality, so the latter can just
go away?

And same story for the 'exit data' implementations, of course:
'goacc_exit_datum' vs. 'goacc_exit_data_internal'.


Grüße
 Thomas


>       libgomp/
>       * oacc-mem.c (goacc_enter_datum): Use scalar kind argument instead of
>       kinds array.
>       (acc_create, acc_create_async, acc_copyin, acc_copyin_async): Update
>       calls to goacc_enter_datum.
> ---
>  libgomp/oacc-mem.c | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c
> index fff0d573f59..20d241382a8 100644
> --- a/libgomp/oacc-mem.c
> +++ b/libgomp/oacc-mem.c
> @@ -501,7 +501,8 @@ acc_unmap_data (void *h)
>  /* Enter dynamic mapping for a single datum.  Return the device pointer.  */
>
>  static void *
> -goacc_enter_datum (void **hostaddrs, size_t *sizes, void *kinds, int async)
> +goacc_enter_datum (void **hostaddrs, size_t *sizes, unsigned short kind,
> +                int async)
>  {
>    void *d;
>    splay_tree_key n;
> @@ -560,7 +561,7 @@ goacc_enter_datum (void **hostaddrs, size_t *sizes, void 
> *kinds, int async)
>
>        struct target_mem_desc *tgt
>       = gomp_map_vars_async (acc_dev, aq, mapnum, hostaddrs, NULL, sizes,
> -                            kinds, true, GOMP_MAP_VARS_ENTER_DATA);
> +                            &kind, true, GOMP_MAP_VARS_ENTER_DATA);
>        assert (tgt);
>        assert (tgt->list_count == 1);
>        n = tgt->list[0].key;
> @@ -584,15 +585,13 @@ goacc_enter_datum (void **hostaddrs, size_t *sizes, 
> void *kinds, int async)
>  void *
>  acc_create (void *h, size_t s)
>  {
> -  unsigned short kinds[1] = { GOMP_MAP_ALLOC };
> -  return goacc_enter_datum (&h, &s, &kinds, acc_async_sync);
> +  return goacc_enter_datum (&h, &s, GOMP_MAP_ALLOC, acc_async_sync);
>  }
>
>  void
>  acc_create_async (void *h, size_t s, int async)
>  {
> -  unsigned short kinds[1] = { GOMP_MAP_ALLOC };
> -  goacc_enter_datum (&h, &s, &kinds, async);
> +  goacc_enter_datum (&h, &s, GOMP_MAP_ALLOC, async);
>  }
>
>  /* acc_present_or_create used to be what acc_create is now.  */
> @@ -617,15 +616,13 @@ acc_pcreate (void *h, size_t s)
>  void *
>  acc_copyin (void *h, size_t s)
>  {
> -  unsigned short kinds[1] = { GOMP_MAP_TO };
> -  return goacc_enter_datum (&h, &s, &kinds, acc_async_sync);
> +  return goacc_enter_datum (&h, &s, GOMP_MAP_TO, acc_async_sync);
>  }
>
>  void
>  acc_copyin_async (void *h, size_t s, int async)
>  {
> -  unsigned short kinds[1] = { GOMP_MAP_TO };
> -  goacc_enter_datum (&h, &s, &kinds, async);
> +  goacc_enter_datum (&h, &s, GOMP_MAP_TO, async);
>  }
>
>  /* acc_present_or_copyin used to be what acc_copyin is now.  */
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter

Reply via email to