From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 105396 invoked by alias); 29 Nov 2019 14:41:32 -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 105380 invoked by uid 89); 29 Nov 2019 14:41:32 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.2 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT autolearn=ham version=3.3.1 spammy=Exercise, black X-HELO: esa2.mentor.iphmx.com Received: from esa2.mentor.iphmx.com (HELO esa2.mentor.iphmx.com) (68.232.141.98) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 29 Nov 2019 14:41:30 +0000 IronPort-SDR: bMFTPNQ31/OPguVPN/A4b8/8BPreUQdZB1jH53m87HFS2FKkT71360yi3EkIwOjF6SueR9Eipu wE+kEi1nETA/QmRJ/cLdv4JdrqmdnWS6yX2wVy+R2GFiOmJJlmngqWKezMFcTSjctNqq4uL39n 7BGny0TN7S8G6o+k9FrP05i2YeXBWpTO3MXyK+TIazCALfLrUmzQSZR1CQaPocNkLEbrdtEatk VOhIYL8nTGyVHsDiF1Eh+Dc1Nm4wGvQD6OkiLsPZNimPPi8k6+Kq4f8EXbGMKxzG7KvJKJczUn 9So= Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa2.mentor.iphmx.com with ESMTP; 29 Nov 2019 06:41:28 -0800 IronPort-SDR: 9ugHvycQnHSg5gF9D+z0Gvbg6Tdc2DrO4EDXHeND2TWSNOiwJn57N3pZzNipi8ZMSkZ2ISCwlA 6EslLUU0Txr3atdB4uU3CRFiVLP1+o/wbPsVxHUcZui6+j2w2L2oOkeDw9EY7YUQDn0uQMwdTL m1s/6pra5dI/2BkruVHOehi2/Zf+QhtsLpKVDCP1SIvpT8W1HrXst8n60TN8qKwladSg8oaUzN 5fittBadzKvv1mtnit8cuz//0U1UYpxP/qS99JnQ66w584esyWRzXJdvaxsQ42rBLO/j1G+mKu wuQ= From: Thomas Schwinge To: Tobias Burnus CC: Kwok Cheung Yeung , , , Subject: [PR92726] OpenACC: 'NULL'-in -> no-op, and/or 'NULL'-out (was: [PATCH 1/5, OpenACC] Allow NULL as an argument to OpenACC 2.6 directives) In-Reply-To: References: <75222f0b-b55d-dcd9-b4f4-8e218675e8d7@mentor.com> <6aaaeec8-9c6d-9293-5b6c-622d9fcf2664@codesourcery.com> User-Agent: Notmuch/0.29.1+93~g67ed7df (https://notmuchmail.org) Emacs/26.1 (x86_64-pc-linux-gnu) Date: Fri, 29 Nov 2019 14:42:00 -0000 Message-ID: <87lfrylp8k.fsf@euler.schwinge.homeip.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Return-Path: tschwing@mentor.com X-SW-Source: 2019-11/txt/msg02661.txt.bz2 --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Content-length: 7233 Hi Tobias! Reviewing your "[Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments" reminded me that still this behavioral change has not been split out, cited below, that you described as "trivial". I've just filed "OpenACC: 'NULL'-in -> no-op, and/or 'NULL'-out", so please reference that one in the ChangeLog updates. So, eventually that'll be more than just 'libgomp/oacc-mem.c:update_dev_host', but I understand we need that one now, for Fortran optional arguments support. Any other changes can then be handled later (once the OpenACC specification changes have been completed). Please also add a new test case 'libgomp.oacc-c-c++-common/null-1.c' with a "Test 'NULL'-in -> no-op, and/or 'NULL'-out" header, executing things like 'acc_update_device (NULL, [...])' etc. for everything that calls 'update_dev_host': 'acc_update_device', 'acc_update_device_async', 'acc_update_self', 'acc_update_self_async'. These functions are also called for OpenACC 'update' directives ('libgomp/oacc-parallel.c:GOACC_update'), but I suppose it's not possible to construct an OpenACC 'update' directive conveying a 'NULL' pointer, that is, something that would result in 'hostaddrs[i] =3D=3D NULL'? Likewi= se for Fortran, I suppose. In 'libgomp/libgomp.texi' then add a note to 'acc_update_device' (and other relevant functions) like this: @@ -2586,6 +2586,9 @@ This function updates the device copy from the pr= eviously mapped host memory. The host memory is specified with the host address @var{a} and a lengt= h of @var{len} bytes. =20=20=20=20 +If @var{a} is the @code{NULL} pointer, this is a no-op. + [...] As for 'libgomp/oacc-mem.c:gomp_acc_insert_pointer', that's only called for OpenACC 'enter data' directives ('libgomp/oacc-parallel.c:GOACC_enter_exit_data'), and specifically only for 'GOMP_MAP_POINTER', 'GOMP_MAP_TO_PSET'. Is there a way to construct a test case that will result in a 'NULL' pointer there, other than via Fortran optional arguments? If not, then that hunk should be removed here, and move into/stay in the Fortran optional arguments patch that you've posted. (And that said, Julian's got a patch pending review that gets rid of 'gomp_acc_insert_pointer' and other such black magic, yay.) Gr=C3=BC=C3=9Fe Thomas On 2019-07-12T12:35:05+0100, Kwok Cheung Yeung wrote: > Fortran pass-by-reference optional arguments behave much like normal=20 > Fortran arguments when lowered to GENERIC/GIMPLE, except they can be=20 > null (representing a non-present argument). > > Some parts of libgomp (those dealing with updating mappings) currently=20 > do not expect to take a null address and fail. These need to be changed=20 > to deal with the null appropriately, by turning the operation into a=20 > no-op (as you never need to update a non-present argument). > > libgomp/ > * oacc-mem.c (update_dev_host): Return early if the host address > is NULL. > (gomp_acc_insert_pointer): Likewise. > * testsuite/libgomp.oacc-c-c++-common/lib-43.c: Remove. > * testsuite/libgomp.oacc-c-c++-common/lib-47.c: Likewise. > --- > libgomp/oacc-mem.c | 9 ++++ > .../testsuite/libgomp.oacc-c-c++-common/lib-43.c | 51 ---------------= ------- > .../testsuite/libgomp.oacc-c-c++-common/lib-47.c | 49 ---------------= ------ > 3 files changed, 9 insertions(+), 100 deletions(-) > delete mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/lib-43.c > delete mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/lib-47.c > > diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c > index 2f27100..8cc5120 100644 > --- a/libgomp/oacc-mem.c > +++ b/libgomp/oacc-mem.c > @@ -831,6 +831,12 @@ update_dev_host (int is_dev, void *h, size_t s, int = async) > if (acc_dev->capabilities & GOMP_OFFLOAD_CAP_SHARED_MEM) > return; > > + /* Fortran optional arguments that are non-present result in a > + null host address here. This can safely be ignored as it is > + not possible to 'update' a non-present optional argument. */ > + if (h =3D=3D NULL) > + return; > + > acc_prof_info prof_info; > acc_api_info api_info; > bool profiling_p =3D GOACC_PROFILING_SETUP_P (thr, &prof_info, &api_i= nfo); > @@ -901,6 +907,9 @@ gomp_acc_insert_pointer (size_t mapnum, void **hostad= drs, size_t *sizes, > struct goacc_thread *thr =3D goacc_thread (); > struct gomp_device_descr *acc_dev =3D thr->dev; > > + if (*hostaddrs =3D=3D NULL) > + return; > + > if (acc_is_present (*hostaddrs, *sizes)) > { > splay_tree_key n; > diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-43.c b/libgo= mp/testsuite/libgomp.oacc-c-c++-common/lib-43.c > deleted file mode 100644 > index 5db2912..0000000 > --- a/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-43.c > +++ /dev/null > @@ -1,51 +0,0 @@ > -/* Exercise acc_update_device with a NULL data address on nvidia targets= . */ > - > -/* { dg-do run { target openacc_nvidia_accel_selected } } */ > - > -#include > -#include > -#include > - > -int > -main (int argc, char **argv) > -{ > - const int N =3D 256; > - int i; > - unsigned char *h; > - void *d; > - > - h =3D (unsigned char *) malloc (N); > - > - for (i =3D 0; i < N; i++) > - { > - h[i] =3D i; > - } > - > - d =3D acc_copyin (h, N); > - if (!d) > - abort (); > - > - for (i =3D 0; i < N; i++) > - { > - h[i] =3D 0xab; > - } > - > - fprintf (stderr, "CheCKpOInT\n"); > - acc_update_device (0, N); > - > - acc_copyout (h, N); > - > - for (i =3D 0; i < N; i++) > - { > - if (h[i] !=3D 0xab) > - abort (); > - } > - > - free (h); > - > - return 0; > -} > - > -/* { dg-output "CheCKpOInT(\n|\r\n|\r).*" } */ > -/* { dg-output "\\\[\[^\n\r]*,256\\\] is not mapped" } */ > -/* { dg-shouldfail "" } */ > diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-47.c b/libgo= mp/testsuite/libgomp.oacc-c-c++-common/lib-47.c > deleted file mode 100644 > index c214042..0000000 > --- a/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-47.c > +++ /dev/null > @@ -1,49 +0,0 @@ > -/* Exercise acc_update_self with a NULL data mapping on nvidia targets. = */ > - > -/* { dg-do run { target openacc_nvidia_accel_selected } } */ > - > -#include > -#include > -#include > -#include > - > -int > -main (int argc, char **argv) > -{ > - const int N =3D 256; > - int i; > - unsigned char *h; > - void *d; > - > - h =3D (unsigned char *) malloc (N); > - > - for (i =3D 0; i < N; i++) > - { > - h[i] =3D i; > - } > - > - d =3D acc_copyin (h, N); > - if (!d) > - abort (); > - > - memset (&h[0], 0, N); > - > - fprintf (stderr, "CheCKpOInT\n"); > - acc_update_self (0, N); > - > - for (i =3D 0; i < N; i++) > - { > - if (h[i] !=3D i) > - abort (); > - } > - > - acc_delete (h, N); > - > - free (h); > - > - return 0; > -} > - > -/* { dg-output "CheCKpOInT(\n|\r\n|\r).*" } */ > -/* { dg-output "\\\[\[^\n\r]*,256\\\] is not mapped" } */ > -/* { dg-shouldfail "" } */ --=-=-= Content-Type: application/pgp-signature; name="signature.asc" Content-length: 658 -----BEGIN PGP SIGNATURE----- iQGzBAEBCgAdFiEEU9WEfWKGQazCmycCAKI7+41Q4XkFAl3hLfsACgkQAKI7+41Q 4XlnCAwAxs1tdaIkslvSwG8mIqvtNawBxX7xuiKJlNVPe6veIva4Oz4lw/MghOKx NKYrrhq6/6k8KsWd3IRMN6vWpJSab5C9Rk+sHRyn96lRRrEqHDy79rCXcUeZpza8 sqS7zKKCbz8213QkKC8psPbi5gGnmJEqjkjLhDKwMsbPdXMnC7Jj2d9SZaEdPBE4 4SjHCwIPkDqZacgaeOnGCLK85T5/B/ksF253K7F+ptpjqrCTMekt+5OOFxXs4nJL 35upzI6PdnfX0xBwr3EAkjfNl9uFnsbz8vKtFCxZvfkWlJhbBnx3CQIOpvX+QvMg Poch6j7SVU7cEPNJD9GSHqiVlj3fXkf/cq5ZYcOGDoO2PKlYSKvp3njqRTwYPLwm 2nyq8zHDqS4YqRETsB1KTvly5O648hmD2vyNMD+gDrxxHScA0FUw2taUyqF7JPtw 763+5wOQxlC3s31bt/2ZuV9R6SYbfT34zXAF4FlSaTYL4ApFVgsQDciIFyyo9W0Q jhTiNtlg =yXPC -----END PGP SIGNATURE----- --=-=-=--