[Patch][Fortran] OpenMP – libgomp/testsuite – use 'stop' and 'dg-do run'

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[Patch][Fortran] OpenMP – libgomp/testsuite – use 'stop' and 'dg-do run'

Tobias Burnus-3
This patch is about: libgomp/testsuite/libgomp.fortran/, only

The two test cases I added recently use 'call abort()', which is
nowadays frowned on as that's a ventor extension. Hence, I change it to 
'stop'.

Additionally, the 'fortran.exp' in the directory states: "For Fortran
we're doing torture testing, as Fortran has far more tests with arrays
etc. that testing just -O0 or -O2 is insufficient, that is typically not
the case for C/C++."

The torture testing is only done if there is a "dg-do run"; without
dg-do, it also does run, but only with a single compile flag setting.

I have only selectively added it; I think 'dg-do run' does not make
sense for:
* condinc*.f – only do uses '!$ include'
* omp_cond*.f* – only tests '!$' code, including comments.
Hence, I excluded those and only changed the others. (However, one can
still argue about the remaining ones – such as 'omp_hello.f' or tabs*.f*.)

OK for the trunk? Add/remove for 'dg-do run' from additional test cases?

Tobias


openmp-stop-run.diff (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patch][Fortran] OpenMP – libgomp/testsuite – use 'stop' and 'dg-do run'

Steve Kargl
On Fri, Oct 25, 2019 at 06:17:26PM +0200, Tobias Burnus wrote:

> This patch is about: libgomp/testsuite/libgomp.fortran/, only
>
> The two test cases I added recently use 'call abort()', which is
> nowadays frowned on as that's a ventor extension. Hence, I change it to 
> 'stop'.
>
> Additionally, the 'fortran.exp' in the directory states: "For Fortran
> we're doing torture testing, as Fortran has far more tests with arrays
> etc. that testing just -O0 or -O2 is insufficient, that is typically not
> the case for C/C++."
>
> The torture testing is only done if there is a "dg-do run"; without
> dg-do, it also does run, but only with a single compile flag setting.
>
> I have only selectively added it; I think 'dg-do run' does not make
> sense for:
> * condinc*.f – only do uses '!$ include'
> * omp_cond*.f* – only tests '!$' code, including comments.
> Hence, I excluded those and only changed the others. (However, one can
> still argue about the remaining ones – such as 'omp_hello.f' or tabs*.f*.)
>
> OK for the trunk? Add/remove for 'dg-do run' from additional test cases?
>
> Tobias
>

I think this patch and the openacc patch are fine.

With openmp and openacc, I usually defer to Jakub, Thomas,
and Cesar for their expertise with these standards.  I do,
however, recognize that everyone has limited amounts of time.
If you have openmp/openacc patches I can cast an eye over
them for formatting issues; otherwise, I'll leave it to your
discretion on what a reasonable review timeout is prior to
committing a change.

Reply | Threaded
Open this post in threaded view
|

Re: [Patch][Fortran] OpenMP – libgomp/testsuite – use 'stop' and 'dg-do run'

Thomas Schwinge-8
In reply to this post by Tobias Burnus-3
Hi Tobias!

On 2019-10-25T18:17:26+0200, Tobias Burnus <[hidden email]> wrote:

> This patch is about: libgomp/testsuite/libgomp.fortran/, only
>
> The two test cases I added recently use 'call abort()', which is
> nowadays frowned on as that's a ventor extension. Hence, I change it to 
> 'stop'.
>
> Additionally, the 'fortran.exp' in the directory states: "For Fortran
> we're doing torture testing, as Fortran has far more tests with arrays
> etc. that testing just -O0 or -O2 is insufficient, that is typically not
> the case for C/C++."
>
> The torture testing is only done if there is a "dg-do run"; without
> dg-do, it also does run, but only with a single compile flag setting.
(It was me who suggested that task.)


> I have only selectively added it; I think 'dg-do run' does not make
> sense for:
> * condinc*.f – only do uses '!$ include'
> * omp_cond*.f* – only tests '!$' code, including comments.
> Hence, I excluded those and only changed the others. (However, one can
> still argue about the remaining ones – such as 'omp_hello.f' or tabs*.f*.)

But why shouldn't these still be torture tested?

Anyway -- as usual ;-) -- I'd prefer if any such rationale ("not doing
torture testing because [...]") was put into the respective testsuite
files, so that it's obvious to the next person reading that file.


> --- a/libgomp/testsuite/libgomp.fortran/strassen.f90
> +++ b/libgomp/testsuite/libgomp.fortran/strassen.f90
> @@ -0,0 +1 @@
> +! { dg-do run }
|  ! { dg-options "-O2" }

That's not a useful combination, is it?  (Just noticed this one here;
didn't verify all files suggested to be changed.)

Not sure into which direction this should be fixed: continue to just test
'-O2', or remove the '-O2' override.


> --- a/libgomp/testsuite/libgomp.fortran/use_device_addr-1.f90
> +++ b/libgomp/testsuite/libgomp.fortran/use_device_addr-1.f90
> @@ -0,0 +1 @@
> +! { dg-do run }

> --- a/libgomp/testsuite/libgomp.fortran/use_device_addr-2.f90
> +++ b/libgomp/testsuite/libgomp.fortran/use_device_addr-2.f90
> @@ -0,0 +1 @@
> +! { dg-do run }

On powerpc64le-unknown-linux-gnu without offloading, I'm seeing (only)
the '-O0' execution tests FAIL for both these, with 'STOP 1' message.


Grüße
 Thomas

signature.asc (671 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patch][Fortran] OpenMP – libgomp/testsuite – use 'stop' and 'dg-do run'

Jakub Jelinek
In reply to this post by Tobias Burnus-3
Hi!

Generally, we need to consider the aim to test things sufficiently vs. the
already way too long test time of libgomp.fortran testsuite which is not
parallelized itself and often takes the longest time in the testsuite.

The primary reason to do any option cycling in libgomp.fortran are tests
involving variable length types where it matters matters quite a lot if it
is -O0 or -O2 or -O3, e.g. with -O0 some temporaries need to be privatized
when with optimization they don't, etc., though perhaps we could trim the
list and just test
-O0
-O2
-O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions -g
and not the other ones.

On Fri, Oct 25, 2019 at 06:17:26PM +0200, Tobias Burnus wrote:
> diff --git a/libgomp/testsuite/libgomp.fortran/display-affinity-1.f90 b/libgomp/testsuite/libgomp.fortran/display-affinity-1.f90
> index 4811b6f801b..4e732e5427f 100644
> --- a/libgomp/testsuite/libgomp.fortran/display-affinity-1.f90
> +++ b/libgomp/testsuite/libgomp.fortran/display-affinity-1.f90
> @@ -0,0 +1 @@
> +! { dg-do run }

In this testcase I don't see the value to cycle through, it is a test for
library routines.

> +++ b/libgomp/testsuite/libgomp.fortran/lastprivate1.f90
> +++ b/libgomp/testsuite/libgomp.fortran/lastprivate2.f90
> +++ b/libgomp/testsuite/libgomp.fortran/nestedfn4.f90

These are ok.

> --- a/libgomp/testsuite/libgomp.fortran/omp_hello.f
> +++ b/libgomp/testsuite/libgomp.fortran/omp_hello.f
> @@ -0,0 +1 @@
> +C { dg-do run }

Not worth it.

> --- a/libgomp/testsuite/libgomp.fortran/omp_orphan.f
> +++ b/libgomp/testsuite/libgomp.fortran/omp_orphan.f
> @@ -0,0 +1 @@
> +C { dg-do run }
> --- a/libgomp/testsuite/libgomp.fortran/omp_reduction.f
> +++ b/libgomp/testsuite/libgomp.fortran/omp_reduction.f
> @@ -0,0 +1 @@
> +C { dg-do run }
> --- a/libgomp/testsuite/libgomp.fortran/omp_workshare1.f
> +++ b/libgomp/testsuite/libgomp.fortran/omp_workshare1.f
> @@ -0,0 +1 @@
> +C { dg-do run }
> --- a/libgomp/testsuite/libgomp.fortran/omp_workshare2.f
> +++ b/libgomp/testsuite/libgomp.fortran/omp_workshare2.f
> @@ -0,0 +1 @@
> +C { dg-do run }

Dunno, maybe, though not clear advantages of doing so.

> +++ b/libgomp/testsuite/libgomp.fortran/pr25219.f90
> +++ b/libgomp/testsuite/libgomp.fortran/pr28390.f
> +++ b/libgomp/testsuite/libgomp.fortran/pr35130.f90
> +++ b/libgomp/testsuite/libgomp.fortran/pr90779.f90

Ok.

> --- a/libgomp/testsuite/libgomp.fortran/strassen.f90
> +++ b/libgomp/testsuite/libgomp.fortran/strassen.f90
> @@ -0,0 +1 @@
> +! { dg-do run }

Not ok.  This is quite an expensive test and it is intentionally run just at
-O2.

> --- a/libgomp/testsuite/libgomp.fortran/target-simd.f90
> +++ b/libgomp/testsuite/libgomp.fortran/target-simd.f90
> @@ -17 +17 @@ program test
> -  if (any (b - 5.0 *a > 10.0*epsilon(a))) call abort()
> +  if (any (b - 5.0 *a > 10.0*epsilon(a))) stop 1
> @@ -25 +25 @@ program test
> -  if (any (b - 2.0 *a > 10.0*epsilon(a))) call abort()
> +  if (any (b - 2.0 *a > 10.0*epsilon(a))) stop 2

The call abort -> stop changes are ok.  I'm not really happy
about the uppercase STOP in all the libgomp.fortran tests that are
written completely in lowercase except these stops, but didn't get
around to changing it yet.

> +++ b/libgomp/testsuite/libgomp.fortran/task2.f90
> +++ b/libgomp/testsuite/libgomp.fortran/taskgroup1.f90
> +++ b/libgomp/testsuite/libgomp.fortran/taskloop1.f90
> +++ b/libgomp/testsuite/libgomp.fortran/use_device_addr-1.f90
> +++ b/libgomp/testsuite/libgomp.fortran/use_device_addr-2.f90
> +++ b/libgomp/testsuite/libgomp.fortran/workshare1.f90
> +++ b/libgomp/testsuite/libgomp.fortran/workshare2.f90

Ok.

        Jakub