Hi Frederik!

On 2019-12-20T17:46:57+0100, "Harwath, Frederik" <frederik@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'.

> @@ -0,0 +1,68 @@
> +/* Test the `acc_get_property' and '`acc_get_property_string' library
> +   functions on Nvidia devices by comparing property values with
> +   those obtained through the CUDA API. */
> +/* { dg-additional-sources acc_get_property-aux.c } */
> +/* { dg-additional-options "-lcuda -lcudart" } */
> +/* { dg-do run { target openacc_nvidia_accel_selected } } */
> +
> +#include <openacc.h>
> +#include <cuda.h>
> +#include <cuda_runtime_api.h>
> +#include <string.h>
> +#include <stdio.h>
> +
> +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);
> +
> +int main ()
> +{
> +  int dev_count;
> +  cudaGetDeviceCount (&dev_count);
> +
> +  for (int dev_num = 0; dev_num < dev_count; ++dev_num)
> +    {
> +      if (cudaSetDevice (dev_num) != cudaSuccess)
> +	{
> +	  fprintf (stderr, "cudaSetDevice failed.\n");
> +	  abort ();
> +	}
> +
> +      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)
> +	{
> +	  fprintf (stderr, "cudaMemGetInfo failed.\n");
> +	  abort ();
> +	}
> +
> +      struct cudaDeviceProp p;
> +      if (cudaGetDeviceProperties(&p, dev_num) != cudaSuccess)
> +	{
> +	  fprintf (stderr, "cudaGetDeviceProperties failed.\n");
> +	  abort ();
> +	}
> +
> +      int driver_version;
> +      if (cudaDriverGetVersion(&driver_version) != cudaSuccess)
> +	{
> +	  fprintf (stderr, "cudaDriverGetVersion failed.\n");
> +	  abort ();
> +	}
> +      /* The version string should contain the version of the CUDA Toolkit
> +	 in the same MAJOR.MINOR format that is used by Nvidia.
> +	 The format string below is the same that is used by the deviceQuery
> +	 program, which belongs to Nvidia's CUDA samples, to print the version. */
> +      char driver[30];
> +      snprintf (driver, sizeof driver, "CUDA Driver %u.%u",
> +		driver_version / 1000, driver_version % 1000 / 10);
> +
> +      expect_device_properties(acc_device_nvidia, dev_num,

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.

> +			       total_mem, free_mem, vendor, p.name, driver);
> +    }
> +}

So I just witnessed a FAIL here, because:

    Expected acc_property_free_memory to equal -929226752, but was -928956416.

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.  ;-)

> --- /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'.

> @@ -0,0 +1,19 @@
> +/* Test the `acc_get_property' and '`acc_get_property_string' library
> +   functions for the host device. */
> +/* { dg-additional-sources acc_get_property-aux.c } */
> +/* { dg-do run } */
> +
> +#include <openacc.h>
> +#include <stdio.h>
> +
> +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);
> +
> +int main()
> +{
> +  printf ("Checking acc_device_host device properties\n");
> +  expect_device_properties (acc_device_host, 0, 0, 0, "GNU", "GOMP", "1.0");
> +}

> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.c
> @@ -0,0 +1,80 @@
> +/* Auxiliary functions for acc_get_property tests */
> +/* { dg-do compile  { target skip-all-targets } } */
> +
> +#include <openacc.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +
> +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)
> +{
> +  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 ();
> +    }
> +
> +  int total_mem = acc_get_property (dev_num, dev_type,
> +				    acc_property_memory);
> +  if (total_mem != expected_total_mem)
> +    {
> +      fprintf (stderr, "Expected acc_property_memory to equal %d, "
> +	       "but was %d.\n", expected_total_mem, total_mem);
> +      abort ();
> +
> +    }
> +
> +  int free_mem = acc_get_property (dev_num, dev_type,
> +				   acc_property_free_memory);
> +  if (free_mem != expected_free_mem)
> +    {
> +      fprintf (stderr, "Expected acc_property_free_memory to equal %d, "
> +	       "but was %d.\n", expected_free_mem, free_mem);
> +      abort ();
> +    }

Better to just verify that 'free_mem >= 0' (by means of 'size_t' data
type, I suppose), and 'free_mem <= total_mem'?

(..., 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.)

> +
> +  const char *name = acc_get_property_string (dev_num, dev_type,
> +					      acc_property_name);
> +  if (strcmp (name, expected_name))
> +    {
> +      fprintf(stderr, "Expected acc_property_name to equal \"%s\", "
> +	      "but was \"%s\".\n", expected_name, name);
> +      abort ();
> +    }
> +
> +  const char *driver = acc_get_property_string (dev_num, dev_type,
> +						acc_property_driver);
> +  if (strcmp (expected_driver, driver))
> +    {
> +      fprintf (stderr, "Expected acc_property_driver to equal %s, "
> +	       "but was %s.\n", expected_driver, driver);
> +      abort ();
> +    }
> +
> +  int unknown_property = 16058;
> +  int 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);
> +      abort ();
> +    }
> +
> +  int unknown_property2 = -16058;
> +  const char *s = acc_get_property_string (dev_num, dev_type, (acc_device_property_t)unknown_property2);
> +  if (s != NULL)
> +    {
> +      fprintf (stderr, "Expected value of unknown string property to be NULL, "
> +	       "but was %d.\n", s);
> +      abort ();
> +    }
> +
> +
> +}


Grüße
 Thomas