Re: [ping] Add 'libgomp.{,oacc-}fortran/fortran-torture_execute_math.f90'

[Tobias Burnus <tobias@codesourcery.com>]

On 13.06.23 12:42, Thomas Schwinge wrote:
> On 2023-06-05T14:18:48+0200, I wrote:
>> OK to push the attached
>> "Add 'libgomp.{,oacc-}fortran/fortran-torture_execute_math.f90'"?
>
> Subject: [PATCH] Add
>   'libgomp.{,oacc-}fortran/fortran-torture_execute_math.f90'
>
>       gcc/testsuite/
>       * gfortran.fortran-torture/execute/math.f90: Enhance for optional
>       OpenACC, OpenMP 'target' usage.

I think it is more readable with a linebreak here and with "OpenACC
'serial' and OpenMP ..." instead of "OpenACC, OpenMP".

What I would like to see a hint somewhere in the commit log that the
libgomp files include the gfortran.fortran-torture file. I don't care
whether you add the hint before the changelog items as free text – or in
the bullet above (e.g. "as it is included in libgomp/testsuite") – or
after "New." in the following bullet list.

>       libgomp/
>       * testsuite/libgomp.fortran/fortran-torture_execute_math.f90: New.
>       * testsuite/libgomp.oacc-fortran/fortran-torture_execute_math.f90:
>       Likewise.

> ---
>   .../gfortran.fortran-torture/execute/math.f90 | 23 +++++++++++++++++--
>   .../fortran-torture_execute_math.f90          |  4 ++++
>   .../fortran-torture_execute_math.f90          |  5 ++++
>   3 files changed, 30 insertions(+), 2 deletions(-)
>   create mode 100644 libgomp/testsuite/libgomp.fortran/fortran-torture_execute_math.f90
>   create mode 100644 libgomp/testsuite/libgomp.oacc-fortran/fortran-torture_execute_math.f90
>
> diff --git a/gcc/testsuite/gfortran.fortran-torture/execute/math.f90 b/gcc/testsuite/gfortran.fortran-torture/execute/math.f90
> index 17cc78f7a10..e71f669304f 100644
> --- a/gcc/testsuite/gfortran.fortran-torture/execute/math.f90
> +++ b/gcc/testsuite/gfortran.fortran-torture/execute/math.f90
> @@ -1,9 +1,14 @@
>   ! Program to test mathematical intrinsics
> +
> +! See also 'libgomp/testsuite/libgomp.fortran/fortran-torture_execute_math.f90'; thus the '!$omp' directives.
> +! See also 'libgomp/testsuite/libgomp.oacc-fortran/fortran-torture_execute_math.f90'; thus the '!$acc' directives.

Likewise here: it is not completely obvious that this file is 'include'd
by the other testcases.

Maybe add a line "! This file is also included in:" and remove the "See
also" or some creative variant of it.

Minor remark: The OpenMP part is OK, but strict reading of the spec
requires an "omp declare target' if a subroutine is in a different
compilation unit. And according the glossary, that's the case here. In
practice, it also works without as it is in the same translation unit.
(compilation unit = for C/C++: translation unit, for Fortran:
subprogram). I think the HPE/Cray compiler will complain, but maybe only
when used with modules and not with subroutine subprograms. (As many
compilers write a .mod file for modules, a late change of attributes can
be more problematic.)

Otherwise LGTM.

Tobias

PS: I assume that you have check it with both with an in-build-tree and
an in-install-tree testsuite run.

> +
>   subroutine dotest (n, val4, val8, known)
>      implicit none
>      real(kind=4) val4, known
>      real(kind=8) val8
>      integer n
> +   !$acc routine seq
>
>      if (abs (val4 - known) .gt. 0.001) STOP 1
>      if (abs (real (val8, kind=4) - known) .gt. 0.001) STOP 2
> @@ -14,17 +19,20 @@ subroutine dotestc (n, val4, val8, known)
>      complex(kind=4) val4, known
>      complex(kind=8) val8
>      integer n
> +   !$acc routine seq
> +
>      if (abs (val4 - known) .gt. 0.001) STOP 3
>      if (abs (cmplx (val8, kind=4) - known) .gt. 0.001) STOP 4
>   end subroutine
>
> -program testmath
> +subroutine testmath
>      implicit none
>      real(kind=4) r, two4, half4
>      real(kind=8) q, two8, half8
>      complex(kind=4) cr
>      complex(kind=8) cq
>      external dotest, dotestc
> +   !$acc routine seq
>
>      two4 = 2.0
>      two8 = 2.0_8
> @@ -96,5 +104,16 @@ program testmath
>      cq = log ((-1.0_8, -1.0_8))
>      call dotestc (21, cr, cq, (0.3466, -2.3562))
>
> -end program
> +end subroutine
>
> +program main
> +   implicit none
> +   external testmath
> +
> +   !$acc serial
> +   !$omp target
> +   call testmath
> +   !$acc end serial
> +   !$omp end target
> +
> +end program
> diff --git a/libgomp/testsuite/libgomp.fortran/fortran-torture_execute_math.f90 b/libgomp/testsuite/libgomp.fortran/fortran-torture_execute_math.f90
> new file mode 100644
> index 00000000000..3348a0bb3ad
> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.fortran/fortran-torture_execute_math.f90
> @@ -0,0 +1,4 @@
> +! { dg-do run }
> +! { dg-additional-options -foffload-options=-lm }
> +
> +include '../../../gcc/testsuite/gfortran.fortran-torture/execute/math.f90'
> diff --git a/libgomp/testsuite/libgomp.oacc-fortran/fortran-torture_execute_math.f90 b/libgomp/testsuite/libgomp.oacc-fortran/fortran-torture_execute_math.f90
> new file mode 100644
> index 00000000000..1b2ac440762
> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.oacc-fortran/fortran-torture_execute_math.f90
> @@ -0,0 +1,5 @@
> +! { dg-do run }
> +!TODO { dg-prune-output {using 'vector_length \(32\)', ignoring 1} }
> +! { dg-additional-options -foffload-options=-lm }
> +
> +include '../../../gcc/testsuite/gfortran.fortran-torture/execute/math.f90'
> -- 2.34.1
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955