Hi Julian! On 2019-12-12T19:01:21+0000, Julian Brown <jul...@codesourcery.com> wrote: > This is a fix for PR92881, broken out of the larger "reference counting > overhaul" patch last posted here: > > https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02235.html
Thanks!
> The current implementation (potentially with synchronous unmapping of
> a variable/memory block immediately after an asynchronous copyout which
> might not complete beforehand) is "obviously incorrect" by observation
ACK.
> but does not appear to cause any issues with the current testsuite
> (i.e., there is no test improvement with this patch).
Ah, too bad. ;-|
> Tested with offloading to AMD GCN. OK for trunk?
Yes, with a tiny change as noted below:
> --- a/libgomp/oacc-mem.c
> +++ b/libgomp/oacc-mem.c
> @@ -660,7 +660,6 @@ static void
> delete_copyout (unsigned f, void *h, size_t s, int async, const char
> *libfnname)
> {
> splay_tree_key n;
> - void *d;
> struct goacc_thread *thr = goacc_thread ();
> struct gomp_device_descr *acc_dev = thr->dev;
>
> @@ -689,9 +688,6 @@ delete_copyout (unsigned f, void *h, size_t s, int async,
> const char *libfnname)
> gomp_fatal ("[%p,%d] is not mapped", (void *)h, (int)s);
> }
>
> - d = (void *) (n->tgt->tgt_start + n->tgt_offset
> - + (uintptr_t) h - n->host_start);
> -
> if ((uintptr_t) h < n->host_start || (uintptr_t) h + s > n->host_end)
> {
> size_t host_size = n->host_end - n->host_start;
> @@ -723,12 +719,15 @@ delete_copyout (unsigned f, void *h, size_t s, int
> async, const char *libfnname)
> if (f & FLAG_COPYOUT)
> {
> - [...]
> + void *d = (void *) (n->tgt->tgt_start + n->tgt_offset
> + + (uintptr_t) h - n->host_start);
Not sure why you're moving 'd' as part of this patch, but OK if you'd
like. ;-)
> --- a/libgomp/target.c
> +++ b/libgomp/target.c
(For anyone else reading, the diff displays a bit weird; what's really
going on is that 'gomp_remove_var' got moved further down, as is nicely
shown by 'git diff --patience'.)
> +static bool
> +gomp_unref_tgt (void *ptr)
> {
> bool is_tgt_unmapped = false;
> - [...]
> +
> + struct target_mem_desc *tgt = (struct target_mem_desc *) ptr;
> +
> + assert (tgt->refcount != REFCOUNT_INFINITY);
Please leave out this 'assert': we don't have any of these yet for
'tgt->refcount' (as far as I remember), and fixing that is a separate
change, to generally deal that issue;
<87r22an29u.fsf@euler.schwinge.homeip.net">http://mid.mail-archive.com/87r22an29u.fsf@euler.schwinge.homeip.net>.
Grüße
Thomas
signature.asc
Description: PGP signature
