[Fortran] Actually permit OpenMP's 'target simd'

Seemingly, 'target simd' was forgotten – which yielded the error:
"Unexpected !$OMP TARGET SIMD statement"

OK for the trunk?

Tobias

PS: The test case should also work as 'dg-do run' test, if it makes more 
sense. (Only tested on a system w/o offloading, but I would test it with 
nvptx before committing it.)

On Tue, Oct 08, 2019 at 02:04:17PM +0200, Tobias Burnus wrote:
> Seemingly, 'target simd' was forgotten – which yielded the error:
> "Unexpected !$OMP TARGET SIMD statement"
> 
> OK for the trunk?
> 
> Tobias
> 
> PS: The test case should also work as 'dg-do run' test, if it makes more
> sense. (Only tested on a system w/o offloading, but I would test it with
> nvptx before committing it.)

gfortran.dg/gomp/ shouldn't contain dg-do link or dg-do run tests,
those should be in libgomp/testsuite/libgomp.fortran/
but then there is no point duplicating the test in gfortran.dg/gomp/.

> 

> 	fortran/
> 	* parse.c (parse_executable): Add missing ST_OMP_TARGET_SIMD.
> 
> 	testsuite/
> 	* gfortran.dg/gomp/target-simd.f90: New.

Ok, with moving the test to libgomp.fortran/ and removing the dg-do compile
line, or without.

> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/gomp/target-simd.f90
> @@ -0,0 +1,26 @@
> +! { dg-do compile }
> +
> +program test
> +  implicit none
> +  real, allocatable :: a(:), b(:)
> +  integer :: i
> +
> +  a = [(i, i = 1, 100)]
> +  allocate(b, mold=a)
> +  b = 0
> +
> +  !$omp target simd map(to:a) map(from:b)
> +  do i = 0, size(a)
> +    b(i) = 5.0 * a(i)
> +  end do
> +
> +  if (any (b - 5.0 *a > 10.0*epsilon(a))) call abort()
> +
> +  !$omp target simd map(to:a) map(from:b)
> +  do i = 0, size(a)
> +    b(i) = 2.0 * a(i)
> +  end do
> +  !$omp end target simd
> +
> +  if (any (b - 2.0 *a > 10.0*epsilon(a))) call abort()
> +end program test


	Jakub

Trunk (r276698): I have tested it with nvptx, it worked; hence, I moved 
it to libgomp as run-time test.

OG9: I have also committed the patch to the OG9 / openacc-gcc-9 branch. 
(54fbada7d4d38e420efb5a10d39e03b02533b1e7)

Thanks,

Tobias

On 10/8/19 2:12 PM, Jakub Jelinek wrote:
> On Tue, Oct 08, 2019 at 02:04:17PM +0200, Tobias Burnus wrote:
>> Seemingly, 'target simd' was forgotten – which yielded the error:
>> "Unexpected !$OMP TARGET SIMD statement"
>>
>> OK for the trunk?
>>
>> Tobias
>>
>> PS: The test case should also work as 'dg-do run' test, if it makes more
>> sense. (Only tested on a system w/o offloading, but I would test it with
>> nvptx before committing it.)
> gfortran.dg/gomp/ shouldn't contain dg-do link or dg-do run tests,
> those should be in libgomp/testsuite/libgomp.fortran/
> but then there is no point duplicating the test in gfortran.dg/gomp/.
>
>> 	fortran/
>> 	* parse.c (parse_executable): Add missing ST_OMP_TARGET_SIMD.
>>
>> 	testsuite/
>> 	* gfortran.dg/gomp/target-simd.f90: New.
> Ok, with moving the test to libgomp.fortran/ and removing the dg-do compile
> line, or without.
>
>> --- /dev/null
>> +++ b/gcc/testsuite/gfortran.dg/gomp/target-simd.f90
>> @@ -0,0 +1,26 @@
>> +! { dg-do compile }
>> +
>> +program test
>> +  implicit none
>> +  real, allocatable :: a(:), b(:)
>> +  integer :: i
>> +
>> +  a = [(i, i = 1, 100)]
>> +  allocate(b, mold=a)
>> +  b = 0
>> +
>> +  !$omp target simd map(to:a) map(from:b)
>> +  do i = 0, size(a)
>> +    b(i) = 5.0 * a(i)
>> +  end do
>> +
>> +  if (any (b - 5.0 *a > 10.0*epsilon(a))) call abort()
>> +
>> +  !$omp target simd map(to:a) map(from:b)
>> +  do i = 0, size(a)
>> +    b(i) = 2.0 * a(i)
>> +  end do
>> +  !$omp end target simd
>> +
>> +  if (any (b - 2.0 *a > 10.0*epsilon(a))) call abort()
>> +end program test
>
> 	Jakub

On 8 October 2019 14:12:47 CEST, Jakub Jelinek <jakub@redhat.com> wrote:
>On Tue, Oct 08, 2019 at 02:04:17PM +0200, Tobias Burnus wrote:
>> Seemingly, 'target simd' was forgotten – which yielded the error:
>> "Unexpected !$OMP TARGET SIMD statement"
>> 
>> OK for the trunk?


>Ok, with moving the test to libgomp.fortran/ and removing the dg-do
>compile
>line, or without.

What is the reasoning to call the nonstandard abort instead if the usual STOP N?

thanks,

Hi!

Tobias, if that's not inconvenient for you for any reson (which?), would
you please adopt the habit of using the same text for a commit's first
line in the log message (r276698: "Fortran - fix OpenMP 'target simd'),
and the email's "Subject" line ("[Fortran] Actually permit OpenMP's
'target simd'").  That's less typing for you ;-) -- and more importantly,
makes it easy to find one from the other.


On 2019-10-09T08:04:29+0200, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
> On 8 October 2019 14:12:47 CEST, Jakub Jelinek <jakub@redhat.com> wrote:
>>On Tue, Oct 08, 2019 at 02:04:17PM +0200, Tobias Burnus wrote:
>>> Seemingly, 'target simd' was forgotten – which yielded the error:
>>> "Unexpected !$OMP TARGET SIMD statement"

Shouldn't this get a PR opened, and the fix eventually backported also to
release branches?

(Note that at least for OpenACC changes, we're way, way behind on such
backports...)


> What is the reasoning to call the nonstandard abort instead if the usual STOP N?

(ACK, that works.)


Grüße
 Thomas

On Wed, Oct 09, 2019 at 11:46:37AM +0200, Thomas Schwinge wrote:
> On 2019-10-09T08:04:29+0200, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
> > On 8 October 2019 14:12:47 CEST, Jakub Jelinek <jakub@redhat.com> wrote:
> >>On Tue, Oct 08, 2019 at 02:04:17PM +0200, Tobias Burnus wrote:
> >>> Seemingly, 'target simd' was forgotten – which yielded the error:
> >>> "Unexpected !$OMP TARGET SIMD statement"
> 
> Shouldn't this get a PR opened, and the fix eventually backported also to
> release branches?

Not sure about that.  target simd is an OpenMP 4.5 feature, and we've never
claimed OpenMP 4.5 is fully implemented in GCC 9 or earlier, so I think it
is fine if it is just one of several missing 4.5 features implemented in
GCC 10.  If we've accepted target simd, fine, it would be a bug fix, but my
understanding is that because of this bug we've not really supported it.

The actual problem is that the
https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01125.html
patch wasn't accompanied by sufficient test coverage, some of it has been later
covered, like taskloop/taskloop simd, but most of it has not, because I
eventually got busy with non-OpenMP stuff that year.
In particular, we don't have any testsuite coverage for
target enter data
target exit data
target parallel
target parallel do
target parallel do simd
(for target simd we now have some) just from the above patch.  The plan was
basically to port the C/C++ testcases from the OpenMP 4.5 submission, so
https://gcc.gnu.org/ml/gcc-patches/2015-10/msg01291.html
https://gcc.gnu.org/ml/gcc-patches/2015-11/msg00477.html
https://gcc.gnu.org/ml/gcc-patches/2015-11/msg01835.html
to Fortran, but that never happened, like finishing the 4.5 support.

	Jakub

On Tue, Oct 08, 2019 at 02:04:17PM +0200, Tobias Burnus wrote:
> 	testsuite/
> 	* gfortran.dg/gomp/target-simd.f90: New.

John reported in bugzilla the testcase fails on hppa with an error that
looks like heap corruption, and indeed, running the test under valgrind
fails on x86_64-linux too, the allocatable arrays are 1..100, but the loops
were reading and writing 0..100.  Fixed thusly, tested on x86_64-linux,
committed to trunk.

2019-10-14  Jakub Jelinek  <jakub@redhat.com>

	PR libgomp/92081
	* testsuite/libgomp.fortran/target-simd.f90: Iterate from 1 rather
	than 0.

--- libgomp/testsuite/libgomp.fortran/target-simd.f90.jj	2019-10-09 09:32:06.826126101 +0200
+++ libgomp/testsuite/libgomp.fortran/target-simd.f90	2019-10-14 10:45:04.459650503 +0200
@@ -10,14 +10,14 @@ program test
   b = 0
 
   !$omp target simd map(to:a) map(from:b)
-  do i = 0, size(a)
+  do i = 1, size(a)
     b(i) = 5.0 * a(i)
   end do
 
   if (any (b - 5.0 *a > 10.0*epsilon(a))) call abort()
 
   !$omp target simd map(to:a) map(from:b)
-  do i = 0, size(a)
+  do i = 1, size(a)
     b(i) = 2.0 * a(i)
   end do
   !$omp end target simd


	Jakub

On 14 October 2019 10:50:55 CEST, Jakub Jelinek <jakub@redhat.com> wrote:
>On Tue, Oct 08, 2019 at 02:04:17PM +0200, Tobias Burnus wrote:
>> 	testsuite/
>> 	* gfortran.dg/gomp/target-simd.f90: New.
>
>John reported in bugzilla the testcase fails on hppa with an error that
>looks like heap corruption, and indeed, running the test under valgrind
>fails on x86_64-linux too, the allocatable arrays are 1..100, but the
>loops
>were reading and writing 0..100.  Fixed thusly, tested on x86_64-linux,
>committed to trunk.

Which suggests that we ought to diagnose this, obviously.

Hi Bernhard,

> Am 14.10.2019 um 11:58 schrieb Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>:
> 
> On 14 October 2019 10:50:55 CEST, Jakub Jelinek <jakub@redhat.com> wrote:
>>> On Tue, Oct 08, 2019 at 02:04:17PM +0200, Tobias Burnus wrote:
>>>    testsuite/
>>>    * gfortran.dg/gomp/target-simd.f90: New.
>> 
>> John Reporte in bugzilla the testcase fails on hppa with an error that
>> looks like heap corruption, and indeed, running the test under valgrind
>> fails on x86_64-linux too, the allocatable arrays are 1..100, but the
>> loops
>> were reading and writing 0..100.  Fixed thusly, tested on x86_64-linux,
>> committed to trunk.
> 
> Which suggests that we ought to diagnose this, obviously.

We already do for arrays with static bounds.

Allocatable - not straightforward. This would need value propagation from the ALLOCATE statement, which in turn would suggest splitting up the FE represenation into basic blocks, which of course can be higher level than what the middle end can do after scalarization.

Anyway, I have just submitted PR 92087 for this. Feel free to comment or, even better, participate 😉