On Tue, Mar 31, 2020 at 05:14:17PM +0200, Tobias Burnus wrote:
> gomp_map_vars_internal contains:
>
> if ((kind & typemask) == GOMP_MAP_TO_PSET)
> {
> size_t j;
> for (j = i + 1; j < mapnum; j++)
> if (!GOMP_MAP_POINTER_P (get_kind (short_mapkind, kinds, j)
>
> where one accesses not only the i-th hostaddr but items following it.
>
> On the other hand, in GOMP_target_enter_exit_data one currently has:
>
> 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);
> passing the argument one by one.
>
> I first thought to pass all non MAP_STRUCT items as block before the
> MAP_STRUCT, then a MAP_STRUCT and then try to group the next items.
Doing the mappings separately is intentional, while for target data or
target region mappings it is very likely they will be all released together
as well, so it doesn't matter if they all go from the single same mapping,
for target enter data it is often not the case.
If everything is mapped together, then it can't be really freed until all
the mappings get refcount of 0.
So, instead of the change you've posted, there should be in
GOMP_target_enter_exit_data code to also handle GOMP_MAP_TO_PSET and put the
right number of mappings after it in one block and leave the rest separate.
Jakub
