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

Reply via email to