Hi!

On 2019-10-06T15:32:35-0700, Julian Brown <jul...@codesourcery.com> wrote:
> This patch uses gomp_map_val for OpenACC host-to-device address
> translation instead of open-coding the device address calculation.

(As has been discussed before.)

(And then, also see <https://gcc.gnu.org/PR90596> "'GOACC_parallel_keyed'
should use 'GOMP_MAP_VARS_TARGET'" for additional simplification.)

> OK for trunk?

Thanks, that's OK, with one item addressed (see below).  To record the
review effort please include "Reviewed-by: Thomas Schwinge
<tho...@codesourcery.com>" in the commit log, see
<https://gcc.gnu.org/wiki/Reviewed-by>.

>       libgomp/
>       * libgomp.h (gomp_map_val): Add prototype.
>       * oacc-parallel.c (GOACC_parallel_keyed): Use gomp_map_val instead of
>       open-coding device-address calculation.
>       * target.c (gomp_map_val): Make global.
> ---
>  libgomp/libgomp.h       | 1 +
>  libgomp/oacc-parallel.c | 8 ++------
>  libgomp/target.c        | 2 +-
>  3 files changed, 4 insertions(+), 7 deletions(-)

> --- a/libgomp/libgomp.h
> +++ b/libgomp/libgomp.h
> @@ -1095,6 +1095,7 @@ extern void gomp_copy_host2dev (struct 
> gomp_device_descr *,
>  extern void gomp_copy_dev2host (struct gomp_device_descr *,
>                               struct goacc_asyncqueue *, void *, const void *,
>                               size_t);
> +extern uintptr_t gomp_map_val (struct target_mem_desc *, void **, size_t);
>  
>  #ifdef RC_CHECKING
>  extern void dump_tgt (const char *, struct target_mem_desc *);
> diff --git a/libgomp/oacc-parallel.c b/libgomp/oacc-parallel.c
> index 18feac5f31c..02472c56270 100644
> --- a/libgomp/oacc-parallel.c
> +++ b/libgomp/oacc-parallel.c
> @@ -350,12 +350,8 @@ GOACC_parallel_keyed (int flags_m, void (*fn) (void *),
>  
>    devaddrs = gomp_alloca (sizeof (void *) * mapnum);
>    for (i = 0; i < mapnum; i++)
> -    if (tgt->list[i].key != NULL)
> -      devaddrs[i] = (void *) (tgt->list[i].key->tgt->tgt_start
> -                           + tgt->list[i].key->tgt_offset
> -                           + tgt->list[i].offset);
> -    else
> -      devaddrs[i] = NULL;
> +    devaddrs[i] = (void *) gomp_map_val (tgt, hostaddrs, i);
> +
>    if (aq == NULL)
>      acc_dev->openacc.exec_func (tgt_fn, mapnum, hostaddrs, devaddrs, dims,
>                               tgt);
> diff --git a/libgomp/target.c b/libgomp/target.c
> index 29cb7ca8348..aa7a1df1e46 100644
> --- a/libgomp/target.c
> +++ b/libgomp/target.c
> @@ -670,7 +670,7 @@ gomp_map_fields_existing (struct target_mem_desc *tgt,
>             (void *) cur_node.host_end);
>  }
>  
> -static inline uintptr_t
> +uintptr_t
>  gomp_map_val (struct target_mem_desc *tgt, void **hostaddrs, size_t i)

In <https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01620.html> I asked
whether that "Should be 'attribute_hidden'"?


Grüße
 Thomas

Attachment: signature.asc
Description: PGP signature

Reply via email to