Hi Andrew,
On 28.01.20 16:42, Andrew Stubbs wrote:
> On 28/01/2020 14:55, Harwath, Frederik wrote:
>
> If we're going to use a fixed-size buffer then we should use snprintf and
> emit GCN_WARNING if the return value is greater than
> "sizeof(driver_version_s)", even though that is unlikely. Do the same in the
> testcase, but use a bigger buffer so that truncation causes a mismatch and
> test failure.
Ok.
> I realise that an existing function in this testcase uses this layout, but
> the code style does not normally have the parameter list on the next line,
> and certainly not in column 1.
Ok. I have also adjusted the formatting in the other acc_get_property tests to
the code style. I have turned this into a separate trivial patch.
Ok to commit the revised patch?
Best regards,
Frederik
From fb15cb9058feeda8891d6454d32f43fda885b789 Mon Sep 17 00:00:00 2001
From: Frederik Harwath <frede...@codesourcery.com>
Date: Wed, 29 Jan 2020 10:19:50 +0100
Subject: [PATCH 1/2] Add OpenACC acc_get_property support for AMD GCN
Add full support for the OpenACC 2.6 acc_get_property and
acc_get_property_string functions to the libgomp GCN plugin.
libgomp/
* plugin-gcn.c (struct agent_info): Add fields "name" and
"vendor_name" ...
(GOMP_OFFLOAD_init_device): ... and init from here.
(struct hsa_context_info): Add field "driver_version_s" ...
(init_hsa_contest): ... and init from here.
(GOMP_OFFLOAD_openacc_get_property): Replace stub with a proper
implementation.
* testsuite/libgomp.oacc-c-c++-common/acc_get_property.c:
Enable test execution for amdgcn and host offloading targets.
* testsuite/libgomp.oacc-fortran/acc_get_property.f90: Likewise.
* testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.c
(expect_device_properties): Split function into ...
(expect_device_string_properties): ... this new function ...
(expect_device_memory): ... and this new function.
* testsuite/libgomp.oacc-c-c++-common/acc_get_property-gcn.c:
Add test.
---
libgomp/plugin/plugin-gcn.c | 71 ++++++++--
.../acc_get_property-aux.c | 79 ++++++-----
.../acc_get_property-gcn.c | 132 ++++++++++++++++++
.../acc_get_property.c | 5 +-
.../libgomp.oacc-fortran/acc_get_property.f90 | 2 -
5 files changed, 242 insertions(+), 47 deletions(-)
create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-gcn.c
diff --git a/libgomp/plugin/plugin-gcn.c b/libgomp/plugin/plugin-gcn.c
index 7854c142f05..45c625495b9 100644
--- a/libgomp/plugin/plugin-gcn.c
+++ b/libgomp/plugin/plugin-gcn.c
@@ -425,7 +425,10 @@ struct agent_info
/* The instruction set architecture of the device. */
gcn_isa device_isa;
-
+ /* Name of the agent. */
+ char name[64];
+ /* Name of the vendor of the agent. */
+ char vendor_name[64];
/* Command queues of the agent. */
hsa_queue_t *sync_queue;
struct goacc_asyncqueue *async_queues, *omp_async_queue;
@@ -544,6 +547,8 @@ struct hsa_context_info
int agent_count;
/* Array of agent_info structures describing the individual HSA agents. */
struct agent_info *agents;
+ /* Driver version string. */
+ char driver_version_s[30];
};
/* Format of the on-device heap.
@@ -1513,6 +1518,23 @@ init_hsa_context (void)
GOMP_PLUGIN_error ("Failed to list all HSA runtime agents");
}
+ uint16_t minor, major;
+ status = hsa_fns.hsa_system_get_info_fn (HSA_SYSTEM_INFO_VERSION_MINOR, &minor);
+ if (status != HSA_STATUS_SUCCESS)
+ GOMP_PLUGIN_error ("Failed to obtain HSA runtime minor version");
+ status = hsa_fns.hsa_system_get_info_fn (HSA_SYSTEM_INFO_VERSION_MAJOR, &major);
+ if (status != HSA_STATUS_SUCCESS)
+ GOMP_PLUGIN_error ("Failed to obtain HSA runtime major version");
+
+ size_t len = sizeof hsa_context.driver_version_s;
+ int printed = snprintf (hsa_context.driver_version_s, len,
+ "HSA Runtime %hu.%hu", (unsigned short int)major,
+ (unsigned short int)minor);
+ if (printed >= len)
+ GCN_WARNING ("HSA runtime version string was truncated."
+ "Version %hu.%hu is too long.", (unsigned short int)major,
+ (unsigned short int)minor);
+
hsa_context.initialized = true;
return true;
}
@@ -3410,15 +3432,19 @@ GOMP_OFFLOAD_init_device (int n)
return hsa_error ("Error requesting maximum queue size of the GCN agent",
status);
- char buf[64];
status = hsa_fns.hsa_agent_get_info_fn (agent->id, HSA_AGENT_INFO_NAME,
- &buf);
+ &agent->name);
if (status != HSA_STATUS_SUCCESS)
return hsa_error ("Error querying the name of the agent", status);
- agent->device_isa = isa_code (buf);
+ agent->device_isa = isa_code (agent->name);
if (agent->device_isa < 0)
- return hsa_error ("Unknown GCN agent architecture.", HSA_STATUS_ERROR);
+ return hsa_error ("Unknown GCN agent architecture", HSA_STATUS_ERROR);
+
+ status = hsa_fns.hsa_agent_get_info_fn (agent->id, HSA_AGENT_INFO_VENDOR_NAME,
+ &agent->vendor_name);
+ if (status != HSA_STATUS_SUCCESS)
+ return hsa_error ("Error querying the vendor name of the agent", status);
status = hsa_fns.hsa_queue_create_fn (agent->id, queue_size,
HSA_QUEUE_TYPE_MULTI,
@@ -4115,12 +4141,37 @@ GOMP_OFFLOAD_openacc_async_dev2host (int device, void *dst, const void *src,
union goacc_property_value
GOMP_OFFLOAD_openacc_get_property (int device, enum goacc_property prop)
{
- /* Stub. Check device and return default value for unsupported properties. */
- /* TODO: Implement this function. */
- get_agent_info (device);
+ struct agent_info *agent = get_agent_info (device);
+
+ union goacc_property_value propval = { .val = 0 };
+
+ switch (prop)
+ {
+ case GOACC_PROPERTY_FREE_MEMORY:
+ /* Not supported. */
+ break;
+ case GOACC_PROPERTY_MEMORY:
+ {
+ size_t size;
+ hsa_region_t region = agent->data_region;
+ hsa_status_t status =
+ hsa_fns.hsa_region_get_info_fn (region, HSA_REGION_INFO_SIZE, &size);
+ if (status == HSA_STATUS_SUCCESS)
+ propval.val = size;
+ break;
+ }
+ case GOACC_PROPERTY_NAME:
+ propval.ptr = agent->name;
+ break;
+ case GOACC_PROPERTY_VENDOR:
+ propval.ptr = agent->vendor_name;
+ break;
+ case GOACC_PROPERTY_DRIVER:
+ propval.ptr = hsa_context.driver_version_s;
+ break;
+ }
- union goacc_property_value nullval = { .val = 0 };
- return nullval;
+ return propval;
}
/* Set up plugin-specific thread-local-data (host-side). */
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 6bb01250148..f43f75b364a 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
@@ -6,39 +6,20 @@
#include <stdio.h>
#include <string.h>
-void expect_device_properties
-(acc_device_t dev_type, int dev_num,
- 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);
- if (strcmp (vendor, expected_vendor))
- {
- fprintf (stderr, "Expected acc_property_vendor to equal \"%s\", "
- "but was \"%s\".\n", expected_vendor, vendor);
- abort ();
- }
-
- 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 %zu, "
- "but was %zu.\n", expected_memory, total_mem);
- abort ();
-
- }
- size_t free_mem = acc_get_property (dev_num, dev_type,
- acc_property_free_memory);
- if (free_mem > total_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 ();
- }
+void expect_device_string_properties (acc_device_t dev_type, int dev_num,
+ 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);
+if (strcmp (vendor, expected_vendor))
+{
+ fprintf (stderr, "Expected acc_property_vendor to equal \"%s\", "
+ "but was \"%s\".\n", expected_vendor, vendor);
+ abort ();
+}
const char *name = acc_get_property_string (dev_num, dev_type,
acc_property_name);
@@ -75,6 +56,40 @@ void expect_device_properties
"but was %s.\n", s);
abort ();
}
+}
+
+void expect_device_memory (acc_device_t dev_type, int dev_num,
+ size_t expected_total_memory)
+{
+
+ size_t total_mem = acc_get_property (dev_num, dev_type,
+ acc_property_memory);
+
+ if (total_mem != expected_total_memory)
+ {
+ fprintf (stderr, "Expected acc_property_memory to equal %zu, "
+ "but was %zu.\n", expected_total_memory, total_mem);
+ abort ();
+ }
+ size_t free_mem = acc_get_property (dev_num, dev_type,
+ acc_property_free_memory);
+ if (free_mem > total_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 ();
+ }
+}
+void expect_device_properties (acc_device_t dev_type, int dev_num,
+ size_t expected_total_memory,
+ const char* expected_vendor,
+ const char* expected_name,
+ const char* expected_driver)
+{
+ expect_device_string_properties (dev_type, dev_num, expected_vendor,
+ expected_name, expected_driver);
+ expect_device_memory (dev_type, dev_num, expected_total_memory);
}
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-gcn.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-gcn.c
new file mode 100644
index 00000000000..059b503bbf6
--- /dev/null
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-gcn.c
@@ -0,0 +1,132 @@
+/* Test the `acc_get_property' and `acc_get_property_string' library
+ functions on amdgcn devices by comparing property values with
+ those obtained through the HSA API. */
+/* { dg-additional-sources acc_get_property-aux.c } */
+/* { dg-additional-options "-ldl" } */
+/* { dg-do run { target openacc_amdgcn_accel_selected } } */
+
+#include <dlfcn.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <openacc.h>
+
+#ifndef __cplusplus
+typedef int bool;
+#endif
+#include <hsa.h>
+
+
+void expect_device_string_properties (acc_device_t dev_type, int dev_num,
+ const char* expected_vendor,
+ const char* expected_name,
+ const char* expected_driver);
+
+hsa_status_t (*hsa_agent_get_info_fn) (hsa_agent_t agent,
+ hsa_agent_info_t attribute,
+ void *value);
+hsa_status_t (*hsa_system_get_info_fn) (hsa_system_info_t attribute,
+ void *value);
+hsa_status_t (*hsa_iterate_agents_fn)
+(hsa_status_t (*callback)(hsa_agent_t agent, void *data), void *data);
+hsa_status_t (*hsa_init_fn) (void);
+
+char* support_cpu_devices;
+
+void test_setup ()
+{
+ char* env_runtime;
+ char* hsa_runtime_lib;
+ void *handle;
+
+#define DLSYM_FN(function) \
+ function##_fn = (typeof(function##_fn))dlsym (handle, #function); \
+ if (function##_fn == NULL) \
+ { \
+ fprintf (stderr, "Could not get symbol " #function ".\n"); \
+ abort (); \
+ }
+
+ env_runtime = getenv ("HSA_RUNTIME_LIB");
+ hsa_runtime_lib = env_runtime ? env_runtime : (char*)"libhsa-runtime64.so";
+
+ handle = dlopen (hsa_runtime_lib, RTLD_LAZY);
+ if (!handle)
+ {
+ fprintf (stderr, "Could not load %s.\n", hsa_runtime_lib);
+ abort ();
+ }
+
+ DLSYM_FN (hsa_agent_get_info)
+ DLSYM_FN (hsa_system_get_info)
+ DLSYM_FN (hsa_iterate_agents)
+ DLSYM_FN (hsa_init)
+
+ hsa_init_fn ();
+
+ support_cpu_devices = getenv ("GCN_SUPPORT_CPU_DEVICES");
+}
+
+static hsa_status_t check_agent_properties (hsa_agent_t agent, void *dev_num_arg)
+{
+
+ char name[64];
+ char vendor_name[64];
+ uint16_t minor;
+ uint16_t major;
+ char driver[60];
+
+ hsa_status_t status;
+ hsa_device_type_t device_type;
+ int* dev_num = (int*)dev_num_arg;
+
+#define AGENT_GET_INFO(info_type, val) \
+ status = hsa_agent_get_info_fn (agent, info_type, &val); \
+ if (status != HSA_STATUS_SUCCESS) \
+ { \
+ fprintf (stderr, "Failed to obtain " #info_type ".\n"); \
+ abort (); \
+ }
+#define SYSTEM_GET_INFO(info_type, val) \
+ status = hsa_system_get_info_fn (info_type, &val); \
+ if (status != HSA_STATUS_SUCCESS) \
+ { \
+ fprintf (stderr, "Failed to obtain " #info_type ".\n"); \
+ abort (); \
+ }
+
+ AGENT_GET_INFO (HSA_AGENT_INFO_DEVICE, device_type)
+
+ /* Skip unsupported device types. Mimic the GCN plugin's behavior. */
+ if (!(device_type == HSA_DEVICE_TYPE_GPU
+ || (support_cpu_devices && device_type == HSA_DEVICE_TYPE_CPU)))
+ return HSA_STATUS_SUCCESS;
+
+ AGENT_GET_INFO (HSA_AGENT_INFO_NAME, name)
+ AGENT_GET_INFO (HSA_AGENT_INFO_VENDOR_NAME, vendor_name)
+
+ SYSTEM_GET_INFO (HSA_SYSTEM_INFO_VERSION_MINOR, minor)
+ SYSTEM_GET_INFO (HSA_SYSTEM_INFO_VERSION_MAJOR, major)
+
+ snprintf (driver, sizeof driver, "HSA Runtime %hu.%hu",
+ (unsigned short int)major, (unsigned short int)minor);
+
+ expect_device_string_properties(acc_device_radeon, *dev_num,
+ vendor_name, name, driver);
+
+ (*dev_num)++;
+
+ return status;
+}
+
+int main ()
+{
+ int dev_num = 0;
+ test_setup ();
+
+ hsa_status_t status =
+ hsa_iterate_agents_fn (&check_agent_properties, &dev_num);
+
+ return status;
+}
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 388c66c1319..1984ad3a28d 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
@@ -3,8 +3,7 @@
of all device types mentioned in the OpenACC standard.
See also acc_get_property.f90. */
-/* { dg-do run { target { { ! { openacc_host_selected } } && { ! { openacc_amdgcn_accel_selected } } } } } */
-/* FIXME: This test does not work with the GCN implementation stub yet. */
+/* { dg-do run } */
#include <openacc.h>
#include <stdlib.h>
@@ -20,7 +19,7 @@ print_device_properties(acc_device_t type)
const char *s;
size_t v;
- int dev_count = acc_get_num_devices(type);
+ int dev_count = acc_get_num_devices (type);
for (int i = 0; i < dev_count; ++i)
{
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/acc_get_property.f90 b/libgomp/testsuite/libgomp.oacc-fortran/acc_get_property.f90
index ce695475ae4..80ae292f41f 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/acc_get_property.f90
+++ b/libgomp/testsuite/libgomp.oacc-fortran/acc_get_property.f90
@@ -3,8 +3,6 @@
! of all device types mentioned in the OpenACC standard.
!
! See also acc_get_property.c
-! { dg-do run { target { { ! { openacc_host_selected } } && { ! { openacc_amdgcn_accel_selected } } } } }
-! FIXME: This test does not work with the GCN implementation stub yet.
program test
use openacc
--
2.17.1
From fd9cdadbbada9ec7f0f108991937455952df7697 Mon Sep 17 00:00:00 2001
From: Frederik Harwath <frede...@codesourcery.com>
Date: Wed, 29 Jan 2020 10:21:18 +0100
Subject: [PATCH 2/2] Adjust formatting of acc_get_property tests
libgomp/
* testsuite/libgomp.oacc-c-c++-common/acc_get_property-host.c:
Adjust to GNU coding style.
* testsuite/libgomp.oacc-c-c++-common/acc_get_property-nvptx.c: Likewise.
* testsuite/libgomp.oacc-c-c++-common/acc_get_property.c: Likewise.
---
.../acc_get_property-host.c | 11 +++++-----
.../acc_get_property-nvptx.c | 21 ++++++++++---------
.../acc_get_property.c | 20 +++++++++---------
3 files changed, 27 insertions(+), 25 deletions(-)
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-host.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-host.c
index f1cd7cfef39..bb80f3f52a3 100644
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-host.c
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-host.c
@@ -6,12 +6,13 @@
#include <openacc.h>
#include <stdio.h>
-void expect_device_properties
-(acc_device_t dev_type, int dev_num,
- size_t expected_memory, const char* expected_vendor,
- const char* expected_name, const char* expected_driver);
+void expect_device_properties (acc_device_t dev_type, int dev_num,
+ size_t expected_memory,
+ const char* expected_vendor,
+ const char* expected_name,
+ const char* expected_driver);
-int main()
+int main ()
{
printf ("Checking acc_device_host device properties\n");
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-nvptx.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-nvptx.c
index 0dcaea7c3e8..3e1b51036bc 100644
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-nvptx.c
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-nvptx.c
@@ -11,10 +11,11 @@
#include <string.h>
#include <stdio.h>
-void expect_device_properties
-(acc_device_t dev_type, int dev_num,
- size_t expected_memory, const char* expected_vendor,
- const char* expected_name, const char* expected_driver);
+void expect_device_properties (acc_device_t dev_type, int dev_num,
+ size_t expected_memory,
+ const char* expected_vendor,
+ const char* expected_name,
+ const char* expected_driver);
int main ()
{
@@ -29,26 +30,26 @@ int main ()
abort ();
}
- printf("Checking device %d\n", dev_num);
+ printf ("Checking device %d\n", dev_num);
const char *vendor = "Nvidia";
size_t free_mem;
size_t total_mem;
- if (cudaMemGetInfo(&free_mem, &total_mem) != cudaSuccess)
+ if (cudaMemGetInfo (&free_mem, &total_mem) != cudaSuccess)
{
fprintf (stderr, "cudaMemGetInfo failed.\n");
abort ();
}
struct cudaDeviceProp p;
- if (cudaGetDeviceProperties(&p, dev_num) != cudaSuccess)
+ if (cudaGetDeviceProperties (&p, dev_num) != cudaSuccess)
{
fprintf (stderr, "cudaGetDeviceProperties failed.\n");
abort ();
}
int driver_version;
- if (cudaDriverGetVersion(&driver_version) != cudaSuccess)
+ if (cudaDriverGetVersion (&driver_version) != cudaSuccess)
{
fprintf (stderr, "cudaDriverGetVersion failed.\n");
abort ();
@@ -63,7 +64,7 @@ int main ()
/* 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, vendor, p.name, driver);
+ expect_device_properties (acc_device_nvidia, dev_num,
+ 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 1984ad3a28d..79b67084852 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
@@ -14,7 +14,7 @@
and do basic device independent validation. */
void
-print_device_properties(acc_device_t type)
+print_device_properties (acc_device_t type)
{
const char *s;
size_t v;
@@ -23,7 +23,7 @@ print_device_properties(acc_device_t type)
for (int i = 0; i < dev_count; ++i)
{
- printf(" Device %d:\n", i+1);
+ printf (" Device %d:\n", i+1);
s = acc_get_property_string (i, type, acc_property_vendor);
printf (" Vendor: %s\n", s);
@@ -59,17 +59,17 @@ print_device_properties(acc_device_t type)
int main ()
{
- printf("acc_device_none:\n");
+ printf ("acc_device_none:\n");
/* For completness; not expected to print anything since there
should be no devices of this type. */
- print_device_properties(acc_device_none);
+ print_device_properties (acc_device_none);
- printf("acc_device_default:\n");
- print_device_properties(acc_device_default);
+ printf ("acc_device_default:\n");
+ print_device_properties (acc_device_default);
- printf("acc_device_host:\n");
- print_device_properties(acc_device_host);
+ printf ("acc_device_host:\n");
+ print_device_properties (acc_device_host);
- printf("acc_device_not_host:\n");
- print_device_properties(acc_device_not_host);
+ printf ("acc_device_not_host:\n");
+ print_device_properties (acc_device_not_host);
}
--
2.17.1
