Hi Julian!
Ping.
On 2020-06-26T11:20:40+0200, I wrote:
> On 2019-12-17T22:03:47-0800, Julian Brown <jul...@codesourcery.com> wrote:
>> This part contains the libgomp runtime support for the GOMP_MAP_ATTACH and
>> GOMP_MAP_DETACH mapping kinds (etc.), as introduced by the front-end
>> patches following in this series.
>
>> --- a/libgomp/target.c
>> +++ b/libgomp/target.c
>
>> @@ -1534,6 +1571,18 @@ gomp_unmap_vars_internal (struct target_mem_desc
>> *tgt, bool do_copyfrom,
>
> This is the code path at the end of a structured OpenACC 'data'
> construct.
>
>> + /* We must perform detachments before any copies back to the host. */
>> + for (i = 0; i < tgt->list_count; i++)
>> + {
>> + splay_tree_key k = tgt->list[i].key;
>> +
>> + if (k != NULL && tgt->list[i].do_detach)
>> + gomp_detach_pointer (devicep, aq, k, tgt->list[i].key->host_start
>> + + tgt->list[i].offset,
>> + k->refcount == 1, NULL);
>> + }
>
> Can you please explain (as a source code comment) the logic for here
> using 'k->refcount == 1' for the 'bool finalize' parameter of
> 'gomp_detach_pointer'; this somehow feels "strange"?
>
> Nonwithstanding the question whether that's a valid thing to do or not,
> but doesn't the current code hide the "attach count underflow" error if
> you reach the above code with 'attach_count == 0' (user already
> explicitly 'detach'ed), but then given 'k->refcount == 1' (thus
> 'finalize' semantics), 'gomp_detach_pointer' will then re-initialize
> 'attach_count = 1', and then do another 'gomp_copy_host2dev', etc.
> instead of emitting an error.
>
> (I have not attempted to produce a libgomp test case.)
>
> Shouldn't this just always be 'finalize = false' given that there is no
> 'finalize' semantics for 'detach' on a structured OpenACC 'data'
> constructs -- at least that's what I remember right now?
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
