Hi Thomas, On 23.01.20 15:32, Thomas Schwinge wrote:
> On 2020-01-20T15:01:01+0100, "Harwath, Frederik" <frede...@codesourcery.com> > wrote: >> On 16.01.20 17:00, Thomas Schwinge wrote: >>> On 2019-12-20T17:46:57+0100, "Harwath, Frederik" >>> <frede...@codesourcery.com> wrote: >> Ok to push the commit to master? > > Thanks, OK. Reviewed-by: Thomas Schwinge <tho...@codesourcery.com> Thank you. Committed as 4bd03ed69bd789278a0286017b692f49052ffe5c, including the changes to the size_t formatting. Best regards, Frederik > > > As a low-priority follow-up, please look into: > > > source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.c: > In function 'expect_device_properties': > > source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.c:74:24: > warning: format '%d' expects argument of type 'int', but argument 3 has type > 'const char *' [-Wformat=] > 74 | fprintf (stderr, "Expected value of unknown string property > to be NULL, " > | > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 75 | "but was %d.\n", s); > | ~ > | | > | const char * > > source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.c:75:19: > note: format string is defined here > 75 | "but was %d.\n", s); > | ~^ > | | > | int > | %s > > ..., and (random example): > >> int unknown_property = 16058; >> - int v = acc_get_property (dev_num, dev_type, >> (acc_device_property_t)unknown_property); >> + size_t v = acc_get_property (dev_num, dev_type, >> (acc_device_property_t)unknown_property); >> if (v != 0) >> { >> fprintf (stderr, "Expected value of unknown numeric property to equal >> 0, " >> - "but was %d.\n", v); >> + "but was %zd.\n", v); >> abort (); >> } > > ..., shouldn't that be '%zu' given that 'size_t' is 'unsigned'? > > libgomp.oacc-c-c++-common/acc_get_property-aux.c: fprintf (stderr, > "Expected acc_property_memory to equal %zd, " > libgomp.oacc-c-c++-common/acc_get_property-aux.c: "but was > %zd.\n", expected_memory, total_mem); > libgomp.oacc-c-c++-common/acc_get_property-aux.c: ", but free > memory was %zd and total memory was %zd.\n", > libgomp.oacc-c-c++-common/acc_get_property-aux.c: "but was > %zd.\n", v); > libgomp.oacc-c-c++-common/acc_get_property.c: printf (" Total > memory: %zd\n", v); > libgomp.oacc-c-c++-common/acc_get_property.c: printf (" Free > memory: %zd\n", v); > > > Grüße > Thomas >
From 4bd03ed69bd789278a0286017b692f49052ffe5c Mon Sep 17 00:00:00 2001 From: Frederik Harwath <frede...@codesourcery.com> Date: Mon, 20 Jan 2020 14:07:03 +0100 Subject: [PATCH 1/2] 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, adapt format strings. * testsuite/libgomp.oacc-c-c++-common/acc_get_property.c: Use %zu instead of %zd to print size_t values. * 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 <tho...@codesourcery.com> --- .../acc_get_property-aux.c | 30 +++++++++---------- ...t_property-3.c => acc_get_property-host.c} | 7 ++--- ..._property-2.c => acc_get_property-nvptx.c} | 9 +++--- .../acc_get_property.c | 4 +-- 4 files changed, 25 insertions(+), 25 deletions(-) rename libgomp/testsuite/libgomp.oacc-c-c++-common/{acc_get_property-3.c => acc_get_property-host.c} (63%) rename libgomp/testsuite/libgomp.oacc-c-c++-common/{acc_get_property-2.c => acc_get_property-nvptx.c} (86%) diff --git 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 index 952bdbf6aea..6bb01250148 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 @@ 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 = acc_get_property_string (dev_num, dev_type, acc_property_vendor); @@ -21,22 +20,23 @@ void expect_device_properties abort (); } - int total_mem = acc_get_property (dev_num, dev_type, - acc_property_memory); - if (total_mem != expected_total_mem) + size_t total_mem = acc_get_property (dev_num, dev_type, + acc_property_memory); + if (total_mem != 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 %zu, " + "but was %zu.\n", expected_memory, total_mem); abort (); } - int free_mem = acc_get_property (dev_num, dev_type, + size_t free_mem = acc_get_property (dev_num, dev_type, acc_property_free_memory); - if (free_mem != 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 <= acc_property_memory" + ", but free memory was %zu and total memory was %zu.\n", + free_mem, total_mem); abort (); } @@ -59,11 +59,11 @@ void expect_device_properties } int unknown_property = 16058; - int v = acc_get_property (dev_num, dev_type, (acc_device_property_t)unknown_property); + size_t v = acc_get_property (dev_num, dev_type, (acc_device_property_t)unknown_property); if (v != 0) { fprintf (stderr, "Expected value of unknown numeric property to equal 0, " - "but was %d.\n", v); + "but was %zu.\n", v); abort (); } @@ -72,7 +72,7 @@ void expect_device_properties if (s != NULL) { fprintf (stderr, "Expected value of unknown string property to be NULL, " - "but was %d.\n", s); + "but was %s.\n", s); abort (); } 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 @@ 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); 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-nvptx.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 @@ 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); int main () { @@ -62,7 +61,9 @@ int main () snprintf (driver, sizeof driver, "CUDA Driver %u.%u", driver_version / 1000, driver_version % 1000 / 10); + /* 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); } } diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property.c index 289d1bab7f8..388c66c1319 100644 --- a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property.c +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property.c @@ -35,10 +35,10 @@ print_device_properties(acc_device_t type) } v = acc_get_property (i, type, acc_property_memory); - printf (" Total memory: %zd\n", v); + printf (" Total memory: %zu\n", v); v = acc_get_property (i, type, acc_property_free_memory); - printf (" Free memory: %zd\n", v); + printf (" Free memory: %zu\n", v); s = acc_get_property_string (i, type, acc_property_name); printf (" Name: %s\n", s); -- 2.17.1
From 9e424d974974c18b432c96f0df31b79a857819e6 Mon Sep 17 00:00:00 2001 From: Frederik Harwath <frede...@codesourcery.com> Date: Fri, 24 Jan 2020 09:22:54 +0100 Subject: [PATCH 2/2] Add missing ChangeLog entry for my last commit libgomp/ChangeLog: Add entry for commit 4bd03ed69bd. --- libgomp/ChangeLog | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/libgomp/ChangeLog b/libgomp/ChangeLog index c0dde5cec3c..721627080cd 100644 --- a/libgomp/ChangeLog +++ b/libgomp/ChangeLog @@ -1,3 +1,19 @@ +2020-01-24 Frederik Harwath <frede...@codesourcery.com> + + * 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, + adapt format strings. + * testsuite/libgomp.oacc-c-c++-common/acc_get_property.c: + Use %zu instead of %zd to print size_t values. + * 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. + 2020-01-23 Andrew Stubbs <a...@codesourcery.com> * plugin/plugin-gcn.c (parse_target_attributes): Use correct mask for -- 2.17.1