subject:"Add OpenACC 2.6 `acc_get

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

2020-04-29 Thread Thomas Schwinge

Hi!

On 2019-12-17T00:00:04+0100, I wrote:
> On 2019-11-14T16:35:31+0100, Frederik Harwath  
> wrote:
>> this patch implements OpenACC 2.6 "acc_get_property" and related functions.

> As I mentioned before ("thinking aloud"):
>
> | [...] 'acc_device_current' [is] relevant only for
> | 'acc_get_property', to return "the value of the property for the current
> | device".  This [now has a] special (negative?) value
> | [...], so that when additional real device types are added
> | later on, we can just add them with increasing numbers, and keep the
> | scanning code simple.

> Should this actually get value '-1' instead of '-3'?  Or, is the OpenACC
> 'acc_device_t' code already paying special attention to negative values
> '-1', '-2'?  (I don't think so.)

Now pushed this change to master branch in commit
a5d0bc12e1bfa956941cd9c49d5b978256bd11ec "[OpenACC] Set
'acc_device_current = -1'", see attached.


Grüße
 Thomas


-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter
>From a5d0bc12e1bfa956941cd9c49d5b978256bd11ec Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Wed, 29 Apr 2020 08:12:36 +0200
Subject: [PATCH] [OpenACC] Set 'acc_device_current = -1'

There's no point in using value '-3', and even though not directly related,
value '-1' does match 'GOMP_DEVICE_ICV'.

	libgomp/
	* config/accel/openacc.f90 (acc_device_current): Set to '-1'.
	* openacc.f90 (acc_device_current): Likewise.
	* openacc.h (acc_device_current): Likewise.
	* openacc_lib.h (acc_device_current): Likewise.
---
 libgomp/ChangeLog| 5 +
 libgomp/config/accel/openacc.f90 | 2 +-
 libgomp/openacc.f90  | 2 +-
 libgomp/openacc.h| 2 +-
 libgomp/openacc_lib.h| 2 +-
 5 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/libgomp/ChangeLog b/libgomp/ChangeLog
index 1a7046f2fc64..b6828adcbe3d 100644
--- a/libgomp/ChangeLog
+++ b/libgomp/ChangeLog
@@ -1,5 +1,10 @@
 2020-04-29  Thomas Schwinge  
 
+	* config/accel/openacc.f90 (acc_device_current): Set to '-1'.
+	* openacc.f90 (acc_device_current): Likewise.
+	* openacc.h (acc_device_current): Likewise.
+	* openacc_lib.h (acc_device_current): Likewise.
+
 	PR target/94282
 	* testsuite/libgomp.c-c++-common/function-not-offloaded.c: Remove
 	'dg-allow-blank-lines-in-output'.
diff --git a/libgomp/config/accel/openacc.f90 b/libgomp/config/accel/openacc.f90
index 275afe43475c..99330733d8f8 100644
--- a/libgomp/config/accel/openacc.f90
+++ b/libgomp/config/accel/openacc.f90
@@ -44,7 +44,7 @@ module openacc_kinds
   integer, parameter :: acc_device_kind = int32
 
   ! Keep in sync with include/gomp-constants.h.
-  integer (acc_device_kind), parameter :: acc_device_current = -3
+  integer (acc_device_kind), parameter :: acc_device_current = -1
   integer (acc_device_kind), parameter :: acc_device_none = 0
   integer (acc_device_kind), parameter :: acc_device_default = 1
   integer (acc_device_kind), parameter :: acc_device_host = 2
diff --git a/libgomp/openacc.f90 b/libgomp/openacc.f90
index 467fb612c548..111705d0fb60 100644
--- a/libgomp/openacc.f90
+++ b/libgomp/openacc.f90
@@ -41,7 +41,7 @@ module openacc_kinds
   integer, parameter :: acc_device_kind = int32
 
   ! Keep in sync with include/gomp-constants.h.
-  integer (acc_device_kind), parameter :: acc_device_current = -3
+  integer (acc_device_kind), parameter :: acc_device_current = -1
   integer (acc_device_kind), parameter :: acc_device_none = 0
   integer (acc_device_kind), parameter :: acc_device_default = 1
   integer (acc_device_kind), parameter :: acc_device_host = 2
diff --git a/libgomp/openacc.h b/libgomp/openacc.h
index 617364634748..1dc471f62bc7 100644
--- a/libgomp/openacc.h
+++ b/libgomp/openacc.h
@@ -49,7 +49,7 @@ extern "C" {
 /* Types */
 typedef enum acc_device_t {
   /* Keep in sync with include/gomp-constants.h.  */
-  acc_device_current = -3,
+  acc_device_current = -1,
   acc_device_none = 0,
   acc_device_default = 1,
   acc_device_host = 2,
diff --git a/libgomp/openacc_lib.h b/libgomp/openacc_lib.h
index ee08e9787cc9..82a3735b1063 100644
--- a/libgomp/openacc_lib.h
+++ b/libgomp/openacc_lib.h
@@ -37,7 +37,7 @@
   integer, parameter :: acc_device_kind = 4
 
 ! Keep in sync with include/gomp-constants.h.
-  integer (acc_device_kind), parameter :: acc_device_current = -3
+  integer (acc_device_kind), parameter :: acc_device_current = -1
   integer (acc_device_kind), parameter :: acc_device_none = 0
   integer (acc_device_kind), parameter :: acc_device_default = 1
   integer (acc_device_kind), parameter :: acc_device_host = 2
-- 
2.26.2

Re: Make OpenACC 'acc_get_property' with 'acc_device_current' work (was: [PATCH] Add OpenACC 2.6 `acc_get_property' support)

2020-02-03 Thread Harwath, Frederik

Hi Thomas,

On 30.01.20 16:54, Thomas Schwinge wrote:
> 
> [...] the 'acc_device_current' interface should work already now.
> 
> [...] Please review
> the attached (Tobias the Fortran test cases, please), and test with AMD
> GCN offloading.  If approving this patch, please respond with

I have tested the patch with AMD GCN offloading and I have observed no 
regressions.
The new tests pass as expected and print the correct output.
Great that you have extended the Fortran tests!

> diff --git a/libgomp/oacc-init.c b/libgomp/oacc-init.c
> index ef12b4c16d01..c28c0f689ba2 100644
> --- a/libgomp/oacc-init.c
> +++ b/libgomp/oacc-init.c
> @@ -796,7 +796,9 @@ get_property_any (int ord, acc_device_t d, 
> acc_device_property_t prop)
> size_t
> acc_get_property (int ord, acc_device_t d, acc_device_property_t prop)
> {
> -  if (!known_device_type_p (d))
> +  if (d == acc_device_current)
> +; /* Allowed only for 'acc_get_property', 'acc_get_property_string'.  */
> +  else if (!known_device_type_p (d))
> unknown_device_type_error(d);

I don't like the empty if branch very much. Introducing a variable
(for instance, "bool allowed_device_type = acc_device_current
|| known_device_type(d);") would also provide a place for your comment.
You could also extract a function to avoid duplicating the explanation
in acc_get_property_string.

The patch looks good to me.

Reviewed-by: Frederik Harwath  

Best regards,
Frederik

Make OpenACC 'acc_get_property' with 'acc_device_current' work (was: [PATCH] Add OpenACC 2.6 `acc_get_property' support)

2020-01-30 Thread Thomas Schwinge

Hi!

On 2020-01-10T23:52:11+0100, I wrote:
> On 2019-12-21T23:02:38+0100, I wrote:
>> On 2019-12-20T17:46:57+0100, "Harwath, Frederik"  
>> wrote:
 > --- a/include/gomp-constants.h
 > +++ b/include/gomp-constants.h
>
 > +#define GOMP_DEVICE_CURRENT -3
>
 Should this actually get value '-1' instead of '-3'?  Or, is the OpenACC
 'acc_device_t' code already paying special attention to negative values
 '-1', '-2'?  (I don't think so.)
>
 | Also, 'acc_device_current' is a libgomp-internal thing (doesn't interface
 | with the compiler proper), so strictly speaking 'GOMP_DEVICE_CURRENT'
 | isn't needed in 'include/gomp-constants.h'.  But probably still a good
 | idea to list it there, in this canonical place, to keep the several lists
 | of device types coherent.
>
 I still wonder about that...  ;-)
>
>> I still think that 'GOMP_DEVICE_CURENT' should get value '-1' (and
>> probably be rename 'GOACC_DEVICE_CURRENT' to make more obvious that it's
>> not related to the 'GOMP_DEVICE_*' ones), but we shall have a look at
>> that later (before GCC 10 release); that's libgomp/OpenACC-internal,
>> doesn't affect anything else.
>
> That's still pending.  Recently,
>  "Missing definition
> for acc_device_current" got filed; let's (also/first) watch/wait what
> comes out of that.

(That's still pending, but) notwithstanding the specific value we'll use
eventually, the 'acc_device_current' interface should work already now.

..., but I noticed that we don't have any test cases for that (so by that
definition, it must be broken).  The curious guy that I am sometimes ;-)
I gave that a try, and... "of course"... it doesn't work.  Please review
the attached (Tobias the Fortran test cases, please), and test with AMD
GCN offloading.  If approving this patch, please respond with
"Reviewed-by: NAME " so that your effort will be recorded in the
commit log, see .


Grüße
 Thomas


From 5ce3725cf160f086e99c01e73c26a0bf5654f5b6 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Wed, 29 Jan 2020 22:11:15 +0100
Subject: [PATCH] Make OpenACC 'acc_get_property' with 'acc_device_current'
 work

	libgomp/
	* oacc-init.c (acc_get_property, acc_get_property_string): Allow
	'acc_device_current'.
	* openacc.f90 (module openacc): Export 'acc_device_current'.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.c
	(expect_device_memory): Rename to...
	(expect_device_memory_properties): ... this.  Make 'static'.
	(expect_device_string_properties): Rename to...
	(expect_device_non_memory_properties): ... this.  Adjust all
	users.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.h: New
	file.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.c: Use it.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-gcn.c:
	Likewise.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-host.c:
	Likewise.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-nvptx.c:
	Likewise.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-gcn.c: Add
	some more testing.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-host.c:
	Likewise.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-nvptx.c:
	Likewise.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property.c:
	Likewise.
	* testsuite/libgomp.oacc-fortran/acc_get_property.f90: Likewise.
	* testsuite/libgomp.oacc-fortran/acc_get_property-aux.f90: New
	file.
	* testsuite/libgomp.oacc-fortran/acc_get_property-host.F90: New
	file.
---
 libgomp/oacc-init.c   |   8 +-
 libgomp/openacc.f90   |   1 +
 .../acc_get_property-aux.c|  76 ++---
 .../acc_get_property-aux.h|  14 +++
 .../acc_get_property-gcn.c|  26 +++--
 .../acc_get_property-host.c   |  26 +++--
 .../acc_get_property-nvptx.c  |  27 +++--
 .../acc_get_property.c|  16 ++-
 .../acc_get_property-aux.f90  | 102 ++
 .../acc_get_property-host.F90 |  31 ++
 .../libgomp.oacc-fortran/acc_get_property.f90 |  15 ++-
 11 files changed, 274 insertions(+), 68 deletions(-)
 create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.h
 create mode 100644 libgomp/testsuite/libgomp.oacc-fortran/acc_get_property-aux.f90
 create mode 100644 libgomp/testsuite/libgomp.oacc-fortran/acc_get_property-host.F90

diff --git a/libgomp/oacc-init.c b/libgomp/oacc-init.c
index ef12b4c16d01..c28c0f689ba2 100644
--- a/libgomp/oacc-init.c
+++ b/libgomp/oacc-init.c
@@ -796,7 +796,9 @@ get_property_any (int ord, acc_device_t d, acc_device_property_t prop)
 size_t
 acc_get_property (int ord, acc_device_t d, acc_device_property_t prop)
 {
-  if (!known_device_type_p (d))
+  if (d == acc_device_current)
+; /* Allowed only for 'acc_get_property',

Fortran 'acc_get_property' return type (was: [PATCH] Add OpenACC 2.6 `acc_get_property' support)

2020-01-27 Thread Thomas Schwinge

Hi!

On 2019-12-20T17:46:57+0100, "Harwath, Frederik"  
wrote:
>> > --- a/libgomp/libgomp-plugin.h
>> > +++ b/libgomp/libgomp-plugin.h
>> > @@ -54,6 +54,13 @@ enum offload_target_type
>> >OFFLOAD_TARGET_TYPE_GCN =3D 8
>> >  };
>> >=20=20
>> > +/* Container type for passing device properties.  */
>> > +union gomp_device_property_value
>> > +{
>> > +  void *ptr;
>> > +  uintmax_t val;
>> > +};
>>
>> Why wouldn't that be 'size_t', 'const char *', as the actual data types
>> used?  (Maybe I'm missing something.)
>
> I do not see a reason for this either. Changed.

For reference: C/C++ has 'size_t' ('acc_get_property'), or 'const char*'
('acc_get_property_string') return types.

>> > --- a/libgomp/openacc.f90
>> > +++ b/libgomp/openacc.f90
>> > @@ -28,7 +28,7 @@
>> >  !  .
>> >=20=20
>> >  module openacc_kinds
>> > -  use iso_fortran_env, only: int32
>> > +  use iso_fortran_env, only: int32, int64
>> >implicit none
>> >=20=20
>> >private :: int32
>> > @@ -47,6 +47,21 @@ module openacc_kinds
>> >integer (acc_device_kind), parameter :: acc_device_not_host =3D 4
>> >integer (acc_device_kind), parameter :: acc_device_nvidia =3D 5
>> >integer (acc_device_kind), parameter :: acc_device_gcn =3D 8
>> > +  integer (acc_device_kind), parameter :: acc_device_current =3D -3
>> > +
>> > +  public :: acc_device_property
>> > +
>> > +  integer, parameter :: acc_device_property =3D int64
>>
>> Why 'int64'?  I changed this to 'int32', but please tell if there's a
>> reason for 'int64'.
>
> int32 is too narrow as - conforming to the OpenACC spec - acc_device_property
> is also used for the return type of acc_get_property (a bit strang, isn't 
> it?).
> int64 also did not seem quite right. I have changed the type of 
> acc_device_property
> to c_size_t to match the type that is used internally and as the return type 
> of the
> corresponding C function.

I filed  "Fortran
'acc_get_property' return type":

| During review/implementation of `acc_get_property` in GCC, @frederik-h
| found that for Fortran `function acc_get_property`, the return type is
| specified as `integer(acc_device_property) :: acc_get_property`,
| whereas in C/C++, it is `size_t`.  For avoidance of doubt: it's correct
| to map the C/C++ `acc_device_property_t property` formal parameter to
| Fortran `integer(acc_device_property), value :: property`, but it's not
| clear why `integer(acc_device_property)` is also used as the function's
| return type -- the return type/values don't actually (conceptually)
| relate to the `integer(acc_device_property)` data type.
| 
| Should we use `c_size_t` for Fortran `acc_get_property` return type to
| explicitly match C/C++, or use plain `integer` (as used in all other
| interfaces taking `size_t` in C/C++ -- currently only as input formal
| parameters though)?


Grüße
 Thomas


> --- a/libgomp/libgomp.texi
> +++ b/libgomp/libgomp.texi

> +@node acc_get_property
> +@section @code{acc_get_property} -- Get device property.
> +@cindex acc_get_property
> +@cindex acc_get_property_string
> +@table @asis
> +@item @emph{Description}
> +These routines return the value of the specified @var{property} for the
> +device being queried according to @var{devicenum} and @var{devicetype}.
> +Integer-valued and string-valued properties are returned by
> +@code{acc_get_property} and @code{acc_get_property_string} respectively.
> +The Fortran @code{acc_get_property_string} subroutine returns the string
> +retrieved in its fourth argument while the remaining entry points are
> +functions, which pass the return value as their result.
> +
> +@item @emph{C/C++}:
> +@multitable @columnfractions .20 .80
> +@item @emph{Prototype}: @tab @code{size_t acc_get_property(int devicenum, 
> acc_device_t devicetype, acc_device_property_t property);}
> +@item @emph{Prototype}: @tab @code{const char *acc_get_property_string(int 
> devicenum, acc_device_t devicetype, acc_device_property_t property);}
> +@end multitable
> +
> +@item @emph{Fortran}:
> +@multitable @columnfractions .20 .80
> +@item @emph{Interface}: @tab @code{function acc_get_property(devicenum, 
> devicetype, property)}
> +@item @emph{Interface}: @tab @code{subroutine 
> acc_get_property_string(devicenum, devicetype, property, string)}
> +@item   @tab @code{integer devicenum}
> +@item   @tab @code{integer(kind=acc_device_kind) devicetype}
> +@item   @tab @code{integer(kind=acc_device_property) 
> property}
> +@item   @tab @code{integer(kind=acc_device_property) 
> acc_get_property}
> +@item   @tab @code{character(*) string}
> +@end multitable
> +
> +@item @emph{Reference}:
> +@uref{https://www.openacc.org, OpenACC specification v2.6}, section
> +3.2.6.
> +@end table


> --- a/libgomp/openacc.f90
> +++ b/libgomp/openacc.f90
> @@ -31,16 +31,18 @@
>  
>  module openacc_kinds
>use iso_fortran_env, only: int32
> +

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

2020-01-24 Thread Harwath, Frederik

Hi Thomas,

On 23.01.20 15:32, Thomas Schwinge wrote:

> On 2020-01-20T15:01:01+0100, "Harwath, Frederik"  
> wrote:
>> On 16.01.20 17:00, Thomas Schwinge wrote:
>>> On 2019-12-20T17:46:57+0100, "Harwath, Frederik" 
>>>  wrote:
>> Ok to push the commit to master?
> 
> Thanks, OK.  Reviewed-by: Thomas Schwinge 

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

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

2020-01-23 Thread Thomas Schwinge

Hi Frederik!

On 2020-01-20T15:01:01+0100, "Harwath, Frederik"  
wrote:
> 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"  
>> wrote:
> Ok to push the commit to master?

Thanks, OK.  Reviewed-by: Thomas Schwinge 


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


signature.asc
Description: PGP signature

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

2020-01-20 Thread Harwath, Frederik

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

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

2020-01-16 Thread Thomas Schwinge

Hi Frederik!

On 2019-12-20T17:46:57+0100, "Harwath, Frederik"  
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 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +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 (_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(_mem, _mem) != cudaSuccess)
> + {
> +   fprintf (stderr, "cudaMemGetInfo failed.\n");
> +   abort ();
> + }
> +
> +  struct cudaDeviceProp p;
> +  if (cudaGetDeviceProperties(, dev_num) != cudaSuccess)
> + {
> +   fprintf (stderr, "cudaGetDeviceProperties failed.\n");
> +   abort ();
> + }
> +
> +  int driver_version;
> +  if (cudaDriverGetVersion(_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 
> +#include 
> +
> +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 
> +#include 
> +#include 
> +#include 
> +
> +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 ();
>

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

2020-01-10 Thread Thomas Schwinge

Hi!

On 2019-12-21T23:02:38+0100, I wrote:
> On 2019-12-20T17:46:57+0100, "Harwath, Frederik"  
> wrote:
>>> > --- a/include/gomp-constants.h
>>> > +++ b/include/gomp-constants.h

>>> > +#define GOMP_DEVICE_CURRENT  -3

>>> Should this actually get value '-1' instead of '-3'?  Or, is the OpenACC
>>> 'acc_device_t' code already paying special attention to negative values
>>> '-1', '-2'?  (I don't think so.)

>>> | Also, 'acc_device_current' is a libgomp-internal thing (doesn't interface
>>> | with the compiler proper), so strictly speaking 'GOMP_DEVICE_CURRENT'
>>> | isn't needed in 'include/gomp-constants.h'.  But probably still a good
>>> | idea to list it there, in this canonical place, to keep the several lists
>>> | of device types coherent.

>>> I still wonder about that...  ;-)

> I still think that 'GOMP_DEVICE_CURENT' should get value '-1' (and
> probably be rename 'GOACC_DEVICE_CURRENT' to make more obvious that it's
> not related to the 'GOMP_DEVICE_*' ones), but we shall have a look at
> that later (before GCC 10 release); that's libgomp/OpenACC-internal,
> doesn't affect anything else.

That's still pending.  Recently,
 "Missing definition
for acc_device_current" got filed; let's (also/first) watch/wait what
comes out of that.


Also pending is the 'libgomp/plugin/plugin-gcn.c' update; Frederik is
working on that.


A few other (minor) open issue we shall discuss at some later point in
time.


>>> | Before Frederik starts working on integrating this into GCC trunk, do you
>>> | (Jakub) agree with the libgomp plugin interface changes as implemented by
>>> | Maciej?  For example, top-level 'GOMP_OFFLOAD_get_property' function in
>>> | 'struct gomp_device_descr' instead of stuffing this into its
>>> | 'acc_dispatch_t openacc'.  (I never understood why the OpenACC functions
>>> | need to be segregated like they are.)
>>>
>>> Jakub didn't answer, but I now myself decided that we should group this
>>> with the other OpenACC libgomp-plugin functions, as this interface is
>>> defined in terms of OpenACC-specific stuff such as 'acc_device_t'.

Done.  This also means that we don't anymore need (stub) implementations
in the libgomp plugins that don't implement OpenACC support.


>>> Maybe [stuff] should move from 'include/gomp-constants.h' to
>>> 'libgomp/oacc-int.h'.  I'll think about that again, when I'm awake again
>>> tomorrow.  ;-)
>>
>> Have you made up your mind yet? :-)
>
> Still sleepy.  ;-)

Done.  In the end, 'libgomp/libgomp-plugin.h' is the place to be, as that
defines, well, the libgomp plugin interface.  ;-)


See attached "OpenACC 'acc_get_property' cleanup"; committed to trunk in
r280150.


Grüße
 Thomas


From f98381a87233ddef09bf7d3a0d47807215930063 Mon Sep 17 00:00:00 2001
From: tschwinge 
Date: Fri, 10 Jan 2020 22:24:36 +
Subject: [PATCH] OpenACC 'acc_get_property' cleanup

	include/
	* gomp-constants.h (enum gomp_device_property): Remove.
	libgomp/
	* libgomp-plugin.h (enum goacc_property): New.  Adjust all users
	to use this instead of 'enum gomp_device_property'.
	(GOMP_OFFLOAD_get_property): Rename to...
	(GOMP_OFFLOAD_openacc_get_property): ... this.  Adjust all users.
	* libgomp.h (struct gomp_device_descr): Move
	'GOMP_OFFLOAD_openacc_get_property'...
	(struct acc_dispatch_t): ... here.  Adjust all users.
	* plugin/plugin-hsa.c (GOMP_OFFLOAD_get_property): Remove.
	liboffloadmic/
	* plugin/libgomp-plugin-intelmic.cpp (GOMP_OFFLOAD_get_property):
	Remove.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@280150 138bc75d-0d04-0410-961f-82ee72b054a4
---
 include/ChangeLog |   4 +
 include/gomp-constants.h  |  15 --
 libgomp/ChangeLog |   9 ++
 libgomp/libgomp-plugin.h  |  36 -
 libgomp/libgomp.h |   3 +-
 libgomp/oacc-host.c   |  47 +++---
 libgomp/oacc-init.c   |  10 +-
 libgomp/openacc.f90   |   2 +-
 libgomp/openacc.h |   3 +-
 libgomp/plugin/plugin-gcn.c   |  22 +--
 libgomp/plugin/plugin-hsa.c   |  26 
 libgomp/plugin/plugin-nvptx.c | 138 +-
 libgomp/target.c  |   4 +-
 liboffloadmic/ChangeLog   |   5 +
 .../plugin/libgomp-plugin-intelmic.cpp|  21 ---
 15 files changed, 163 insertions(+), 182 deletions(-)

diff --git a/include/ChangeLog b/include/ChangeLog
index 0069df3c5c6..8a2feb911cb 100644
--- a/include/ChangeLog
+++ b/include/ChangeLog
@@ -1,3 +1,7 @@
+2020-01-10  Thomas Schwinge  
+
+	* gomp-constants.h (enum gomp_device_property): Remove.
+
 2020-01-01  Jakub Jelinek  
 
 	Update copyright years.
diff --git a/include/gomp-constants.h b/include/gomp-constants.h
index 5a7cc2c3f01..1587e4d2ba2 100644
--- a/include/gomp-constants.h
+++

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

2019-12-22 Thread Harwath, Frederik

Hi Thomas,

>> Is it ok to commit the patch to trunk?
> 
> OK, thanks.  And then some follow-up/clean-up next year, also including
> some of the open questions that I've snipped off here.

Right, thanks for the review! I have committed the patch as r279710 with a
minor change: I have disabled the new acc_get_property.{c,f90} tests for
the amdgcn offload target for now.

Best regards,
Frederik

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

2019-12-21 Thread Thomas Schwinge

Hi Frederik!

On 2019-12-20T17:46:57+0100, "Harwath, Frederik"  
wrote:
>> | Before Frederik starts working on integrating this into GCC trunk, do you
>> | (Jakub) agree with the libgomp plugin interface changes as implemented by
>> | Maciej?  For example, top-level 'GOMP_OFFLOAD_get_property' function in
>> | 'struct gomp_device_descr' instead of stuffing this into its
>> | 'acc_dispatch_t openacc'.  (I never understood why the OpenACC functions
>> | need to be segregated like they are.)
>>
>> Jakub didn't answer, but I now myself decided that we should group this
>> with the other OpenACC libgomp-plugin functions, as this interface is
>> defined in terms of OpenACC-specific stuff such as 'acc_device_t'.
>> Frederik, please work on that, also try to move function definitions etc.
>> into appropriate places in case they aren't; ask if you need help.
>> That needs to be updated.
>
> Is it ok to do this in a separate follow-up patch?

Yes.  This doesn't affect anything but the libgomp-plugin interface.


>> > --- a/include/gomp-constants.h
>> > +++ b/include/gomp-constants.h
>> > @@ -178,6 +178,20 @@ enum gomp_map_kind
>> >=20=20
>> >  #define GOMP_DEVICE_ICV   -1
>> >  #define GOMP_DEVICE_HOST_FALLBACK -2
>> > +#define GOMP_DEVICE_CURRENT   -3
>> [...]
>>
>> Not should if this should be grouped with 'GOMP_DEVICE_ICV',
>> 'GOMP_DEVICE_HOST_FALLBACK', for it is not related to there.
>>
>> [...]
>>
>> Should this actually get value '-1' instead of '-3'?  Or, is the OpenACC
>> 'acc_device_t' code already paying special attention to negative values
>> '-1', '-2'?  (I don't think so.)
>> | Also, 'acc_device_current' is a libgomp-internal thing (doesn't interface
>> | with the compiler proper), so strictly speaking 'GOMP_DEVICE_CURRENT'
>> | isn't needed in 'include/gomp-constants.h'.  But probably still a good
>> | idea to list it there, in this canonical place, to keep the several lists
>> | of device types coherent.
>> still wonder about that...  ;-)

> Changing the value of GOMP_DEVICE_ICV violates the following static asserts 
> in oacc-parallel.c: [...]

Hmm, I didn't intend to suggest changing the 'GOMP_DEVICE_ICV' value,
which indeed can't/shouldn't be done.

I still think that 'GOMP_DEVICE_CURENT' should get value '-1' (and
probably be rename 'GOACC_DEVICE_CURRENT' to make more obvious that it's
not related to the 'GOMP_DEVICE_*' ones), but we shall have a look at
that later (before GCC 10 release); that's libgomp/OpenACC-internal,
doesn't affect anything else.

>> Maybe this stuff should move from 'include/gomp-constants.h' to
>> 'libgomp/oacc-int.h'.  I'll think about that again, when I'm awake again
>> tomorrow.  ;-)
>
> Have you made up your mind yet? :-)

Still sleepy.  ;-)


> Is it ok to commit the patch to trunk?

OK, thanks.  And then some follow-up/clean-up next year, also including
some of the open questions that I've snipped off here.


Grüße
 Thomas


signature.asc
Description: PGP signature

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

2019-12-20 Thread Harwath, Frederik

penacc_lib.h' has been deprecated ("no longer
> supported"), but so far, we continued to support it, and it's (maybe?)
> strange when that one now works for everything but the 'acc_get_property'
> interface?  Or, is that a statement that users really should move to the
> Fortran 'openacc' module?

Should they? You are probably best qualified to answer this :-).

> > +typedef enum acc_device_property_t {
> > +  /* Keep in sync with include/gomp-constants.h.  */
> > +  /* Start from 1 to catch uninitialized use.  */
> > +  acc_property_memory =3D 1,
> > +  acc_property_free_memory =3D 2,
> > +  acc_property_name =3D 0x10001,
> > +  acc_property_vendor =3D 0x10002,
> > +  acc_property_driver =3D 0x10003
> > +} acc_device_property_t;
>
> Do we also need the magic here so that "Ensure enumeration is layout
> compatible with int"?  But I see that is not done for the 'typedef enum
> acc_async_t' either.  I don't remember the history behind that.

I understand the "Ensure enumeration is layout compatible with int" comment for
acc_device_t, but I fail to see how those values achieve this.
If you do not see a good reason to keep those magic values, I would change them
to 3, 4, 5 and if we need the "layout compatibility", I would rather do this as
it is done for acc_device_t.


> > --- a/libgomp/plugin/plugin-hsa.c
> > +++ b/libgomp/plugin/plugin-hsa.c
> > @@ -699,6 +699,32 @@ GOMP_OFFLOAD_get_num_devices (void)
> >return hsa_context.agent_count;
> >  }
> >=20=20
> > +/* Part of the libgomp plugin interface.  Return the value of property
> > +   PROP of agent number N.  */
> > +
> > +union gomp_device_property_value
> > +GOMP_OFFLOAD_get_property (int n, int prop)
> > +{
> > +  union gomp_device_property_value nullval =3D { .val =3D 0 };
> > +
> > +  if (!init_hsa_context ())
> > +return nullval;
>
> I'm not familiar with that code, but similar to other plugins,
> 'init_hsa_context' already is called via 'GOMP_OFFLOAD_get_num_devices'
> (and 'GOMP_OFFLOAD_init_device', hmm...), so probably don't need to call
> it here?

Since Martin Jambor wrote that the call does no harm, I would just keep it.

> > +
> > +  switch (prop)
> > +{
> > +case GOMP_DEVICE_PROPERTY_VENDOR:
> > +  return (union gomp_device_property_value) { .ptr =3D "AMD" };
> > +default:
> > +  return nullval;
> > +}
> > +}
>
> Not sure if "AMD" is actually correct here -- isn't HSA a
> vendor-independent standard?

I have changed "AMD" to "HSA".

> > +union gomp_device_property_value
> > +GOMP_OFFLOAD_get_property (int n, int prop)
> > +{
> > +  union gomp_device_property_value propval =3D { .val =3D 0 };
> > +
> > +  pthread_mutex_lock (_dev_lock);
>
> Everything (?) else seems to be accessing 'ptx_devices' without locking?
> (I don't quickly understand the locking protocol used there...  Will look
> again tomorrow.)

GOMP_OFFLOAD_init_device, GOMP_OFFLOAD_fini_device take the lock
before accessing ptx_devices. I kept it.

> > +   CUDA_CALL_ERET (propval, cuCtxGetDevice, );
> > +   if (ptx_dev->dev =3D=3D ctxdev)
> > + CUDA_CALL_ERET (propval, cuMemGetInfo, _mem, _mem);
> > +   else if (ptx_dev->ctx)
> > + {
> > +   CUcontext old_ctx;
> > +
> > +   CUDA_CALL_ERET (propval, cuCtxPushCurrent, ptx_dev->ctx);
> > +   CUDA_CALL_ERET (propval, cuMemGetInfo, _mem, _mem);
> > +   CUDA_CALL_ASSERT (cuCtxPopCurrent, _ctx);
> > + }
> > +   else
> > + {
> > +   CUcontext new_ctx;
> > +
> > +   CUDA_CALL_ERET (propval, cuCtxCreate, _ctx, CU_CTX_SCHED_AUTO,
> > +   ptx_dev->dev);
> > +   CUDA_CALL_ERET (propval, cuMemGetInfo, _mem, _mem);
> > +   CUDA_CALL_ASSERT (cuCtxDestroy, new_ctx);
> > + }

>
>
> (Have not yet reviewed that CUDA magic.  Do you understand that?)

Yes, sort of. If the current thread's CUDA context is the a context for the 
device, perform the query (cuMemGetInfo) right away.
Otherwise, we have to set the context first to ensure that we query the right 
device. Note that this is not necessary
for the functions that are used to retrieve the values of the other properties 
because they use CUDA functions that take a
device argument. If the ptx_dev already contains a context for the device, set 
it temporarily for the duration of the query.
Otherwise, do the same with a newly created context for the device and dispose 
of the new context afterwards.
The last case might arise only if the device has not been initialized, as 
GOMP_OFFLOAD_init_device
c

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

2019-12-17 Thread Andrew Stubbs


On 16/12/2019 23:00, Thomas Schwinge wrote:

There is no AMD GCN support yet. This will be added later on.


ACK, just to note that there now is a 'libgomp/plugin/plugin-gcn.c' that
at least needs to get a stub implementation (can mostly copy from
'libgomp/plugin/plugin-hsa.c'?) as otherwise the build will fail.


The code exists on the OG9 branch. It was omitted from the trunk 
submission because the other half of the properties support wasn't there 
yet.


Andrew

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

2019-12-17 Thread Martin Jambor

Hi,

On Tue, Dec 17 2019, Thomas Schwinge wrote:
> On 2019-11-14T16:35:31+0100, Frederik Harwath  
> wrote:
>> this patch implements OpenACC 2.6 "acc_get_property" and related functions.
>
> [...]
>
>> --- a/libgomp/plugin/plugin-hsa.c
>> +++ b/libgomp/plugin/plugin-hsa.c
>> @@ -699,6 +699,32 @@ GOMP_OFFLOAD_get_num_devices (void)
>>return hsa_context.agent_count;
>>  }
>>  
>> +/* Part of the libgomp plugin interface.  Return the value of property
>> +   PROP of agent number N.  */
>> +
>> +union gomp_device_property_value
>> +GOMP_OFFLOAD_get_property (int n, int prop)
>> +{
>> +  union gomp_device_property_value nullval = { .val = 0 };
>> +
>> +  if (!init_hsa_context ())
>> +return nullval;
>
> I'm not familiar with that code, but similar to other plugins,
> 'init_hsa_context' already is called via 'GOMP_OFFLOAD_get_num_devices'
> (and 'GOMP_OFFLOAD_init_device', hmm...), so probably don't need to call
> it here?

I assume you always want to get the number of devices before querying
their properties but OTOH there is no harm in calling the initialization
function.

>> +
>> +  switch (prop)
>> +{
>> +case GOMP_DEVICE_PROPERTY_VENDOR:
>> +  return (union gomp_device_property_value) { .ptr = "AMD" };
>> +default:
>> +  return nullval;
>> +}
>> +}
>
> Not sure if "AMD" is actually correct here -- isn't HSA a
> vendor-independent standard?
>

Yes, it is supposed to be.  I think HSA is the correct "vendor" too,
essentially an abbreviation for HSA Foundation.

Thanks,

Martin

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

2019-12-16 Thread Thomas Schwinge

erfaces.)

I see 'libgomp/oacc-host.c:host_get_property',
'libgomp/plugin/plugin-hsa.c:GOMP_OFFLOAD_get_property',
'liboffloadmic/plugin/libgomp-plugin-intelmic.cpp:GOMP_OFFLOAD_get_property'
do have a 'default: return nullval'; that's probably what we need to do
here, too?

> +
> +  pthread_mutex_unlock (_dev_lock);
> +  return propval;
> +}


> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc-get-property-2.c

FAIL: libgomp.oacc-c++/../libgomp.oacc-c-c++-common/acc-get-property-2.c 
-DACC_DEVICE_TYPE_nvidia=1 -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O0  (test 
for excess errors)
UNRESOLVED: 
libgomp.oacc-c++/../libgomp.oacc-c-c++-common/acc-get-property-2.c 
-DACC_DEVICE_TYPE_nvidia=1 -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O0  
compilation failed to produce executable

[...]/libgomp.oacc-c-c++-common/acc-get-property-2.c:61:28: error: invalid 
conversion from 'void*' to 'char*' [-fpermissive]
   61 |   char *driver = malloc(sizeof(char) * 40);
  |  ~~^~~
  |    |
          |        void*

> +  char *driver = malloc(sizeof(char) * 40);
> +  snprintf (driver, 40, "CUDA Driver %u.%u", driver_version / 1000,
> + driver_version % 1000 / 10);

Let's just use 'char driver[30]', and 'sizeof driver'?


> --- a/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
> +++ b/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
> @@ -174,6 +174,28 @@ GOMP_OFFLOAD_get_num_devices (void)
>return num_devices;
>  }
>  
> +extern "C" union gomp_device_property_value
> +GOMP_OFFLOAD_get_property (int n, int prop)
> +{
> +  union gomp_device_property_value nullval = { .val = 0 };
> +
> +  if (n >= num_devices)
> +{
> +  GOMP_PLUGIN_error
> +   ("Request for a property of a non-existing Intel MIC device %i", n);
> +  return nullval;
> +}
> +
> +  switch (prop)
> +{
> +case GOMP_DEVICE_PROPERTY_VENDOR:
> +  /* TODO: "error: invalid conversion from 'const void*' to 'void*' 
> [-fpermissive]" */
> +  return (union gomp_device_property_value) { .ptr =  (char *) "Intel" };

Type cast maybe unnecessary per my 'libgomp/libgomp-plugin.h' comment above?

> +default:
> +  return nullval;
> +}
> +}


Grüße
 Thomas


From 51a21aab33902fbe85cb8bdc42ee07799e69051e Mon Sep 17 00:00:00 2001
From: Frederik Harwath 
Date: Thu, 14 Nov 2019 16:35:31 +0100
Subject: [PATCH] Add OpenACC 2.6 `acc_get_property' support

Hi,
this patch implements OpenACC 2.6 "acc_get_property" and related functions.
I have tested the patch on x86_64-linux-gnu with nvptx-none offloading.
There is no AMD GCN support yet. This will be added later on.

Can this be committed to trunk?

Best regards,
Frederik

--- 8< ---

Add generic support for the OpenACC 2.6 `acc_get_property' and
`acc_get_property_string' routines, as well as full handlers for the
host and the NVPTX offload targets and minimal handlers for the HSA
and Intel MIC offload targets.

Included are C/C++ and Fortran tests that, in particular, print
the property values for acc_property_vendor, acc_property_memory,
acc_property_free_memory, acc_property_name, and acc_property_driver.
The output looks as follows:

Vendor: GNU
Name: GOMP
Total memory: 0
Free memory: 0
Driver: 1.0

with the host driver (where the memory related properties are not
supported for the host device and yield 0, conforming to the standard)
and output like:

OpenACC vendor: Nvidia
OpenACC total memory: 12651462656
OpenACC free memory: 12202737664
OpenACC name: TITAN V
OpenACC driver: CUDA Driver 9.1

with the NVPTX driver.

2019-11-14  Maciej W. Rozycki  
	Frederik Harwath  
	Thomas Schwinge  

	include/
	* gomp-constants.h (GOMP_DEVICE_CURRENT,
	GOMP_DEVICE_PROPERTY_MEMORY, GOMP_DEVICE_PROPERTY_FREE_MEMORY,
	GOMP_DEVICE_PROPERTY_NAME, GOMP_DEVICE_PROPERTY_VENDOR,
	GOMP_DEVICE_PROPERTY_DRIVER, GOMP_DEVICE_PROPERTY_STRING_MASK):
	New Macros.

	libgomp/
	* libgomp.h (gomp_device_descr): Add `get_property_func' member.
	* libgomp-plugin.h (gomp_device_property_value): New union.
	(gomp_device_property_value): New prototype.
	* openacc.h (acc_device_t): Add `acc_device_current' enumeration
	constant.
	(acc_device_property_t): New enum.
	(acc_get_property, acc_get_property_string): New prototypes.
	* oacc-init.c (acc_get_device_type): Also assert on
	`!acc_device_current' result.
	(get_property_any, acc_get_property, acc_get_property_string):
	New functions.
	* openacc.f90 (openacc_kinds): From `iso_fortran_env' also
	import `int64'.  Add `acc_device_current' and
	`acc_property_memory', `acc_property_free_memory',
	`acc_property_name', `acc_property_vendor' and
	`acc_property_driver' constants.  Add

[PATCH] Add OpenACC 2.6 `acc_get_property' support

2019-11-14 Thread Frederik Harwath

Hi,
this patch implements OpenACC 2.6 "acc_get_property" and related functions.
I have tested the patch on x86_64-linux-gnu with nvptx-none offloading.
There is no AMD GCN support yet. This will be added later on.

Can this be committed to trunk?

Best regards,
Frederik

--- 8< ---

Add generic support for the OpenACC 2.6 `acc_get_property' and
`acc_get_property_string' routines, as well as full handlers for the
host and the NVPTX offload targets and minimal handlers for the HSA
and Intel MIC offload targets.

Included are C/C++ and Fortran tests that, in particular, print
the property values for acc_property_vendor, acc_property_memory,
acc_property_free_memory, acc_property_name, and acc_property_driver.
The output looks as follows:

Vendor: GNU
Name: GOMP
Total memory: 0
Free memory: 0
Driver: 1.0

with the host driver (where the memory related properties are not
supported for the host device and yield 0, conforming to the standard)
and output like:

OpenACC vendor: Nvidia
OpenACC total memory: 12651462656
OpenACC free memory: 12202737664
OpenACC name: TITAN V
OpenACC driver: CUDA Driver 9.1

with the NVPTX driver.

2019-11-14  Maciej W. Rozycki  
Frederik Harwath  
Thomas Schwinge  

include/
* gomp-constants.h (GOMP_DEVICE_CURRENT,
GOMP_DEVICE_PROPERTY_MEMORY, GOMP_DEVICE_PROPERTY_FREE_MEMORY,
GOMP_DEVICE_PROPERTY_NAME, GOMP_DEVICE_PROPERTY_VENDOR,
GOMP_DEVICE_PROPERTY_DRIVER, GOMP_DEVICE_PROPERTY_STRING_MASK):
New Macros.

libgomp/
* libgomp.h (gomp_device_descr): Add `get_property_func' member.
* libgomp-plugin.h (gomp_device_property_value): New union.
(gomp_device_property_value): New prototype.
* openacc.h (acc_device_t): Add `acc_device_current' enumeration
constant.
(acc_device_property_t): New enum.
(acc_get_property, acc_get_property_string): New prototypes.
* oacc-init.c (acc_get_device_type): Also assert on
`!acc_device_current' result.
(get_property_any, acc_get_property, acc_get_property_string):
New functions.
* openacc.f90 (openacc_kinds): From `iso_fortran_env' also
import `int64'.  Add `acc_device_current' and
`acc_property_memory', `acc_property_free_memory',
`acc_property_name', `acc_property_vendor' and
`acc_property_driver' constants.  Add `acc_device_property' data
type.
(openacc_internal): Add `acc_get_property' and
`acc_get_property_string' interfaces.  Add `acc_get_property_h',
`acc_get_property_string_h', `acc_get_property_l' and
`acc_get_property_string_l'.
(openacc_c_string): New module.
* oacc-host.c (host_get_property): New function.
(host_dispatch): Wire it.
* target.c (gomp_load_plugin_for_device): Handle `get_property'.
* libgomp.map (OACC_2.6): Add `acc_get_property',
`acc_get_property_h_', `acc_get_property_string' and
`acc_get_property_string_h_' symbols.
* oacc-init.c (acc_known_device_type): Add function.
(unknown_device_type_error): Add function.
(name_of_acc_device_t): Change to call unknown_device_type_error
on unknown type.
(resolve_device): Use acc_known_device_type.
(acc_init): Fail if acc_device_t argument is not valid.
(acc_shutdown): Likewise.
(acc_get_num_devices): Likewise.
(acc_set_device_type): Likewise.
(acc_get_device_num): Likewise.
(acc_set_device_num): Likewise.
(get_property_any): Likewise.
(acc_get_property): Likewise.
(acc_get_property_string): Likewise.
(acc_on_device): Likewise.
(goacc_save_and_set_bind): Likewise.

* libgomp.texi (OpenACC Runtime Library Routines): Add
`acc_get_property'.
(acc_get_property): New node.

* plugin/plugin-hsa.c (GOMP_OFFLOAD_get_property): New function.
* plugin/plugin-nvptx.c (CUDA_CALLS): Add `cuDeviceGetName',
`cuDeviceTotalMem', `cuDriverGetVersion' and `cuMemGetInfo'
calls.
(GOMP_OFFLOAD_get_property): New function.
(struct ptx_device): Add new field "name" ...
(nvptx_open_device): ... and alloc and init from here.
(nvptx_close_device): ... and free from here.
(cuda_driver_version): Add new static variable ...
(nvptx_init): ... and init from here.

* testsuite/libgomp.oacc-c-c++-common/acc-get-property.c: New
test.
* testsuite/libgomp.oacc-c-c++-common/acc-get-property-2.c: New
test.
 * testsuite/libgomp.oacc-c-c++-common/acc-get-property-3.c: New
test.
 * testsuite/libgomp.oacc-c-c++-common/acc-get-property-aux.c: New
file with test helper functions.

* testsuite/libgomp.oacc-fortran/acc-get-property.f90: New test.

liboffloadmic/
*

Re: Add OpenACC 2.6 `acc_get_property' support

2019-11-05 Thread Harwath, Frederik

Hi Thomas,

> On 07.10.19 20:41, Thomas Schwinge wrote:
> > On 2018-12-03T16:51:14+, "Maciej W. Rozycki"  
> > wrote:
> > Add generic support for the OpenACC 2.6 `acc_get_property' and
> > `acc_get_property_string' routines [...]
>
> ..., which allow for user code to query the implementation for stuff like:
>
> > OpenACC vendor: GNU
> > OpenACC name: GOMP
> > OpenACC driver: 1.0
>
> [...]
>
> > --- a/include/gomp-constants.h
> > +++ b/include/gomp-constants.h
> > @@ -215,10 +215,24 @@ enum gomp_map_kind
> >  #define GOMP_DEVICE_NVIDIA_PTX 5
> >  #define GOMP_DEVICE_INTEL_MIC  6
> >  #define GOMP_DEVICE_HSA7
> > +#define GOMP_DEVICE_CURRENT8
>
> This is used for 'acc_device_current', relevant only for
> 'acc_get_property', to return "the value of the property for the current
> device".  This should probably use a more special (negative?) value
> instead of eight, so that when additional real device types are added
> later on, we can just add them with increasing numbers, and keep the
> scanning code simple.

Yes, I use the first unused negative value.

> (Use of 'acc_device_current' as an argument to other functions taking an
> 'acc_device_t' is undefined, and should be rejected with 'gomp_fatal'?)

So far, there seems to be essentially no validity checking for acc_device_t
and other enums in the relevant parts of the code. I have added such checks
to public functions which take acc_device_t arguments.

> > --- a/libgomp/plugin/plugin-nvptx.c
> > +++ b/libgomp/plugin/plugin-nvptx.c
>
> > +union gomp_device_property_value
> > +GOMP_OFFLOAD_get_property (int n, int prop)
> > +{
> > +  union gomp_device_property_value propval = { .val = 0 };
> > +
> > +  pthread_mutex_lock (_dev_lock);
> > +
> > +  if (!nvptx_init () || n >= nvptx_get_num_devices ())
> > +{
> > +  pthread_mutex_unlock (_dev_lock);
> > +  return propval;
> > +}
>
> Isn't it implicit that 'get_num_devices' has been called while loading
> the plugin, so we don't have to do any initialization that here?  (But I
> may be misremembering that.)

Yes, a call path for nvptx_get_num_devices during initialization, in case we
are using the nvptx plugin:
acc_init (oacc_init.c) -> gomp_init_targets_once (target.c) ->
gomp_target_init (target.c) -> GOMP_OFFLOAD_get_num_devices (plugin-nvptx.c) ->
nvptx_get_num_devices

For nvptx_init, a call path is:
acc_init (oacc_init.c) -> acc_init_1 (oacc_init.c) -> gomp_init_device 
(oacc_init.c) ->
GOMP_OFFLOAD_init_device (plugin-nvptx.c) -> nvptx_init

Hence, yes, we should not call nvptx_init from here.


> > +  switch (prop)
> > +{
> > +case GOMP_DEVICE_PROPERTY_MEMORY:
> > +  {
> > +   size_t total_mem;
> > +   CUdevice dev;
> > +
> > +   CUDA_CALL_ERET (propval, cuDeviceGet, , n);
>
> Isn't that already known as 'ptx_devices[n]'?  (Likewise elsewhere.)

Yes, that gets set during GOMP_OFFLOAD_init_device.

> + CUDA_CALL_ERET (propval, cuDeviceTotalMem, _mem, dev);
> + propval.val = total_mem;
> +  }
> +  break;

> +case GOMP_DEVICE_PROPERTY_NAME:
> +  {
> + static char name[256];
> + CUdevice dev;
> +
> + CUDA_CALL_ERET (propval, cuDeviceGet, , n);
> + CUDA_CALL_ERET (propval, cuDeviceGetName, name, sizeof (name), dev);
> + propval.ptr = name;
> +  }
> +  break;

> Uh, that's not thread-safe, is it?

Not at all.

> Otherwise, perhaps make this 'name' a property of 'struct ptx_device' in
> the nvptx plugin here, and keep it live while the device is open
> ('nvptx_open_device'), together with other per-device data?

That's what I do now.

> Is that 'snprintf' formatting the generic way to display a CUDA driver
> version number?

At least, the same formatting is applied by NVidia's deviceQuery example
from cuda-samples
(i.e. 
https://github.com/NVIDIA/cuda-samples/blob/master/Samples/deviceQuery/deviceQuery.cpp#L106).
For me, the output yields "CUDA Driver Version / Runtime Version 9.1 / 9.1" with
the nvidia-cuda-toolkit 9.1.


> > As, in theory, such Nvidia GPU offloading support could also be
> > implemented via the Nouveau/Mesa GalliumCompute driver, should the string
> > returned here actually include "CUDA Driver"?

This seems like a good way to disambiguate between different drivers, but I am 
not sure if there
are any compatibility issues that we have to consider (PGI?). The standard does 
not impose
any restrictions on the format of the string.


> > +default:
> > +  break;
>
> Should this 'GOMP_PLUGIN_error' or even 'GOMP_PLUGIN_fatal'?  (Similar
> then elsewhere.)

Yes, I chose GOMP_PLUGIN_error.

> > --- /dev/null
> > +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc-get-property.c
> > @@ -0,0 +1,37 @@
> > +/* Test the `acc_get_property' and '`acc_get_property_string' library
> > +   functions. */
> > +/* { dg-do run } */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +int main ()
> > +{
> > +  const char *s;
> > +  size_t

Add OpenACC 2.6 `acc_get_property' support

2019-10-07 Thread Thomas Schwinge

).  (So, returning a 'malloc'ed object would be a memory
leak, for example.)  Best would probably be to return some 'const char *'
living in read-only program memory.  (But that likely is not available,
otherwise I suppose Maciej would have done that.)

Otherwise, perhaps make this 'name' a property of 'struct ptx_device' in
the nvptx plugin here, and keep it live while the device is open
('nvptx_open_device'), together with other per-device data?

> +case GOMP_DEVICE_PROPERTY_DRIVER:
> +  {
> + static char ver[11];
> + int v;
> +
> + CUDA_CALL_ERET (propval, cuDriverGetVersion, );
> + snprintf (ver, sizeof (ver) - 1, "%u.%u", v / 1000, v % 1000 / 10);
> + propval.ptr = ver;
> +  }
> +  break;

Similar here.

Is that 'snprintf' formatting the generic way to display a CUDA driver
version number?  

As, in theory, such Nvidia GPU offloading support could also be
implemented via the Nouveau/Mesa GalliumCompute driver, should the string
returned here actually include "CUDA Driver"?

> +default:
> +  break;

Should this 'GOMP_PLUGIN_error' or even 'GOMP_PLUGIN_fatal'?  (Similar
then elsewhere.)


> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc-get-property.c
> @@ -0,0 +1,37 @@
> +/* Test the `acc_get_property' and '`acc_get_property_string' library
> +   functions. */
> +/* { dg-do run } */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +int main ()
> +{
> +  const char *s;
> +  size_t v;
> +  int r;
> +
> +  /* Verify that the vendor is a proper non-empty string.  */
> +  s = acc_get_property_string (0, acc_device_default, acc_property_vendor);
> +  r = !s || !strlen (s);
> +  if (s)
> +printf ("OpenACC vendor: %s\n", s);

Should we check the actual string returned, as defined by OpenACC/our
implementation, as applicable?  Use '#if defined ACC_DEVICE_TYPE_[...]'.
(See 'libgomp/testsuite/libgomp.oacc-c-c++-common/avoid-offloading-2.c',
for example.)

Isn't this the "Device vendor" instead of the "OpenACC vendor"?  Similar
for all other 'printf's?

> +
> +  /* For the rest just check that they do not crash.  */
> +  v = acc_get_property (0, acc_device_default, acc_property_memory);
> +  if (v)
> +printf ("OpenACC total memory: %zd\n", v);
> +  v = acc_get_property (0, acc_device_default, acc_property_free_memory);
> +  if (v)
> +printf ("OpenACC free memory: %zd\n", v);
> +  s = acc_get_property_string (0, acc_device_default, acc_property_name);
> +  if (s)
> +printf ("OpenACC name: %s\n", s);
> +  s = acc_get_property_string (0, acc_device_default, acc_property_driver);
> +  if (s)
> +printf ("OpenACC driver: %s\n", s);
> +
> +  return r;
> +}

Likewise, as we define the implementation, we can declare that something
reasonable must have been returned, so don't need 'if (s)' checks, but
should instead verify 's' to the extent possible.

> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.oacc-fortran/acc-get-property.f

Similar.  (See
'libgomp/testsuite/libgomp.oacc-fortran/avoid-offloading-2.f', for
example.)

These tests only use 'acc_device_default', should they also check other
valid as well as invalid values?


Grüße
 Thomas


From 4674caa90e82c209db51bf1fb5d7ec42364d47a2 Mon Sep 17 00:00:00 2001
From: "Maciej W. Rozycki" 
Date: Thu, 20 Dec 2018 14:10:17 +
Subject: [PATCH] Add OpenACC 2.6 `acc_get_property' support

Add generic support for the OpenACC 2.6 `acc_get_property' and
`acc_get_property_string' routines, as well as full handlers for the
host and the NVPTX offload targets and a minimal handler for the HSA
offload target.

Include test cases for both C/C++ and Fortran support, both producing:

OpenACC vendor: GNU
OpenACC name: GOMP
OpenACC driver: 1.0

with the host driver and output like:

OpenACC vendor: Nvidia
OpenACC total memory: 12651462656
OpenACC free memory: 12202737664
OpenACC name: TITAN V
OpenACC driver: 9.1

with the NVPTX driver.

	include/
	* gomp-constants.h (GOMP_DEVICE_CURRENT): New macro.
	(GOMP_DEVICE_PROPERTY_MEMORY, GOMP_DEVICE_PROPERTY_FREE_MEMORY)
	(GOMP_DEVICE_PROPERTY_NAME, GOMP_DEVICE_PROPERTY_VENDOR)
	(GOMP_DEVICE_PROPERTY_DRIVER): Likewise.
	(GOMP_DEVICE_PROPERTY_STRING_MASK): Likewise.

	libgomp/
	* libgomp.h (gomp_device_descr): Add `get_property_func' member.
	* libgomp-plugin.h (gomp_device_property_value): New union.
	(gomp_device_property_value): New prototype.
	* openacc.h (acc_device_t): Add `acc_device_current' enumeration
	constant.
	(acc_device_property_t): New enum.
	(acc_get_property, acc_get_property_string): New prototypes.
	* oacc-init.c (acc_get_device_type): Also assert on
	`!acc_device_current' result.
	(get_property_any, acc_get_property, acc_get_property_string):
	New functions.

Re: [PATCH, og8] Add OpenACC 2.6 `acc_get_property' support

2019-01-08 Thread Thomas Schwinge

Hi Maciej!

On Mon, 3 Dec 2018 16:51:14 +, "Maciej W. Rozycki"  
wrote:
> Add generic support for the OpenACC 2.6 `acc_get_property' and 
> `acc_get_property_string' routines, as well as full handlers for the 
> host and the NVPTX offload targets and a minimal handler for the HSA 
> offload target.

..., but a similar minimal handler for the Intel MIC offload plugin
missing.  ;-) (... which "for reasons" doesn't live in "libgomp/plugin/",
next to the other plugins.)

To fix that, I pushed the attached to openacc-gcc-8-branch, with the code
copied and adjusted from your minimal handler for the HSA offload target,
but also with an additional TODO added.


Grüße
 Thomas


>From c632bd83096f1a4e4ed59161797087ff800e4c23 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Tue, 8 Jan 2019 15:21:35 +0100
Subject: [PATCH] Add OpenACC 2.6 `acc_get_property' support: restore Intel MIC
 offloading

The "OpenACC 2.6 `acc_get_property' support" changes regressed the relevant
libgomp OpenMP execution test cases to no longer consider Intel MIC offloading
because of:

libgomp: while loading libgomp-plugin-intelmic.so.1: [...]/libgomp-plugin-intelmic.so.1: undefined symbol: GOMP_OFFLOAD_get_property

	liboffloadmic/
	* plugin/libgomp-plugin-intelmic.cpp (GOMP_OFFLOAD_get_property):
	New function.
---
 liboffloadmic/ChangeLog.openacc   | 10 +
 .../plugin/libgomp-plugin-intelmic.cpp| 21 +++
 2 files changed, 31 insertions(+)
 create mode 100644 liboffloadmic/ChangeLog.openacc

diff --git a/liboffloadmic/ChangeLog.openacc b/liboffloadmic/ChangeLog.openacc
new file mode 100644
index ..2e666da2ce0c
--- /dev/null
+++ b/liboffloadmic/ChangeLog.openacc
@@ -0,0 +1,10 @@
+2019-01-08  Thomas Schwinge  
+
+	* plugin/libgomp-plugin-intelmic.cpp (GOMP_OFFLOAD_get_property):
+	New function.
+
+Copyright (C) 2019 Free Software Foundation, Inc.
+
+Copying and distribution of this file, with or without modification,
+are permitted in any medium without royalty provided the copyright
+notice and this notice are preserved.
diff --git a/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp b/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
index d1678d0514e9..f74941c2b549 100644
--- a/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
+++ b/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
@@ -174,6 +174,27 @@ GOMP_OFFLOAD_get_num_devices (void)
   return num_devices;
 }
 
+extern "C" union gomp_device_property_value
+GOMP_OFFLOAD_get_property (int n, int prop)
+{
+  union gomp_device_property_value nullval = { .val = 0 };
+
+  if (n >= num_devices)
+{
+  GOMP_PLUGIN_error
+	("Request for a property of a non-existing Intel MIC device %i", n);
+  return nullval;
+}
+
+  switch (prop)
+{
+case GOMP_DEVICE_PROPERTY_VENDOR:
+  return (union gomp_device_property_value) { .ptr = /* TODO: "error: invalid conversion from 'const void*' to 'void*' [-fpermissive]" */ (char *) "Intel" };
+default:
+  return nullval;
+}
+}
+
 static bool
 offload (const char *file, uint64_t line, int device, const char *name,
 	 int num_vars, VarDesc *vars, const void **async_data)
-- 
2.17.1

Re: [PATCH, og8] Add OpenACC 2.6 `acc_get_property' support

2018-12-20 Thread Maciej W. Rozycki

On Mon, 10 Dec 2018, Chung-Lin Tang wrote:

> I think the patch is okay, although still needs approval from Thomas and Tom
> to commit.

 I have committed this change now, thank you for your review.

  Maciej

Re: [PATCH, og8] Add OpenACC 2.6 `acc_get_property' support

2018-12-10 Thread Chung-Lin Tang


On 2018/12/6 2:16 AM, Maciej W. Rozycki wrote:

AFAIK, things like strlen are already available in iso_c_binding, in forms
like "C_strlen".
Can you check again if that 'openacc_c_string' module is really necessary?

  Any pointers please?

  I can't see `c_strlen' or any equivalent interface defined either in the
Fortran 2003 language standard or in GCC documentation, and neither `grep'
over the GCC tree shows anything relevant.  The `iso_c_binding' module
defines only a bunch of procedures according to said documentation.  The
`strlen' function provided here has been taken from one of our Fortran
test cases, which strongly indicates there's no such API already available
or whoever wrote the test case would have chosen to use it I suppose.


Okay I see. I think I mixed up the common convention with the actual interface
standard.


+   CUcontext new_ctx;
+
+   CUDA_CALL_ERET (propval, cuCtxCreate, _ctx, CU_CTX_SCHED_AUTO,
+   dev);
+   CUDA_CALL_ERET (propval, cuMemGetInfo, _mem, _mem);
+   CUDA_CALL_ASSERT (cuCtxDestroy, new_ctx);
+ }

(I'm CCing Tom here, as he is maintainer for these parts)

As we discussed earlier on our internal list, I think properly using
GOMP_OFFLOAD_init_device
is the right way, instead of using the lower level CUDA context
create/destroy.

I did not mean for you to first init the device and then immediately destroy
it by
GOMP_OFFLOAD_fini_device, just to obtain the property, but for you to just
take the opportunity to initialize
it for use, and leave it there until program exit. That should save resources
overall.
(BTW, CUDA contexts should be quite expensive to create/destroy, using a
cuCtxCreate/Destroy pair is probably
almost as slow)

  I have argued that this looks like a corner-case use case to me, as
querying for the remaining (rather than total) memory available to a
device that hasn't been (yet) used looks like of hardly any use to me,
because obviously at such a stage no memory has been used.  The OpenACC
standard does require us to handle such a request somehow, with returning
0 being another option, however I thought we may well have a quick peek
without pulling in all the state.

  I guess I have no strong opinion either way and I can adapt accordingly.

  NB that would have to be `gomp_init_device' rather than
`GOMP_OFFLOAD_init_device' AFAICS.


You'll have to use GOMP_OFFLOAD_init_device, as you are inside the plugin, 
gomp_init_device()
should not be available.

However, looking into this further, the checking conventions of 
GOMP_OFFLOAD_init_device
will have to be slightly tweaked to accommodate possible further initing from 
libgomp proper,
so this may requirement a longer string of changes...I think it's not worth it, 
or can
be adjusted later. I now think your current approach with the CUDA contexts is 
okay.

I think the patch is okay, although still needs approval from Thomas and Tom to 
commit.

Thanks,
Chung-Lin

Re: [PATCH, og8] Add OpenACC 2.6 `acc_get_property' support

2018-12-05 Thread Maciej W. Rozycki

Hi Chung-Lin,

> > +module openacc_c_string
> > +  implicit none
> > +
> > +  interface
> > +function strlen (s) bind (C, name = "strlen")
> > +  use iso_c_binding, only: c_ptr, c_size_t
> > +  type (c_ptr), intent(in), value :: s
> > +  integer (c_size_t) :: strlen
> > +end function
> > +  end interface
> > +
> > +end module
> 
> > +subroutine acc_get_property_string_h (n, d, p, s)
> > +  use iso_c_binding, only: c_char, c_int, c_ptr, c_f_pointer
> > +  use openacc_internal, only: acc_get_property_string_l
> > +  use openacc_c_string, only: strlen
> > +  use openacc_kinds
> ...> +  pint = int (p, c_int)
> > +  cptr = acc_get_property_string_l (n, d, pint)
> > +  clen = int (strlen (cptr))
> > +  call c_f_pointer (cptr, sptr, [clen])
> 
> AFAIK, things like strlen are already available in iso_c_binding, in forms
> like "C_strlen".
> Can you check again if that 'openacc_c_string' module is really necessary?

 Any pointers please?

 I can't see `c_strlen' or any equivalent interface defined either in the 
Fortran 2003 language standard or in GCC documentation, and neither `grep' 
over the GCC tree shows anything relevant.  The `iso_c_binding' module 
defines only a bunch of procedures according to said documentation.  The 
`strlen' function provided here has been taken from one of our Fortran 
test cases, which strongly indicates there's no such API already available 
or whoever wrote the test case would have chosen to use it I suppose.

> > +union gomp_device_property_value
> > +GOMP_OFFLOAD_get_property (int n, int prop)
> > +{
> > +  union gomp_device_property_value propval = { .val = 0 };
> > +
> > +  pthread_mutex_lock (_dev_lock);
> > +
> > +  if (!nvptx_init () || n >= nvptx_get_num_devices ())
> > +{
> > +  pthread_mutex_unlock (_dev_lock);
> > +  return propval;
> > +}
> > +
> > +  switch (prop)
> > +{
> > +case GOMP_DEVICE_PROPERTY_MEMORY:
> > +  {
> > +   size_t total_mem;
> > +   CUdevice dev;
> > +
> > +   CUDA_CALL_ERET (propval, cuDeviceGet, , n);
> > +   CUDA_CALL_ERET (propval, cuDeviceTotalMem, _mem, dev);
> > +   propval.val = total_mem;
> > +  }
> > +  break;
> > +case GOMP_DEVICE_PROPERTY_FREE_MEMORY:
> > +  {
> > +   size_t total_mem;
> > +   size_t free_mem;
> > +   CUdevice ctxdev;
> > +   CUdevice dev;
> > +
> > +   CUDA_CALL_ERET (propval, cuCtxGetDevice, );
> > +   CUDA_CALL_ERET (propval, cuDeviceGet, , n);
> > +   if (dev == ctxdev)
> > + CUDA_CALL_ERET (propval, cuMemGetInfo, _mem, _mem);
> > +   else if (ptx_devices[n])
> > + {
> > +   CUcontext old_ctx;
> > +
> > +   CUDA_CALL_ERET (propval, cuCtxPushCurrent, ptx_devices[n]->ctx);
> > +   CUDA_CALL_ERET (propval, cuMemGetInfo, _mem, _mem);
> > +   CUDA_CALL_ASSERT (cuCtxPopCurrent, _ctx);
> > + }
> > +   else
> > + {
> > +   CUcontext new_ctx;
> > +
> > +   CUDA_CALL_ERET (propval, cuCtxCreate, _ctx, CU_CTX_SCHED_AUTO,
> > +   dev);
> > +   CUDA_CALL_ERET (propval, cuMemGetInfo, _mem, _mem);
> > +   CUDA_CALL_ASSERT (cuCtxDestroy, new_ctx);
> > + }
> 
> (I'm CCing Tom here, as he is maintainer for these parts)
> 
> As we discussed earlier on our internal list, I think properly using
> GOMP_OFFLOAD_init_device
> is the right way, instead of using the lower level CUDA context
> create/destroy.
> 
> I did not mean for you to first init the device and then immediately destroy
> it by
> GOMP_OFFLOAD_fini_device, just to obtain the property, but for you to just
> take the opportunity to initialize
> it for use, and leave it there until program exit. That should save resources
> overall.
> (BTW, CUDA contexts should be quite expensive to create/destroy, using a
> cuCtxCreate/Destroy pair is probably
> almost as slow)

 I have argued that this looks like a corner-case use case to me, as 
querying for the remaining (rather than total) memory available to a 
device that hasn't been (yet) used looks like of hardly any use to me, 
because obviously at such a stage no memory has been used.  The OpenACC 
standard does require us to handle such a request somehow, with returning 
0 being another option, however I thought we may well have a quick peek 
without pulling in all the state.

 I guess I have no strong opinion either way and I can adapt accordingly.

 NB that would have to be `gomp_init_device' rather than 
`GOMP_OFFLOAD_init_device' AFAICS.

  Maciej

Re: [PATCH, og8] Add OpenACC 2.6 `acc_get_property' support

2018-12-05 Thread Chung-Lin Tang


Hi Maciej, please see below:

On 2018/12/4 12:51 AM, Maciej W. Rozycki wrote:

+module openacc_c_string
+  implicit none
+
+  interface
+function strlen (s) bind (C, name = "strlen")
+  use iso_c_binding, only: c_ptr, c_size_t
+  type (c_ptr), intent(in), value :: s
+  integer (c_size_t) :: strlen
+end function
+  end interface
+
+end module



+subroutine acc_get_property_string_h (n, d, p, s)
+  use iso_c_binding, only: c_char, c_int, c_ptr, c_f_pointer
+  use openacc_internal, only: acc_get_property_string_l
+  use openacc_c_string, only: strlen
+  use openacc_kinds

...> +  pint = int (p, c_int)

+  cptr = acc_get_property_string_l (n, d, pint)
+  clen = int (strlen (cptr))
+  call c_f_pointer (cptr, sptr, [clen])


AFAIK, things like strlen are already available in iso_c_binding, in forms like 
"C_strlen".
Can you check again if that 'openacc_c_string' module is really necessary?


+union gomp_device_property_value
+GOMP_OFFLOAD_get_property (int n, int prop)
+{
+  union gomp_device_property_value propval = { .val = 0 };
+
+  pthread_mutex_lock (_dev_lock);
+
+  if (!nvptx_init () || n >= nvptx_get_num_devices ())
+{
+  pthread_mutex_unlock (_dev_lock);
+  return propval;
+}
+
+  switch (prop)
+{
+case GOMP_DEVICE_PROPERTY_MEMORY:
+  {
+   size_t total_mem;
+   CUdevice dev;
+
+   CUDA_CALL_ERET (propval, cuDeviceGet, , n);
+   CUDA_CALL_ERET (propval, cuDeviceTotalMem, _mem, dev);
+   propval.val = total_mem;
+  }
+  break;
+case GOMP_DEVICE_PROPERTY_FREE_MEMORY:
+  {
+   size_t total_mem;
+   size_t free_mem;
+   CUdevice ctxdev;
+   CUdevice dev;
+
+   CUDA_CALL_ERET (propval, cuCtxGetDevice, );
+   CUDA_CALL_ERET (propval, cuDeviceGet, , n);
+   if (dev == ctxdev)
+ CUDA_CALL_ERET (propval, cuMemGetInfo, _mem, _mem);
+   else if (ptx_devices[n])
+ {
+   CUcontext old_ctx;
+
+   CUDA_CALL_ERET (propval, cuCtxPushCurrent, ptx_devices[n]->ctx);
+   CUDA_CALL_ERET (propval, cuMemGetInfo, _mem, _mem);
+   CUDA_CALL_ASSERT (cuCtxPopCurrent, _ctx);
+ }
+   else
+ {
+   CUcontext new_ctx;
+
+   CUDA_CALL_ERET (propval, cuCtxCreate, _ctx, CU_CTX_SCHED_AUTO,
+   dev);
+   CUDA_CALL_ERET (propval, cuMemGetInfo, _mem, _mem);
+   CUDA_CALL_ASSERT (cuCtxDestroy, new_ctx);
+ }


(I'm CCing Tom here, as he is maintainer for these parts)

As we discussed earlier on our internal list, I think properly using 
GOMP_OFFLOAD_init_device
is the right way, instead of using the lower level CUDA context create/destroy.

I did not mean for you to first init the device and then immediately destroy it 
by
GOMP_OFFLOAD_fini_device, just to obtain the property, but for you to just take 
the opportunity to initialize
it for use, and leave it there until program exit. That should save resources 
overall.
(BTW, CUDA contexts should be quite expensive to create/destroy, using a 
cuCtxCreate/Destroy pair is probably
almost as slow)

Tom, do you have any comments on how to best write this part?

Thanks,
Chung-Lin

[PATCH, og8] Add OpenACC 2.6 `acc_get_property' support

2018-12-03 Thread Maciej W. Rozycki

Add generic support for the OpenACC 2.6 `acc_get_property' and 
`acc_get_property_string' routines, as well as full handlers for the 
host and the NVPTX offload targets and a minimal handler for the HSA 
offload target.

Include test cases for both C/C++ and Fortran support, both producing:

OpenACC vendor: GNU
OpenACC name: GOMP
OpenACC driver: 1.0

with the host driver and output like:

OpenACC vendor: Nvidia
OpenACC total memory: 12651462656
OpenACC free memory: 12202737664
OpenACC name: TITAN V
OpenACC driver: 9.1

with the NVPTX driver.

include/
* gomp-constants.h (GOMP_DEVICE_CURRENT): New macro.
(GOMP_DEVICE_PROPERTY_MEMORY, GOMP_DEVICE_PROPERTY_FREE_MEMORY)
(GOMP_DEVICE_PROPERTY_NAME, GOMP_DEVICE_PROPERTY_VENDOR)
(GOMP_DEVICE_PROPERTY_DRIVER): Likewise.
(GOMP_DEVICE_PROPERTY_STRING_MASK): Likewise.

libgomp/
* libgomp.h (gomp_device_descr): Add `get_property_func' member.
* libgomp-plugin.h (gomp_device_property_value): New union.
(gomp_device_property_value): New prototype.
* openacc.h (acc_device_t): Add `acc_device_current' enumeration
constant.
(acc_device_property_t): New enum.
(acc_get_property, acc_get_property_string): New prototypes.
* oacc-init.c (acc_get_device_type): Also assert on
`!acc_device_current' result.
(get_property_any, acc_get_property, acc_get_property_string): 
New functions.
* openacc.f90 (openacc_kinds): From `iso_fortran_env' also 
import `int64'.  Add `acc_device_current' and 
`acc_property_memory', `acc_property_free_memory', 
`acc_property_name', `acc_property_vendor' and
`acc_property_driver' constants.  Add `acc_device_property' data
type.
(openacc_internal): Add `acc_get_property' and 
`acc_get_property_string' interfaces.  Add `acc_get_property_h',
`acc_get_property_string_h', `acc_get_property_l' and 
`acc_get_property_string_l'.
(openacc_c_string): New module.
* oacc-host.c (host_get_property): New function.
(host_dispatch): Wire it.
* target.c (gomp_load_plugin_for_device): Handle `get_property'.
* libgomp.map (OACC_2.6): Add `acc_get_property', 
`acc_get_property_h_', `acc_get_property_string' and 
`acc_get_property_string_h_' symbols.
* libgomp.texi (OpenACC Runtime Library Routines): Add 
`acc_get_property'.
(acc_get_property): New node.

* plugin/plugin-hsa.c (GOMP_OFFLOAD_get_property): New function.
* plugin/plugin-nvptx.c (CUDA_CALLS): Add `cuDeviceGetName', 
`cuDeviceTotalMem', `cuDriverGetVersion' and `cuMemGetInfo' 
calls.
(GOMP_OFFLOAD_get_property): New function.

* testsuite/libgomp.oacc-c-c++-common/acc-get-property.c: New 
test.
* testsuite/libgomp.oacc-fortran/acc-get-property.f: New test.
---
Hi,

 This has passed regression-testing with the `x86_64-linux-gnu' target and 
the `nvptx-none' offload target.  I will appreciate feedback and if none 
has been given in a couple of days' time, then I will commit this change 
to the og8 branch.

  Maciej
---
 include/gomp-constants.h   |   14 +
 libgomp/libgomp-plugin.h   |8 
 libgomp/libgomp.h  |1 
 libgomp/libgomp.map|4 
 libgomp/libgomp.texi   |   39 +++
 libgomp/oacc-host.c|   22 +
 libgomp/oacc-init.c|   99 
 libgomp/openacc.f90|  116 
+-
 libgomp/openacc.h  |   15 +
 libgomp/plugin/plugin-hsa.c|   26 ++
 libgomp/plugin/plugin-nvptx.c  |   91 +++
 libgomp/target.c   |1 
 libgomp/testsuite/libgomp.oacc-c-c++-common/acc-get-property.c |   37 +++
 libgomp/testsuite/libgomp.oacc-fortran/acc-get-property.f  |   33 ++
 14 files changed, 504 insertions(+), 2 deletions(-)

gcc-openacc-acc-get-property.diff
Index: gcc-openacc-gcc-8-branch/include/gomp-constants.h
===
--- gcc-openacc-gcc-8-branch.orig/include/gomp-constants.h
+++ gcc-openacc-gcc-8-branch/include/gomp-constants.h
@@ -215,10 +215,24 @@ enum gomp_map_kind
 #define GOMP_DEVICE_NVIDIA_PTX 5
 #define GOMP_DEVICE_INTEL_MIC  6
 #define GOMP_DEVICE_HSA7
+#define GOMP_DEVICE_CURRENT8
 
 #define GOMP_DEVICE_ICV-1
 #define GOMP_DEVICE_HOST_FALLBACK  -2
 
+/* Device property codes.  Keep in sync with
+

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

Re: Make OpenACC 'acc_get_property' with 'acc_device_current' work (was: [PATCH] Add OpenACC 2.6 `acc_get_property' support)

Make OpenACC 'acc_get_property' with 'acc_device_current' work (was: [PATCH] Add OpenACC 2.6 `acc_get_property' support)

Fortran 'acc_get_property' return type (was: [PATCH] Add OpenACC 2.6 `acc_get_property' support)

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

[PATCH] Add OpenACC 2.6 `acc_get_property' support

Re: Add OpenACC 2.6 `acc_get_property' support

Add OpenACC 2.6 `acc_get_property' support

Re: [PATCH, og8] Add OpenACC 2.6 `acc_get_property' support

Re: [PATCH, og8] Add OpenACC 2.6 `acc_get_property' support

Re: [PATCH, og8] Add OpenACC 2.6 `acc_get_property' support

Re: [PATCH, og8] Add OpenACC 2.6 `acc_get_property' support

Re: [PATCH, og8] Add OpenACC 2.6 `acc_get_property' support

[PATCH, og8] Add OpenACC 2.6 `acc_get_property' support

24 matches

Site Navigation

Mail list logo

Footer information