From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21955 invoked by alias); 6 Dec 2018 17:21:36 -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 21945 invoked by uid 89); 6 Dec 2018 17:21:36 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: =?ISO-8859-1?Q?No, score=-11.9 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS,TIME_LIMIT_EXCEEDED autolearn=unavailable version=3.3.2 spammy=gr, gr=c3=bc=c3?= 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; Thu, 06 Dec 2018 17:21:26 +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 1gUxL1-0000u5-D2 from Thomas_Schwinge@mentor.com ; Thu, 06 Dec 2018 09:21:23 -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; Thu, 6 Dec 2018 17:21:19 +0000 From: Thomas Schwinge To: Jakub Jelinek CC: Chung-Lin Tang , Subject: Re: [PATCH 4/6, OpenACC, libgomp] Async re-work, libgomp/target.c changes In-Reply-To: References: User-Agent: Notmuch/0.9-101-g81dad07 (http://notmuchmail.org) Emacs/25.2.2 (x86_64-pc-linux-gnu) Date: Thu, 06 Dec 2018 17:21: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/msg00379.txt.bz2 Hi Jakub! On Tue, 25 Sep 2018 21:11:24 +0800, Chung-Lin Tang wrote: > Hi Jakub, > This part has changes to 'struct goacc_asyncqueue*' arguments to various > memory copying/mapping functions. To lessen the amount of code changes ne= w 'gomp_map/unmap_vars_async' > functions names are used (with the non-async original names defined with = the asyncqueue=3D=3DNULL). Is that the way you'd like this to be done, or should instead that "struct goacc_asyncqueue *aq" parameter be added/passed through all the existing functions? (The latter would be my preference, actually.) That is, as Chung-Lin proposed: > --- a/libgomp/target.c > +++ b/libgomp/target.c > @@ -177,6 +177,22 @@ gomp_device_copy (struct gomp_device_descr *devicep, > } > } >=20=20 > +static inline void > +goacc_device_copy_async (struct gomp_device_descr *devicep, > + bool (*copy_func) (int, void *, const void *, size_t, > + struct goacc_asyncqueue *), > + const char *dst, void *dstaddr, > + const char *src, const void *srcaddr, > + size_t size, struct goacc_asyncqueue *aq) > +{ > + if (!copy_func (devicep->target_id, dstaddr, srcaddr, size, aq)) > + { > + gomp_mutex_unlock (&devicep->lock); > + gomp_fatal ("Copying of %s object [%p..%p) to %s object [%p..%p) f= ailed", > + src, srcaddr, srcaddr + size, dst, dstaddr, dstaddr + size); > + } > +} ..., or should we instead add "struct goacc_asyncqueue *aq" to the existing "gomp_device_copy", and then, recursively, also add it to the existing plugin functions "host2dev" and "dev2host", instead of adding new functions "openacc.async.host2dev" and "openacc.async.dev2host" (see "GOMP_OFFLOAD_host2dev" vs. "GOMP_OFFLOAD_openacc_async_host2dev", and "GOMP_OFFLOAD_dev2host" vs. "GOMP_OFFLOAD_openacc_async_dev2host" as proposed in "[PATCH 6/6, OpenACC, libgomp] Async re-work, nvptx changes")? Similarly for "gomp_map_vars"/"gomp_map_vars_async", "gomp_unmap_vars"/"gomp_unmap_vars_async", see below. I'd rather have one single interface (optionally called with a "NULL" "struct goacc_asyncqueue *aq"), instead of adding more/similar async interfaces. Aside from avoiding adding to the cognitive load, the rationaly also being that in the long term, for performance reasons, we'll probably want to make more stuff asynchronous that currently is synchronous, thus eventually obsoleting the synchronous interfaces. For reference: > @@ -263,8 +279,9 @@ gomp_to_device_kind_p (int kind) > } > } >=20=20 > -static void > +attribute_hidden void > gomp_copy_host2dev (struct gomp_device_descr *devicep, > + struct goacc_asyncqueue *aq, > void *d, const void *h, size_t sz, > struct gomp_coalesce_buf *cbuf) > { > @@ -293,14 +310,23 @@ gomp_copy_host2dev (struct gomp_device_descr *devic= ep, > } > } > } > - gomp_device_copy (devicep, devicep->host2dev_func, "dev", d, "host", h= , sz); > + if (aq) > + goacc_device_copy_async (devicep, devicep->openacc.async.host2dev_fu= nc, > + "dev", d, "host", h, sz, aq); > + else > + gomp_device_copy (devicep, devicep->host2dev_func, "dev", d, "host",= h, sz); > } >=20=20 > -static void > +attribute_hidden void > gomp_copy_dev2host (struct gomp_device_descr *devicep, > + struct goacc_asyncqueue *aq, > void *h, const void *d, size_t sz) > { > - gomp_device_copy (devicep, devicep->dev2host_func, "host", h, "dev", d= , sz); > + if (aq) > + goacc_device_copy_async (devicep, devicep->openacc.async.dev2host_fu= nc, > + "host", h, "dev", d, sz, aq); > + else > + gomp_device_copy (devicep, devicep->dev2host_func, "host", h, "dev",= d, sz); > } >=20=20 > static void > @@ -318,7 +344,8 @@ gomp_free_device_memory (struct gomp_device_descr *de= vicep, void *devptr) > Helper function of gomp_map_vars. */ >=20=20 > static inline void > -gomp_map_vars_existing (struct gomp_device_descr *devicep, splay_tree_ke= y oldn, > +gomp_map_vars_existing (struct gomp_device_descr *devicep, > + struct goacc_asyncqueue *aq, splay_tree_key oldn, > splay_tree_key newn, struct target_var_desc *tgt_var, > unsigned char kind, struct gomp_coalesce_buf *cbuf) > { > @@ -340,7 +367,7 @@ gomp_map_vars_existing (struct gomp_device_descr *dev= icep, splay_tree_key oldn, > } >=20=20 > if (GOMP_MAP_ALWAYS_TO_P (kind)) > - gomp_copy_host2dev (devicep, > + gomp_copy_host2dev (devicep, aq, > (void *) (oldn->tgt->tgt_start + oldn->tgt_offset > + newn->host_start - oldn->host_start), > (void *) newn->host_start, > @@ -358,8 +385,8 @@ get_kind (bool short_mapkind, void *kinds, int idx) > } >=20=20 > static void > -gomp_map_pointer (struct target_mem_desc *tgt, uintptr_t host_ptr, > - uintptr_t target_offset, uintptr_t bias, > +gomp_map_pointer (struct target_mem_desc *tgt, struct goacc_asyncqueue *= aq, > + uintptr_t host_ptr, uintptr_t target_offset, uintptr_t bias, > struct gomp_coalesce_buf *cbuf) > { > struct gomp_device_descr *devicep =3D tgt->device_descr; > @@ -370,7 +397,7 @@ gomp_map_pointer (struct target_mem_desc *tgt, uintpt= r_t host_ptr, > if (cur_node.host_start =3D=3D (uintptr_t) NULL) > { > cur_node.tgt_offset =3D (uintptr_t) NULL; > - gomp_copy_host2dev (devicep, > + gomp_copy_host2dev (devicep, aq, > (void *) (tgt->tgt_start + target_offset), > (void *) &cur_node.tgt_offset, > sizeof (void *), cbuf); > @@ -392,12 +419,13 @@ gomp_map_pointer (struct target_mem_desc *tgt, uint= ptr_t host_ptr, > array section. Now subtract bias to get what we want > to initialize the pointer with. */ > cur_node.tgt_offset -=3D bias; > - gomp_copy_host2dev (devicep, (void *) (tgt->tgt_start + target_offset), > + gomp_copy_host2dev (devicep, aq, (void *) (tgt->tgt_start + target_off= set), > (void *) &cur_node.tgt_offset, sizeof (void *), cbuf); > } >=20=20 > static void > -gomp_map_fields_existing (struct target_mem_desc *tgt, splay_tree_key n, > +gomp_map_fields_existing (struct target_mem_desc *tgt, > + struct goacc_asyncqueue *aq, splay_tree_key n, > size_t first, size_t i, void **hostaddrs, > size_t *sizes, void *kinds, > struct gomp_coalesce_buf *cbuf) > @@ -417,7 +445,7 @@ gomp_map_fields_existing (struct target_mem_desc *tgt= , splay_tree_key n, > && n2->tgt =3D=3D n->tgt > && n2->host_start - n->host_start =3D=3D n2->tgt_offset - n->tgt_o= ffset) > { > - gomp_map_vars_existing (devicep, n2, &cur_node, > + gomp_map_vars_existing (devicep, aq, n2, &cur_node, > &tgt->list[i], kind & typemask, cbuf); > return; > } > @@ -433,8 +461,8 @@ gomp_map_fields_existing (struct target_mem_desc *tgt= , splay_tree_key n, > && n2->host_start - n->host_start > =3D=3D n2->tgt_offset - n->tgt_offset) > { > - gomp_map_vars_existing (devicep, n2, &cur_node, &tgt->list[i], > - kind & typemask, cbuf); > + gomp_map_vars_existing (devicep, aq, n2, &cur_node, > + &tgt->list[i], kind & typemask, cbuf); > return; > } > } > @@ -445,7 +473,7 @@ gomp_map_fields_existing (struct target_mem_desc *tgt= , splay_tree_key n, > && n2->tgt =3D=3D n->tgt > && n2->host_start - n->host_start =3D=3D n2->tgt_offset - n->tgt_offs= et) > { > - gomp_map_vars_existing (devicep, n2, &cur_node, &tgt->list[i], > + gomp_map_vars_existing (devicep, aq, n2, &cur_node, &tgt->list[i], > kind & typemask, cbuf); > return; > } > @@ -482,6 +510,18 @@ gomp_map_vars (struct gomp_device_descr *devicep, si= ze_t mapnum, > void **hostaddrs, void **devaddrs, size_t *sizes, void *kinds, > bool short_mapkind, enum gomp_map_vars_kind pragma_kind) > { > + struct target_mem_desc *tgt; > + tgt =3D gomp_map_vars_async (devicep, NULL, mapnum, hostaddrs, devaddr= s, > + sizes, kinds, short_mapkind, pragma_kind); > + return tgt; > +} > + > +attribute_hidden struct target_mem_desc * > +gomp_map_vars_async (struct gomp_device_descr *devicep, > + struct goacc_asyncqueue *aq, size_t mapnum, > + void **hostaddrs, void **devaddrs, size_t *sizes, void *kinds, > + bool short_mapkind, enum gomp_map_vars_kind pragma_kind) > +{ > size_t i, tgt_align, tgt_size, not_found_cnt =3D 0; > bool has_firstprivate =3D false; > const int rshift =3D short_mapkind ? 8 : 3; > @@ -594,7 +634,7 @@ gomp_map_vars (struct gomp_device_descr *devicep, siz= e_t mapnum, > continue; > } > for (i =3D first; i <=3D last; i++) > - gomp_map_fields_existing (tgt, n, first, i, hostaddrs, > + gomp_map_fields_existing (tgt, aq, n, first, i, hostaddrs, > sizes, kinds, NULL); > i--; > continue; > @@ -639,7 +679,7 @@ gomp_map_vars (struct gomp_device_descr *devicep, siz= e_t mapnum, > else > n =3D splay_tree_lookup (mem_map, &cur_node); > if (n && n->refcount !=3D REFCOUNT_LINK) > - gomp_map_vars_existing (devicep, n, &cur_node, &tgt->list[i], > + gomp_map_vars_existing (devicep, aq, n, &cur_node, &tgt->list[i], > kind & typemask, NULL); > else > { > @@ -750,7 +790,7 @@ gomp_map_vars (struct gomp_device_descr *devicep, siz= e_t mapnum, > tgt_size =3D (tgt_size + align - 1) & ~(align - 1); > tgt->list[i].offset =3D tgt_size; > len =3D sizes[i]; > - gomp_copy_host2dev (devicep, > + gomp_copy_host2dev (devicep, aq, > (void *) (tgt->tgt_start + tgt_size), > (void *) hostaddrs[i], len, cbufp); > tgt_size +=3D len; > @@ -784,7 +824,7 @@ gomp_map_vars (struct gomp_device_descr *devicep, siz= e_t mapnum, > continue; > } > for (i =3D first; i <=3D last; i++) > - gomp_map_fields_existing (tgt, n, first, i, hostaddrs, > + gomp_map_fields_existing (tgt, aq, n, first, i, hostaddrs, > sizes, kinds, cbufp); > i--; > continue; > @@ -804,7 +844,7 @@ gomp_map_vars (struct gomp_device_descr *devicep, siz= e_t mapnum, > cur_node.tgt_offset =3D gomp_map_val (tgt, hostaddrs, i - 1); > if (cur_node.tgt_offset) > cur_node.tgt_offset -=3D sizes[i]; > - gomp_copy_host2dev (devicep, > + gomp_copy_host2dev (devicep, aq, > (void *) (n->tgt->tgt_start > + n->tgt_offset > + cur_node.host_start > @@ -825,7 +865,7 @@ gomp_map_vars (struct gomp_device_descr *devicep, siz= e_t mapnum, > k->host_end =3D k->host_start + sizeof (void *); > splay_tree_key n =3D splay_tree_lookup (mem_map, k); > if (n && n->refcount !=3D REFCOUNT_LINK) > - gomp_map_vars_existing (devicep, n, k, &tgt->list[i], > + gomp_map_vars_existing (devicep, aq, n, k, &tgt->list[i], > kind & typemask, cbufp); > else > { > @@ -878,18 +918,19 @@ gomp_map_vars (struct gomp_device_descr *devicep, s= ize_t mapnum, > case GOMP_MAP_FORCE_TOFROM: > case GOMP_MAP_ALWAYS_TO: > case GOMP_MAP_ALWAYS_TOFROM: > - gomp_copy_host2dev (devicep, > + gomp_copy_host2dev (devicep, aq, > (void *) (tgt->tgt_start > + k->tgt_offset), > (void *) k->host_start, > k->host_end - k->host_start, cbufp); > break; > case GOMP_MAP_POINTER: > - gomp_map_pointer (tgt, (uintptr_t) *(void **) k->host_start, > + gomp_map_pointer (tgt, aq, > + (uintptr_t) *(void **) k->host_start, > k->tgt_offset, sizes[i], cbufp); > break; > case GOMP_MAP_TO_PSET: > - gomp_copy_host2dev (devicep, > + gomp_copy_host2dev (devicep, aq, > (void *) (tgt->tgt_start > + k->tgt_offset), > (void *) k->host_start, > @@ -911,7 +952,7 @@ gomp_map_vars (struct gomp_device_descr *devicep, siz= e_t mapnum, > tgt->list[j].always_copy_from =3D false; > if (k->refcount !=3D REFCOUNT_INFINITY) > k->refcount++; > - gomp_map_pointer (tgt, > + gomp_map_pointer (tgt, aq, > (uintptr_t) *(void **) hostaddrs[j], > k->tgt_offset > + ((uintptr_t) hostaddrs[j] > @@ -940,7 +981,7 @@ gomp_map_vars (struct gomp_device_descr *devicep, siz= e_t mapnum, > break; > case GOMP_MAP_FORCE_DEVICEPTR: > assert (k->host_end - k->host_start =3D=3D sizeof (void *)); > - gomp_copy_host2dev (devicep, > + gomp_copy_host2dev (devicep, aq, > (void *) (tgt->tgt_start > + k->tgt_offset), > (void *) k->host_start, > @@ -957,9 +998,8 @@ gomp_map_vars (struct gomp_device_descr *devicep, siz= e_t mapnum, > /* Set link pointer on target to the device address of the > mapped object. */ > void *tgt_addr =3D (void *) (tgt->tgt_start + k->tgt_offset); > - devicep->host2dev_func (devicep->target_id, > - (void *) n->tgt_offset, > - &tgt_addr, sizeof (void *)); > + gomp_copy_host2dev (devicep, aq, (void *) n->tgt_offset, > + &tgt_addr, sizeof (void *), cbufp); > } > array++; > } > @@ -971,7 +1011,7 @@ gomp_map_vars (struct gomp_device_descr *devicep, si= ze_t mapnum, > for (i =3D 0; i < mapnum; i++) > { > cur_node.tgt_offset =3D gomp_map_val (tgt, hostaddrs, i); > - gomp_copy_host2dev (devicep, > + gomp_copy_host2dev (devicep, aq, > (void *) (tgt->tgt_start + i * sizeof (void *)), > (void *) &cur_node.tgt_offset, sizeof (void *), > cbufp); > @@ -982,7 +1022,8 @@ gomp_map_vars (struct gomp_device_descr *devicep, si= ze_t mapnum, > { > long c =3D 0; > for (c =3D 0; c < cbuf.chunk_cnt; ++c) > - gomp_copy_host2dev (devicep, (void *) (tgt->tgt_start + cbuf.chunks[2 *= c]), > + gomp_copy_host2dev (devicep, aq, > + (void *) (tgt->tgt_start + cbuf.chunks[2 * c]), > (char *) cbuf.buf + (cbuf.chunks[2 * c] - cbuf.chunks[0]), > cbuf.chunks[2 * c + 1] - cbuf.chunks[2 * c], NULL); > free (cbuf.buf); > @@ -1001,7 +1042,7 @@ gomp_map_vars (struct gomp_device_descr *devicep, s= ize_t mapnum, > return tgt; > } >=20=20 > -static void > +attribute_hidden void > gomp_unmap_tgt (struct target_mem_desc *tgt) > { > /* Deallocate on target the tgt->tgt_start .. tgt->tgt_end region. */ > @@ -1036,6 +1077,13 @@ gomp_remove_var (struct gomp_device_descr *devicep= , splay_tree_key k) > attribute_hidden void > gomp_unmap_vars (struct target_mem_desc *tgt, bool do_copyfrom) > { > + gomp_unmap_vars_async (tgt, do_copyfrom, NULL); > +} > + > +attribute_hidden void > +gomp_unmap_vars_async (struct target_mem_desc *tgt, bool do_copyfrom, > + struct goacc_asyncqueue *aq) > +{ > struct gomp_device_descr *devicep =3D tgt->device_descr; >=20=20 > if (tgt->list_count =3D=3D 0) > @@ -1071,7 +1119,7 @@ gomp_unmap_vars (struct target_mem_desc *tgt, bool = do_copyfrom) >=20=20 > if ((do_unmap && do_copyfrom && tgt->list[i].copy_from) > || tgt->list[i].always_copy_from) > - gomp_copy_dev2host (devicep, > + gomp_copy_dev2host (devicep, aq, > (void *) (k->host_start + tgt->list[i].offset), > (void *) (k->tgt->tgt_start + k->tgt_offset > + tgt->list[i].offset), > @@ -1137,9 +1185,10 @@ gomp_update (struct gomp_device_descr *devicep, si= ze_t mapnum, void **hostaddrs, > size_t size =3D cur_node.host_end - cur_node.host_start; >=20=20 > if (GOMP_MAP_COPY_TO_P (kind & typemask)) > - gomp_copy_host2dev (devicep, devaddr, hostaddr, size, NULL); > + gomp_copy_host2dev (devicep, NULL, devaddr, hostaddr, size, > + NULL); > if (GOMP_MAP_COPY_FROM_P (kind & typemask)) > - gomp_copy_dev2host (devicep, hostaddr, devaddr, size); > + gomp_copy_dev2host (devicep, NULL, hostaddr, devaddr, size); > } > } > gomp_mutex_unlock (&devicep->lock); > @@ -1924,7 +1985,7 @@ gomp_exit_data (struct gomp_device_descr *devicep, = size_t mapnum, >=20=20 > if ((kind =3D=3D GOMP_MAP_FROM && k->refcount =3D=3D 0) > || kind =3D=3D GOMP_MAP_ALWAYS_FROM) > - gomp_copy_dev2host (devicep, (void *) cur_node.host_start, > + gomp_copy_dev2host (devicep, NULL, (void *) cur_node.host_start, > (void *) (k->tgt->tgt_start + k->tgt_offset > + cur_node.host_start > - k->host_start), Gr=C3=BC=C3=9Fe Thomas