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" <frede...@codesourcery.com>
> wrote:
>> --- /dev/null
>> +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-2.c
>
> 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 ...
>
> Better to just verify that 'free_mem >= 0' (by means of 'size_t' data
> type, I suppose), and 'free_mem <= total_mem'?
... this.
>
> (..., 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 <= total_mem" assertion and since we assert
total_mem == 0 and since free_mem >= 0, we effectively also assert that in the
test right now ;-).
Ok to push the commit to master?
Best regards,
Frederik
From ef5a959bedc3214e86d6a683a02b693d82847ecd Mon Sep 17 00:00:00 2001
From: Frederik Harwath <frede...@codesourcery.com>
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 <tho...@codesourcery.com>
---
.../acc_get_property-aux.c | 28 +++++++++----------
...t_property-3.c => acc_get_property-host.c} | 7 ++---
..._property-2.c => 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 => 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..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 @@
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 %zd, "
+ "but was %zd.\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 %zd and total memory was %zd.\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 %zd.\n", v);
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);
}
}
--
2.17.1
