Hi Julian! On 2019-10-29T12:15:01+0000, Julian Brown <jul...@codesourcery.com> wrote: > On Mon, 21 Oct 2019 16:14:11 +0200 > Thomas Schwinge <tho...@codesourcery.com> wrote: > >> On 2019-10-03T09:35:04-0700, Julian Brown <jul...@codesourcery.com> >> wrote: >> > --- a/libgomp/oacc-parallel.c >> > +++ b/libgomp/oacc-parallel.c >> > @@ -56,12 +56,29 @@ find_pointer (int pos, size_t mapnum, unsigned >> > short *kinds) >> >> I've always been confused by this function (before known as >> 'find_pset'); this feels wrong, but I've never gotten to the bottom >> of it. > > This version removes that function in favour of a function that finds > groups of consecutive mappings that should be kept together for a > single gomp_map_vars invocation. I think that fits better with my > findings as written up on the wiki page > https://gcc.gnu.org/wiki/LibgompPointerMappingKinds.
:-) Please guide my trying to understand the changes there:
> --- a/libgomp/oacc-parallel.c
> +++ b/libgomp/oacc-parallel.c
> @@ -47,23 +47,39 @@ _Static_assert (GOACC_FLAGS_UNMARSHAL
> (GOMP_DEVICE_HOST_FALLBACK)
> "legacy GOMP_DEVICE_HOST_FALLBACK broken");
>
>
> -/* Returns the number of mappings associated with the pointer or pset. PSET
> - have three mappings, whereas pointer have two. */
> +/* Some types of (pointer) variables use several consecutive mappings, which
> + must be treated as a group for enter/exit data directives. This function
> + returns the last mapping in such a group (inclusive), or POS for singleton
> + mappings. */
>
> static int
> -find_pointer (int pos, size_t mapnum, unsigned short *kinds)
> +find_group_last (int pos, size_t mapnum, unsigned short *kinds)
> {
> - if (pos + 1 >= mapnum)
> - return 0;
> + unsigned char kind0 = kinds[pos] & 0xff;
> + int first_pos = pos, last_pos = pos;
>
> - unsigned char kind = kinds[pos+1] & 0xff;
> -
> - if (kind == GOMP_MAP_TO_PSET)
> - return 3;
> - else if (kind == GOMP_MAP_POINTER)
> - return 2;
> + if (kind0 == GOMP_MAP_TO_PSET)
> + {
> + while (pos + 1 < mapnum && (kinds[pos + 1] & 0xff) == GOMP_MAP_POINTER)
> + last_pos = ++pos;
> + /* We expect at least one GOMP_MAP_POINTER after a GOMP_MAP_TO_PSET.
> */
> + assert (last_pos > first_pos);
> + }
> + else
> + {
> + /* GOMP_MAP_ALWAYS_POINTER can only appear directly after some other
> + mapping. */
> + if (pos + 1 < mapnum
> + && (kinds[pos + 1] & 0xff) == GOMP_MAP_ALWAYS_POINTER)
> + return pos + 1;
> +
> + /* We can have one or several GOMP_MAP_POINTER mappings after a to/from
> + (etc.) mapping. */
> + while (pos + 1 < mapnum && (kinds[pos + 1] & 0xff) == GOMP_MAP_POINTER)
> + last_pos = ++pos;
> + }
>
> - return 0;
> + return last_pos;
> }
So I ran a simple experiment where I did:
assert (find_group_last (i, mapnum, kinds) == i + pointer);
... where 'pointer' is the current 'find_pointer' function. (That is,
compare that the old and new way are doing the same things, given the
current GCC code generation/test cases.)
This 'assert' triggers for a few test cases:
'libgomp.oacc-fortran/allocatable-array-1.f90',
'libgomp.oacc-fortran/data-2.f90', 'libgomp.oacc-fortran/data-3.f90',
'libgomp.oacc-fortran/data-4-2.f90', 'libgomp.oacc-fortran/data-4.f90',
'libgomp.oacc-fortran/data-5.f90', 'libgomp.oacc-fortran/if-1.f90',
'libgomp.oacc-fortran/optional-data-enter-exit.f90'. (Maybe those are
the only ones actually using that stuff?)
I looked into the first one
('libgomp.oacc-fortran/allocatable-array-1.f90'), and for:
integer, parameter :: n = 40
integer, allocatable :: ar(:,:,:)
allocate (ar(1:n,0:n-1,0:n-1))
!$acc enter data copyin (ar)
... found:
(gdb) print mapnum
$2 = 3
(gdb) print kinds[0]
$3 = 1 // GOMP_MAP_TO
(gdb) print kinds[1]
$4 = 773
(gdb) print kinds[1] & 0xff
$5 = 5 // GOMP_MAP_TO_PSET
(gdb) print kinds[2]
$6 = 772
(gdb) print kinds[2] & 0xff
$7 = 4 // GOMP_MAP_POINTER
Current behavior: 'find_pointer (0, mapnum, kinds) == 3', so all three
get mapped as one group.
New behavior: 'find_group_last (0, mapnum, kinds) == 0', so the
'GOMP_MAP_TO' gets mapped alone. Then, 'find_group_last (1, mapnum,
kinds) == 2', so the 'GOMP_MAP_TO_PSET', 'GOMP_MAP_POINTER' get mapped as
one group.
Is that intentional?
Any then, compating that to
'libgomp/target.c:GOMP_target_enter_exit_data', where (aside from
'GOMP_MAP_STRUCT'; not relevant for us right now, yes?) everything always
gets mapped alone:
for (i = 0; i < mapnum; i++)
if ((kinds[i] & 0xff) == GOMP_MAP_STRUCT)
{ [...] }
else
gomp_map_vars (devicep, 1, &hostaddrs[i], NULL, &sizes[i], &kinds[i],
true, GOMP_MAP_VARS_ENTER_DATA);
Is it just an "accident" that for OpenACC we were and still are going to
do this differently, or is there an actual reason?
I'm not objecting to changing any of that, but would like to understand
this better.
Grüße
Thomas
signature.asc
Description: PGP signature
