For OpenACC libgomp entry points, redefine the "int device" argument to "unsigned int flags" (was: OpenACC 2.6 "host_data" construct, "if_present" clause)

[Thomas Schwinge]

Hi!

On Wed, 19 Dec 2018 00:24:00 +0100, I wrote:
> OpenACC 2.6 adds a new clause to the "host_data" construct:
> 2.8.3. "if_present clause".  Gergő (in CC) is working on that.
> 
>     When an 'if_present' clause appears on the directive, the compiler
>     will only change the address of any variable or array which appears
>     in _var-list_ that is present on the current device.
> 
> So, basically:
> 
>     --- a/libgomp/target.c
>     +++ b/libgomp/target.c
>     @@ -1130,13 +1130,17 @@ gomp_map_vars_async (struct gomp_device_descr *devicep,
>            else if ((kind & typemask) == GOMP_MAP_USE_DEVICE_PTR)
>              {
>                cur_node.host_start = (uintptr_t) hostaddrs[i];
>                cur_node.host_end = cur_node.host_start;
>                splay_tree_key n = gomp_map_lookup (mem_map, &cur_node);
>                if (n == NULL)
>                  {
>     +              if ([...])
>     +                /* No error, continue using the host address.  */
>     +                continue;
>                    gomp_mutex_unlock (&devicep->lock);
>                    gomp_fatal ("use_device_ptr pointer wasn't mapped");
>                  }
> 
> Note that this clause applies to *all* "use_device"
> ("GOMP_MAP_USE_DEVICE_PTR") clauses present on the "host_data" construct,
> so it's just a single bit flag for the construct.
> 
> Do you suggest we yet add a new mapping kind
> "GOMP_MAP_USE_DEVICE_PTR_IF_PRESENT" for that?  And, any preference about
> the specific value to use?  Gergő proposed:
> 
>     --- a/include/gomp-constants.h
>     +++ b/include/gomp-constants.h
>     @@ -80,6 +80,10 @@ enum gomp_map_kind
>          GOMP_MAP_DEVICE_RESIDENT =		(GOMP_MAP_FLAG_SPECIAL_1 | 1),
>          /* OpenACC link.  */
>          GOMP_MAP_LINK =			(GOMP_MAP_FLAG_SPECIAL_1 | 2),
>     +    /* Like GOMP_MAP_USE_DEVICE_PTR below, translate a host to a device
>     +       address.  If translation fails because the target is not mapped,
>     +       continue using the host address. */
>     +    GOMP_MAP_USE_DEVICE_PTR_IF_PRESENT =		(GOMP_MAP_FLAG_SPECIAL_1 | 3),
>          /* Allocate.  */
>          GOMP_MAP_FIRSTPRIVATE =		(GOMP_MAP_FLAG_SPECIAL | 0),
>          /* Similarly, but store the value in the pointer rather than
> 
> Or, I had the idea that we could avoid that, instead continue using
> "GOMP_MAP_USE_DEVICE_PTR", and transmit the "if_present" flag through the
> "int device" argument of "GOACC_data_start" (making sure that old
> executables continue to function as before).  For OpenACC, that argument
> is only ever set to "GOMP_DEVICE_ICV" or "GOMP_DEVICE_HOST_FALLBACK" (for
> "if" clause evaluating to "false"), so has some bits to spare for that.
> However, I've not been able to convince myself that this solution would
> be any much prettier than adding a new mapping kind...  ;-)

Having thought about it some more, the idea doesn't actually seem so bad
anymore.  :-) Just don't think of it as 'merging stuff into "int
device"', but rather 'for OpenACC libgomp entry points, redefine the "int
device" argument to "unsigned int flags"' -- see attached WIP (for GCC
trunk, testing).

Jakub, what do you think?


For the "if_present" clause, we'd then initialize "tree flags" not to
zero but to "omp_find_clause (OMP_CLAUSE_IF_PRESENT) ?
GOACC_FLAG_IF_PRESENT : 0" or similar, and then handle that in libgomp.


Grüße
 Thomas


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-For-OpenACC-libgomp-entry-points-redefine-the-int-de.patch
Type: text/x-diff
Size: 16493 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc/attachments/20181219/a807bb35/attachment.bin>