Hi Julian! On 2020-06-05T21:31:08+0100, Julian Brown wrote: > On Fri, 5 Jun 2020 13:17:09 +0200 > Thomas Schwinge wrote: >> On 2019-12-17T21:03:47-0800, Julian Brown >> wrote: >> > This part contains the libgomp runtime support for the >> > GOMP_MAP_ATTACH and GOMP_MAP_DETACH mapping kinds >> >> > --- a/libgomp/target.c >> > +++ b/libgomp/target.c >> >> > @@ -1203,6 +1211,32 @@ gomp_map_vars_internal (struct gomp_device_descr *devicep, >> >> > + case GOMP_MAP_ATTACH: >> > + { >> > + cur_node.host_start = (uintptr_t) hostaddrs[i]; >> > + cur_node.host_end = cur_node.host_start + sizeof (void *); >> > + splay_tree_key n = splay_tree_lookup (mem_map, &cur_node); >> > + if (n != NULL) >> > + { >> > + tgt->list[i].key = n; >> > + tgt->list[i].offset = cur_node.host_start - n->host_start; >> > + tgt->list[i].length = n->host_end - n->host_start; >> > + tgt->list[i].copy_from = false; >> > + tgt->list[i].always_copy_from = false; >> > + tgt->list[i].do_detach >> > + = (pragma_kind != GOMP_MAP_VARS_OPENACC_ENTER_DATA); >> > + n->refcount++; >> > + } >> > + else >> > + { >> > + gomp_mutex_unlock (&devicep->lock); >> > + gomp_fatal ("outer struct not mapped for attach"); >> > + } >> > + gomp_attach_pointer (devicep, aq, mem_map, n, >> > + (uintptr_t) hostaddrs[i], sizes[i], >> > + cbufp); >> > + continue; >> > + } >> >> For the OpenACC runtime API 'acc_attach' etc. routines they don't, so >> what's the conceptual reason that for the corresponding OpenACC >> directive variants, 'GOMP_MAP_ATTACH' etc. here participate in >> reference counting ('n->refcount++' above)? I understand OpenACC >> 'attach'/'detach' clauses to be simple "executable clauses", which >> just update some values somewhere (say, like >> 'GOMP_MAP_ALWAYS_POINTER'), but they don't alter any mapping state, >> thus wouldn't appear to need reference counting? > > IIUC, n->refcount is not directly the "structural reference count" as > seen at source level, but rather counts the number of target_var_descs > in the lists appended to each target_mem_desc -- and GOMP_MAP_ATTACH > have variable entries in those lists. That may be OK if that's purely an implementation detail that isn't visible to the user, however: > That's not the case for the API > routines. As I had mentioned, the problem is: in contrast to 'acc_attach', an OpenACC 'enter data' directive with 'attach' clause currently uses this same reference-counted code path, and thus such an 'attach' without corresponding 'detach' inhibits unmapping; see 'libgomp.oacc-c-c++-common/mdc-refcount-1.c' in the attached patch "OpenACC 'attach'/'detach' has no business affecting user-visible reference counting". That patch seemed to be the logical next step then, to unify the code paths for 'acc_attach' and 'enter data' directive with 'attach' clause (which have to act in the same way). That's (conceptually) somewhat similar to what you had proposed as part of . (But all these things really need to be discussed individually...) However, that patch regresses 'libgomp.oacc-fortran/deep-copy-6-no_finalize.F90', and also the 'deep-copy-7b2f-2.c', and 'deep-copy-7cf.c' that I'm attaching here. I have not yet made an attempts to understand these regressions. It may be that a Detach Action actually effects an (attached) device pointer being copied back to the host, and then disturbing things -- and if that, then it may be a bug in libgomp, or in the test case. ;-) 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