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

[Thomas Schwinge <thomas@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 9260 bytes --]

Hi!

On 2019-12-20T17:46:57+0100, "Harwath, Frederik" <frederik@codesourcery.com> 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 @@
>> >  !  <http://www.gnu.org/licenses/>.
>> >=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 <https://github.com/OpenACC/openacc-spec/issues/260> "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
> +  use iso_c_binding, only: c_size_t
>    implicit none
>  
>    public
> -  private :: int32
> +  private :: int32, c_size_t
>  

> @@ -89,6 +100,24 @@ module openacc_internal
>        integer (acc_device_kind) d
>      end function
>  
> +    function acc_get_property_h (n, d, p)
> +      import
> +      implicit none (type, external)
> +      integer (acc_device_property) :: acc_get_property_h
> +      integer, value :: n
> +      integer (acc_device_kind), value :: d
> +      integer (acc_device_property), value :: p
> +    end function
> +
> +    subroutine acc_get_property_string_h (n, d, p, s)
> +      import
> +      implicit none (type, external)
> +      integer, value :: n
> +      integer (acc_device_kind), value :: d
> +      integer (acc_device_property), value :: p
> +      character (*) :: s
> +    end subroutine
> +
>      function acc_async_test_h (a)
>        logical acc_async_test_h
>        integer a
> @@ -508,6 +537,26 @@ module openacc_internal
>        integer (c_int), value :: d
>      end function
>  
> +    function acc_get_property_l (n, d, p) &
> +        bind (C, name = "acc_get_property")
> +      use iso_c_binding, only: c_int, c_size_t
> +      implicit none (type, external)
> +      integer (c_size_t) :: acc_get_property_l
> +      integer (c_int), value :: n
> +      integer (c_int), value :: d
> +      integer (c_int), value :: p
> +    end function
> +
> +    function acc_get_property_string_l (n, d, p) &
> +        bind (C, name = "acc_get_property_string")
> +      use iso_c_binding, only: c_int, c_ptr
> +      implicit none (type, external)
> +      type (c_ptr) :: acc_get_property_string_l
> +      integer (c_int), value :: n
> +      integer (c_int), value :: d
> +      integer (c_int), value :: p
> +    end function

> @@ -758,6 +814,14 @@ module openacc
>      procedure :: acc_get_device_num_h
>    end interface
>  
> +  interface acc_get_property
> +    procedure :: acc_get_property_h
> +  end interface
> +
> +  interface acc_get_property_string
> +    procedure :: acc_get_property_string_h
> +  end interface
> +
>    interface acc_async_test
>      procedure :: acc_async_test_h
>    end interface
> @@ -976,6 +1040,63 @@ function acc_get_device_num_h (d)
>    acc_get_device_num_h = acc_get_device_num_l (d)
>  end function
>  
> +function acc_get_property_h (n, d, p)
> +  use iso_c_binding, only: c_int, c_size_t
> +  use openacc_internal, only: acc_get_property_l
> +  use openacc_kinds
> +  implicit none (type, external)
> +  integer (acc_device_property) :: acc_get_property_h
> +  integer, value :: n
> +  integer (acc_device_kind), value :: d
> +  integer (acc_device_property), value :: p
> +
> +  integer (c_int) :: pint
> +
> +  pint = int (p, c_int)
> +  acc_get_property_h = acc_get_property_l (n, d, pint)
> +end function
> +
> +subroutine acc_get_property_string_h (n, d, p, s)
> +  use iso_c_binding, only: c_char, c_int, c_ptr, c_f_pointer, c_associated
> +  use openacc_internal, only: acc_get_property_string_l
> +  use openacc_kinds
> +  implicit none (type, external)
> +  integer, value :: n
> +  integer (acc_device_kind), value :: d
> +  integer (acc_device_property), value :: p
> +  character (*) :: s
> +
> +  integer (c_int) :: pint
> +  type (c_ptr) :: cptr
> +  integer :: clen
> +  character (kind=c_char, len=1), pointer, contiguous :: sptr (:)
> +  integer :: slen
> +  integer :: i
> +
> +  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 strlen
> +  end interface
> +
> +  pint = int (p, c_int)
> +  cptr = acc_get_property_string_l (n, d, pint)
> +  s = ""
> +  if (.not. c_associated (cptr)) then
> +     return
> +  end if
> +
> +  clen = int (strlen (cptr))
> +  call c_f_pointer (cptr, sptr, [clen])
> +
> +  slen = min (clen, len (s))
> +  do i = 1, slen
> +    s (i:i) = sptr (i)
> +  end do
> +end subroutine

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 658 bytes --]