Hi Julian!
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
