Hi Julian!
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.)
On 2020-05-19T17:58:16+0200, I wrote:
> On 2019-12-17T22:02:27-0800, Julian Brown <jul...@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?
>
>> +
>> + 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?
>
>> +
>> + 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
