[3/7,OpenACC] Don't pass kind array via pointer to goacc_enter_datum

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.

OK?

Julian

ChangeLog

	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(-)

Hi Julian!

On 2020-05-22T15:16:06-0700, Julian Brown <julian@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

On Thu, 25 Jun 2020 12:52:23 +0200
Thomas Schwinge <thomas@codesourcery.com> wrote:

> Hi Julian!
> 
> On 2020-05-22T15:16:06-0700, Julian Brown <julian@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.

It's a little ugly for "enter data" because the API routines return the
device pointer, but the directive implementation may involve several
mappings and a single "device pointer" return doesn't really make sense
in that case. I didn't much like the previous approach of returning
NULL.

We can still try to factor out the duplicated code though. I've posted a
new approach here (see the parent "0/2" patch also):

https://gcc.gnu.org/pipermail/gcc-patches/2020-July/549773.html

Julian