From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 58534 invoked by alias); 20 Jan 2020 14:01:55 -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 58489 invoked by uid 89); 20 Jan 2020 14:01:54 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-20.7 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3 autolearn=ham version=3.3.1 spammy=sure! X-HELO: esa4.mentor.iphmx.com Received: from esa4.mentor.iphmx.com (HELO esa4.mentor.iphmx.com) (68.232.137.252) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 20 Jan 2020 14:01:44 +0000 IronPort-SDR: hLobnXLVJdVQDhikFod18F6iGpVDMooXdK3elcWCAHTvshO5RfSxKVmDInnnqWcBCyZzLVZ0Ln ecoAK0+u7n9qaR14ZebE7GO8hwuOwxe6Dp5Ap2i3kpjQlmailaLmwd071l0BeA+B7R7WeYJhpD H67PJtD2m+WS/xcADY19G+KUjsp6ZmWH9oNQAQge5wgOtpgxUaxjhg2DqMOpbi6zubdwvt9ygJ c3TDaZsFFzEZMNKm38asg9VvExABGVY8HUNirmprlwtMVijgXpkUMsq1UAhLFgaVFcvFM+fMYe jD0= Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa4.mentor.iphmx.com with ESMTP; 20 Jan 2020 06:01:11 -0800 IronPort-SDR: vj7roLpt8+OSfTfOLSOACEhHe7qKvi1h7OUWjKBXB/boGgHJtIQooJOQZWRAYfJUmbeOjHt+c0 MLrNUGWvwY6Q== Subject: Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support To: Thomas Schwinge CC: References: <20191113153215.17750-1-frederik@codesourcery.com> <87imp01jr3.fsf@euler.schwinge.homeip.net> <20191114153531.7493-1-frederik@codesourcery.com> <87v9qfyiyz.fsf@euler.schwinge.homeip.net> <1c2e2d57-31ce-8e4d-c8b9-c2fbc7091e86@codesourcery.com> <87imlbv1ae.fsf@euler.schwinge.homeip.net> From: "Harwath, Frederik" X-Pep-Version: 2.0 Message-ID: <784725d4-c598-a35f-adf3-6be704c39a46@codesourcery.com> Date: Mon, 20 Jan 2020 14:20:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: <87imlbv1ae.fsf@euler.schwinge.homeip.net> Content-Type: multipart/mixed; boundary="------------6EA53AD25F878AB90FF59BBD" Return-Path: frederik@codesourcery.com X-IsSubscribed: yes X-SW-Source: 2020-01/txt/msg01226.txt.bz2 --------------6EA53AD25F878AB90FF59BBD Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-length: 1665 Hi Thomas, I have attached a patch containing the changes that you suggested. On 16.01.20 17:00, Thomas Schwinge wrote: > On 2019-12-20T17:46:57+0100, "Harwath, Frederik" wrote: >> --- /dev/null >> +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-2.c >=20 > I suggest to rename this one to 'acc_get_property-nvptx.c'> [...] >> --- /dev/null >> +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-3.c > I suggest to rename this one to 'acc_get_property-host.c'. I renamed both. > This assumes that the 'cuda*' interfaces and OpenACC/libgomp interfaces > handle/order device numbers in the same way -- which it seems they do, > but just noting this in case this becomes an issue at some point. Correct, I have added a corresponding comment to acc_get_property-nvptx.c. > Aside from improper data types being used for storing/printing the memory > information, we have to expect 'acc_property_free_memory' to change > between two invocations. ;-) Right! I have removed the assertion and changed it into ... >=20 > Better to just verify that 'free_mem >=3D 0' (by means of 'size_t' data > type, I suppose), and 'free_mem <=3D total_mem'? ... this. >=20 > (..., and for avoidance of doubt: I think there's no point in > special-casing this one for 'acc_device_host' where we know that > 'free_mem' is always zero -- this may change in the future.) Sure! But with the new "free_mem <=3D total_mem" assertion and since we ass= ert total_mem =3D=3D 0 and since free_mem >=3D 0, we effectively also assert th= at in the test right now ;-). Ok to push the commit to master? Best regards, Frederik --------------6EA53AD25F878AB90FF59BBD Content-Type: text/x-patch; charset="UTF-8"; name="0001-Fix-expectation-and-types-in-acc_get_property-tests.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename*0="0001-Fix-expectation-and-types-in-acc_get_property-tests.pat"; filename*1="ch" Content-length: 6479 =46rom ef5a959bedc3214e86d6a683a02b693d82847ecd Mon Sep 17 00:00:00 2001 From: Frederik Harwath Date: Mon, 20 Jan 2020 14:07:03 +0100 Subject: [PATCH] Fix expectation and types in acc_get_property tests * Weaken expectation concerning acc_property_free_memory. Do not expect the value returned by CUDA since that value might have changed in the meantime. * Use correct type for the results of calls to acc_get_property in tests. libgomp/ * testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.c (expect_device_properties): Remove "expected_free_mem" argument, change "expected_total_mem" argument type to size_t; change types of acc_get_property results to size_t. * testsuite/libgomp.oacc-c-c++-common/acc_get_property-2.c: Adapt and rename to ... * testsuite/libgomp.oacc-c-c++-common/acc_get_property-nvptx.c: ... this. * testsuite/libgomp.oacc-c-c++-common/acc_get_property-3.c: Adapt and rename to ... * testsuite/libgomp.oacc-c-c++-common/acc_get_property-host.c: ... this. Reviewed-by: Thomas Schwinge --- .../acc_get_property-aux.c | 28 +++++++++---------- ...t_property-3.c =3D> acc_get_property-host.c} | 7 ++--- ..._property-2.c =3D> acc_get_property-nvptx.c} | 9 +++--- 3 files changed, 22 insertions(+), 22 deletions(-) rename libgomp/testsuite/libgomp.oacc-c-c++-common/{acc_get_property-3.c = =3D> acc_get_property-host.c} (63%) rename libgomp/testsuite/libgomp.oacc-c-c++-common/{acc_get_property-2.c = =3D> acc_get_property-nvptx.c} (86%) diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-a= ux.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.c index 952bdbf6aea..76c29501839 100644 --- a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.c +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.c @@ -8,9 +8,8 @@ =20 void expect_device_properties (acc_device_t dev_type, int dev_num, - int expected_total_mem, int expected_free_mem, - const char* expected_vendor, const char* expected_name, - const char* expected_driver) + size_t expected_memory, const char* expected_vendor, + const char* expected_name, const char* expected_driver) { const char *vendor =3D acc_get_property_string (dev_num, dev_type, acc_property_vendor); @@ -21,22 +20,23 @@ void expect_device_properties abort (); } =20 - int total_mem =3D acc_get_property (dev_num, dev_type, - acc_property_memory); - if (total_mem !=3D expected_total_mem) + size_t total_mem =3D acc_get_property (dev_num, dev_type, + acc_property_memory); + if (total_mem !=3D expected_memory) { - fprintf (stderr, "Expected acc_property_memory to equal %d, " - "but was %d.\n", expected_total_mem, total_mem); + fprintf (stderr, "Expected acc_property_memory to equal %zd, " + "but was %zd.\n", expected_memory, total_mem); abort (); =20 } =20 - int free_mem =3D acc_get_property (dev_num, dev_type, + size_t free_mem =3D acc_get_property (dev_num, dev_type, acc_property_free_memory); - if (free_mem !=3D expected_free_mem) + if (free_mem > total_mem) { - fprintf (stderr, "Expected acc_property_free_memory to equal %d, " - "but was %d.\n", expected_free_mem, free_mem); + fprintf (stderr, "Expected acc_property_free_memory <=3D acc_propert= y_memory" + ", but free memory was %zd and total memory was %zd.\n", + free_mem, total_mem); abort (); } =20 @@ -59,11 +59,11 @@ void expect_device_properties } =20 int unknown_property =3D 16058; - int v =3D acc_get_property (dev_num, dev_type, (acc_device_property_t)un= known_property); + size_t v =3D acc_get_property (dev_num, dev_type, (acc_device_property_t= )unknown_property); if (v !=3D 0) { fprintf (stderr, "Expected value of unknown numeric property to equa= l 0, " - "but was %d.\n", v); + "but was %zd.\n", v); abort (); } =20 diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-3= .c b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-host.c similarity index 63% rename from libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-3.c rename to libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-host= .c index 92565000e49..f1cd7cfef39 100644 --- a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-3.c +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-host.c @@ -8,12 +8,11 @@ =20 void expect_device_properties (acc_device_t dev_type, int dev_num, - int expected_total_mem, int expected_free_mem, - const char* expected_vendor, const char* expected_name, - const char* expected_driver); + size_t expected_memory, const char* expected_vendor, + const char* expected_name, const char* expected_driver); =20 int main() { printf ("Checking acc_device_host device properties\n"); - expect_device_properties (acc_device_host, 0, 0, 0, "GNU", "GOMP", "1.0"= ); + expect_device_properties (acc_device_host, 0, 0, "GNU", "GOMP", "1.0"); } diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-2= .c b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-nvptx.c similarity index 86% rename from libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-2.c rename to libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-nvpt= x.c index 4dd13c401d3..0dcaea7c3e8 100644 --- a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-2.c +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-nvptx.c @@ -13,9 +13,8 @@ =20 void expect_device_properties (acc_device_t dev_type, int dev_num, - int expected_total_mem, int expected_free_mem, - const char* expected_vendor, const char* expected_name, - const char* expected_driver); + size_t expected_memory, const char* expected_vendor, + const char* expected_name, const char* expected_driver); =20 int main () { @@ -62,7 +61,9 @@ int main () snprintf (driver, sizeof driver, "CUDA Driver %u.%u", driver_version / 1000, driver_version % 1000 / 10); =20 + /* Note that this check relies on the fact that the device numbering + used by the nvptx plugin agrees with the CUDA device ordering. */ expect_device_properties(acc_device_nvidia, dev_num, - total_mem, free_mem, vendor, p.name, driver); + total_mem, vendor, p.name, driver); } } --=20 2.17.1 --------------6EA53AD25F878AB90FF59BBD--