From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 51415 invoked by alias); 18 Dec 2018 21:04:17 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 51399 invoked by uid 89); 18 Dec 2018 21:04:16 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 18 Dec 2018 21:04:14 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=svr-ies-mbx-01.mgc.mentorg.com) by relay1.mentorg.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-SHA384:256) id 1gZMXD-0006fJ-QS from Thomas_Schwinge@mentor.com for gcc-patches@gcc.gnu.org; Tue, 18 Dec 2018 13:04:12 -0800 Received: from hertz.schwinge.homeip.net (137.202.0.90) by svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) with Microsoft SMTP Server (TLS) id 15.0.1320.4; Tue, 18 Dec 2018 21:04:08 +0000 From: Thomas Schwinge To: Chung-Lin Tang CC: Subject: Re: [PATCH 2/6, OpenACC, libgomp] Async re-work, oacc-* parts (revised, v2) In-Reply-To: <34db59a1-0446-1720-9a2e-b6e54f52edb2@mentor.com> References: <12319572-dd02-c946-f2b9-9d047be9c707@mentor.com> <34db59a1-0446-1720-9a2e-b6e54f52edb2@mentor.com> User-Agent: Notmuch/0.9-101-g81dad07 (http://notmuchmail.org) Emacs/25.2.2 (x86_64-pc-linux-gnu) Date: Tue, 18 Dec 2018 21:04:00 -0000 Message-ID: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-SW-Source: 2018-12/txt/msg01340.txt.bz2 --=-=-= Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-length: 3824 Hi Chung-Lin! On Tue, 18 Dec 2018 23:06:38 +0800, Chung-Lin Tang wrote: > this part includes some of the lookup_goacc_asyncqueue fixes we talked ab= out. > I am still thinking about how the queue lock problem should really be sol= ved, 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 asyn= c) > +{ > + /* The special value acc_async_noval (-1) maps to the thread-specific > + default async stream. */ > + if (async =3D=3D acc_async_noval) > + async =3D thr->default_async; > + > + if (async =3D=3D acc_async_sync) > + return NULL; > + > + if (async < 0) > + gomp_fatal ("bad async %d", async); > + > + struct gomp_device_descr *dev =3D thr->dev; > + > + gomp_mutex_lock (&dev->openacc.async.lock); > + > + if (!create > + && (async >=3D dev->openacc.async.nasyncqueue > + || !dev->openacc.async.asyncqueue[async])) > + { > + gomp_mutex_unlock (&dev->openacc.async.lock); > + return NULL; > + } > + > + if (async >=3D dev->openacc.async.nasyncqueue) > + { > + int diff =3D async + 1 - dev->openacc.async.nasyncqueue; > + dev->openacc.async.asyncqueue > + =3D gomp_realloc (dev->openacc.async.asyncqueue, > + sizeof (goacc_aq) * (async + 1)); > + memset (dev->openacc.async.asyncqueue + dev->openacc.async.nasyncq= ueue, > + 0, sizeof (goacc_aq) * diff); > + dev->openacc.async.nasyncqueue =3D async + 1; > + } > + > + if (!dev->openacc.async.asyncqueue[async]) > + { > + dev->openacc.async.asyncqueue[async] =3D dev->openacc.async.constr= uct_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 =3D gomp_malloc (sizeof (struct goacc_asyncqueue_l= ist)); > + n->aq =3D dev->openacc.async.asyncqueue[async]; > + n->next =3D dev->openacc.async.active; > + dev->openacc.async.active =3D 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" >=20=20 > 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 =3D ptr; > - struct gomp_device_descr *devicep =3D 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=C3=BC=C3=9Fe Thomas --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename="0001-into-async-re-work-adjust-host_openacc_async_constru.patch" Content-length: 777 >From 4cb99c3691f95b6b299e7cb2603af36f723f9e8e Mon Sep 17 00:00:00 2001 From: Thomas Schwinge 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 --=-=-=--