From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 110647 invoked by alias); 17 Dec 2018 14:11:02 -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 110637 invoked by uid 89); 17 Dec 2018 14:11:02 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=highlight, worry, researched, FAILs 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; Mon, 17 Dec 2018 14:11:00 +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 1gYtbm-0001YY-4r from Thomas_Schwinge@mentor.com for gcc-patches@gcc.gnu.org; Mon, 17 Dec 2018 06:10:58 -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; Mon, 17 Dec 2018 14:10:54 +0000 From: Thomas Schwinge To: Chung-Lin Tang CC: Subject: Re: [PATCH 2/6, OpenACC, libgomp] Async re-work, oacc-* parts In-Reply-To: References: <12319572-dd02-c946-f2b9-9d047be9c707@mentor.com> User-Agent: Notmuch/0.9-101-g81dad07 (http://notmuchmail.org) Emacs/25.2.2 (x86_64-pc-linux-gnu) Date: Mon, 17 Dec 2018 14:11:00 -0000 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-SW-Source: 2018-12/txt/msg01222.txt.bz2 Hi Chung-Lin! On Fri, 14 Dec 2018 23:00:57 +0800, Chung-Lin Tang wrote: > On 2018/12/14 10:53 PM, Thomas Schwinge wrote: > > Please comment on the one TODO > > which before your async re-work also was -- incorrectly? -- run > > asynchronously? > > @@ -563,6 +563,8 @@ GOACC_update (int device, size_t mapnum, > > the value of the allocated device memory in the > > previous pointer. */ > > *(uintptr_t *) hostaddrs[i] =3D (uintptr_t)dptr; > > + /* This is intentionally no calling acc_update_device_async, > > + because TODO. */ > > acc_update_device (hostaddrs[i], sizeof (uintptr_t)); > >=20=20=20 > > /* Restore the host pointer. */ | *(uintptr_t *) hostaddrs[i] =3D t; >=20 > I don't remember adding this piece of comment, it might have been Cesar I= guess? That "TODO" comment, you mean? It was me who just added that one, to highlight this, to ask you to comment, whether it's feasible to turn that "acc_update_device" into "acc_update_device_async", too, or if that has intentionally not been done, given that before your async re-work that also was -- incorrectly? -- run asynchronously. > I'm not sure if there's any real reason not to use acc_update_device_asyn= c here... My worry was that the data object being copied here ("*hostaddrs[i]") is changed immediatelly after the "acc_update_device" call (now inlined above), so if that update get enqueued for asynchronous execution, it might then actually copy the value after "Restore the host pointer". Given the code before and after it, maybe "acc_update_device" is generally the wrong function to call here? (That's, again, a separate change, please.) I have not yet researched where that code is coming from, and what it's supposed to be used for. But as part of the async re-work we should at least understand that one, too. > Change and test to see? Generally, when testing asynchronous behavior, I'd be wary of reading too much into any such test results ("still PASSes" -- by chance?). Unless, of course, there's then a clear regression somewhere ("now FAILs"). ..., which there isn't, because adding a "gomp_fatal" in that code path, that doesn't trigger a single time when running the libgomp testsuite... Gr=C3=BC=C3=9Fe Thomas