Hi!

To move us one small step forward:

On 2020-06-25T13:03:53+0200, I wrote:
> Ping, in particular my question about different 'GOMP_MAP_FORCE_FROM' vs.
> 'GOMP_MAP_FROM' handling.
>
> (I have not yet looked whether 'GOMP_MAP_ALWAYS_FROM' may be generate
> nowadays, given your pending front end/middle end patches.)

It isn't, at least not given the current test cases, and I'm not aware of
data movement in OpenACC with (OpenMP) "always" semantics.

> On 2020-05-19T17:58:16+0200, I wrote:
>> On 2019-12-17T22:02:27-0800, Julian Brown <julian@codesourcery.com> wrote:
>>> --- a/libgomp/oacc-mem.c
>>> +++ b/libgomp/oacc-mem.c
>>
>> (Unhelpful diff trimmed.)
>>
>>> +/* Unmap variables for OpenACC "exit data", with optional finalization
>>> +   (affecting all mappings in this operation).  */
>>
>>> +static void
>>> +goacc_exit_data_internal (struct gomp_device_descr *acc_dev, size_t mapnum,
>>> +                     void **hostaddrs, size_t *sizes,
>>> +                     unsigned short *kinds, bool finalize, goacc_aq aq)
>>> +{
>>> +  gomp_mutex_lock (&acc_dev->lock);
>>
>>> +  for (size_t i = 0; i < mapnum; ++i)
>>>      {
>>
>>> +      unsigned char kind = kinds[i] & 0xff;
>>> +      bool copyfrom = false;
>>
>>> +      switch (kind)
>>
>>> +   case GOMP_MAP_FROM:
>>> +   case GOMP_MAP_FORCE_FROM:
>>> +   case GOMP_MAP_ALWAYS_FROM:
>>> +     copyfrom = true;
>>> +     /* Fallthrough.  */
>>
>> What is the case that a 'GOMP_MAP_ALWAYS_FROM' would be generated for
>> OpenACC code?  Putting an 'assert' here, it never triggers, given the
>> current set of libgomp test cases.  If there is such a case, we should
>> add a test case, otherwise, I suggest we do put an 'assert' here (whilst
>> leaving in the supposedly correct code, if you'd like), to document that
>> this not currently expected, and thus not tested?

Instead of keeping dead code, I decided it's better to just "[OpenACC]
Remove (unused) 'GOMP_MAP_ALWAYS_FROM' handling from
'libgomp/oacc-mem.c:goacc_exit_data_internal'"; pushed to master branch
in commit 995aba5867b1c64b2b56a200ef16b135effe85f7, and releases/gcc-10
branch in commit ddce10e77f04410c4ce376e6efdf520a7311a11b, see attached.
Should a 'GOMP_MAP_ALWAYS_FROM' now ever appear (I don't see how), it
will be diagnosed via the 'gomp_fatal' with 'UNHANDLED kind'.

>>> +
>>> +   case GOMP_MAP_TO_PSET:
>>> +   case GOMP_MAP_POINTER:
>>> +   case GOMP_MAP_DELETE:
>>> +   case GOMP_MAP_RELEASE:
>>> +     {
>>> +       struct splay_tree_key_s cur_node;
>>> +       cur_node.host_start = (uintptr_t) hostaddrs[i];
>>> +       cur_node.host_end = cur_node.host_start
>>> +                           + (kind == GOMP_MAP_POINTER
>>> +                              ? sizeof (void *) : sizes[i]);
>>> +       splay_tree_key n
>>> +         = splay_tree_lookup (&acc_dev->mem_map, &cur_node);
>>> +
>>> +       if (n == NULL)
>>> +         continue;
>>> +
>>> +       if (finalize)
>>> +         {
>>> +           if (n->refcount != REFCOUNT_INFINITY)
>>> +             n->refcount -= n->virtual_refcount;
>>> +           n->virtual_refcount = 0;
>>> +         }
>>> +
>>> +       if (n->virtual_refcount > 0)
>>> +         {
>>> +           if (n->refcount != REFCOUNT_INFINITY)
>>> +             n->refcount--;
>>> +           n->virtual_refcount--;
>>> +         }
>>> +       else if (n->refcount > 0 && n->refcount != REFCOUNT_INFINITY)
>>> +         n->refcount--;
>>> +
>>> +       if (copyfrom
>>> +           && (kind != GOMP_MAP_FROM || n->refcount == 0))
>>> +         gomp_copy_dev2host (acc_dev, aq, (void *) cur_node.host_start,
>>> +                             (void *) (n->tgt->tgt_start + n->tgt_offset
>>> +                                       + cur_node.host_start
>>> +                                       - n->host_start),
>>> +                             cur_node.host_end - cur_node.host_start);
>>
>> That 'kind != GOMP_MAP_FROM' conditional looks wrong to me.  This should
>> instead be 'kind == GOMP_MAP_ALWAYS_FROM'?  Or, get removed, together
>> with the 'GOMP_MAP_ALWAYS_FROM' handling above?  But definitely
>> 'GOMP_MAP_FORCE_FROM' and 'GOMP_MAP_FROM' need to be handled the same, as
>> far as I can tell?

I've now pushed "[OpenACC] Revert always-copyfrom behavior for
'GOMP_MAP_FORCE_FROM' in 'libgomp/oacc-mem.c:goacc_exit_data_internal'"
to master branch in commit e7f3f7fe08bdd49367f682398e1d2f4e6b60ef84, and
releases/gcc-10 branch in commit
50666d23b52794774eefbeff046d5c3235db8b99, see attached.

>>> +
>>> +       if (n->refcount == 0)
>>> +         gomp_remove_var_async (acc_dev, n, aq);
>>> +     }
>>> +     break;
>>> +   default:
>>> +     gomp_fatal (">>>> goacc_exit_data_internal UNHANDLED kind 0x%.2x",
>>> +                     kind);
>>>     }
>>>      }
>>>
>>>    gomp_mutex_unlock (&acc_dev->lock);
>>
>>>  }


Grüße
 Thomas


-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter