Hi!
On 2019-12-20T17:46:57+0100, "Harwath, Frederik" <frede...@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
signature.asc
Description: PGP signature
