This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 2/6, OpenACC, libgomp] Async re-work, oacc-* parts (revised, v2)
- From: Thomas Schwinge <thomas at codesourcery dot com>
- To: Chung-Lin Tang <cltang at codesourcery dot com>
- Cc: <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 18 Dec 2018 22:03:56 +0100
- Subject: Re: [PATCH 2/6, OpenACC, libgomp] Async re-work, oacc-* parts (revised, v2)
- References: <12319572-dd02-c946-f2b9-9d047be9c707@mentor.com> <34db59a1-0446-1720-9a2e-b6e54f52edb2@mentor.com>
Hi Chung-Lin!
On Tue, 18 Dec 2018 23:06:38 +0800, Chung-Lin Tang <chunglin_tang@mentor.com> wrote:
> this part includes some of the lookup_goacc_asyncqueue fixes we talked about.
> I am still thinking about how the queue lock problem should really be solved, so regard
> this patch as just fixing some of the problems.
Sure, thanks.
Two comments, though:
> --- libgomp/oacc-async.c (revision 267226)
> +++ libgomp/oacc-async.c (working copy)
> +attribute_hidden struct goacc_asyncqueue *
> +lookup_goacc_asyncqueue (struct goacc_thread *thr, bool create, int async)
> +{
> + /* The special value acc_async_noval (-1) maps to the thread-specific
> + default async stream. */
> + if (async == acc_async_noval)
> + async = thr->default_async;
> +
> + if (async == acc_async_sync)
> + return NULL;
> +
> + if (async < 0)
> + gomp_fatal ("bad async %d", async);
> +
> + struct gomp_device_descr *dev = thr->dev;
> +
> + gomp_mutex_lock (&dev->openacc.async.lock);
> +
> + if (!create
> + && (async >= dev->openacc.async.nasyncqueue
> + || !dev->openacc.async.asyncqueue[async]))
> + {
> + gomp_mutex_unlock (&dev->openacc.async.lock);
> + return NULL;
> + }
> +
> + if (async >= dev->openacc.async.nasyncqueue)
> + {
> + int diff = async + 1 - dev->openacc.async.nasyncqueue;
> + dev->openacc.async.asyncqueue
> + = gomp_realloc (dev->openacc.async.asyncqueue,
> + sizeof (goacc_aq) * (async + 1));
> + memset (dev->openacc.async.asyncqueue + dev->openacc.async.nasyncqueue,
> + 0, sizeof (goacc_aq) * diff);
> + dev->openacc.async.nasyncqueue = async + 1;
> + }
> +
> + if (!dev->openacc.async.asyncqueue[async])
> + {
> + dev->openacc.async.asyncqueue[async] = dev->openacc.async.construct_func ();
> +
> + if (!dev->openacc.async.asyncqueue[async])
> + {
> + gomp_mutex_unlock (&dev->openacc.async.lock);
> + gomp_fatal ("async %d creation failed", async);
> + }
That will now always fail for host fallback, where
"host_openacc_async_construct" just always does "return NULL".
Actually, if the device doesn't support asyncqueues, this whole function
should turn into some kind of no-op, so that we don't again and again try
to create a new one for every call to "lookup_goacc_asyncqueue".
I'm attaching one possible solution. I think it's fine to assume that
the majority of devices will support asyncqueues, and for those that
don't, this is just a one-time overhead per async-argument. So, no
special handling required in "lookup_goacc_asyncqueue".
> + /* Link new async queue into active list. */
> + goacc_aq_list n = gomp_malloc (sizeof (struct goacc_asyncqueue_list));
> + n->aq = dev->openacc.async.asyncqueue[async];
> + n->next = dev->openacc.async.active;
> + dev->openacc.async.active = n;
> + }
> + gomp_mutex_unlock (&dev->openacc.async.lock);
You still need to keep "async" locked during...
> + return dev->openacc.async.asyncqueue[async];
... this dereference.
> +}
Oh, and:
> --- libgomp/oacc-plugin.c (revision 267226)
> +++ libgomp/oacc-plugin.c (working copy)
> @@ -31,14 +31,10 @@
> #include "oacc-int.h"
>
> void
> -GOMP_PLUGIN_async_unmap_vars (void *ptr, int async)
> +GOMP_PLUGIN_async_unmap_vars (void *ptr __attribute__((unused)),
> + int async __attribute__((unused)))
> {
> - struct target_mem_desc *tgt = ptr;
> - struct gomp_device_descr *devicep = tgt->device_descr;
> -
> - devicep->openacc.async_set_async_func (async);
> - gomp_unmap_vars (tgt, true);
> - devicep->openacc.async_set_async_func (acc_async_sync);
> + gomp_fatal ("invalid plugin function");
> }
Please add a comment here, something like: "Obsolete entry point, no
longer used."
Grüße
Thomas
>From 4cb99c3691f95b6b299e7cb2603af36f723f9e8e Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Tue, 18 Dec 2018 21:58:41 +0100
Subject: [PATCH] into async re-work: adjust host_openacc_async_construct
---
libgomp/oacc-host.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libgomp/oacc-host.c b/libgomp/oacc-host.c
index 727f8866f45c..cfd8a24f0674 100644
--- a/libgomp/oacc-host.c
+++ b/libgomp/oacc-host.c
@@ -212,7 +212,8 @@ host_openacc_async_queue_callback (struct goacc_asyncqueue *aq
static struct goacc_asyncqueue *
host_openacc_async_construct (void)
{
- return NULL;
+ /* We have to return non-NULL here, but it's OK to use a dummy. */
+ return (struct goacc_asyncqueue *) -1;
}
static bool
--
2.17.1