Message ID | fa0c183a-3479-1709-2fee-f5e76b62b634@mentor.com |
---|---|
State | New |
Headers | show |
Series | [Fortran] Actually permit OpenMP's 'target simd' | expand |
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 😉
fortran/ * parse.c (parse_executable): Add missing ST_OMP_TARGET_SIMD. testsuite/ * gfortran.dg/gomp/target-simd.f90: New. diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c index 03fc716dbf5..15f6bf2937c 100644 --- a/gcc/fortran/parse.c +++ b/gcc/fortran/parse.c @@ -5534,6 +5534,7 @@ parse_executable (gfc_statement st) case ST_OMP_SIMD: case ST_OMP_TARGET_PARALLEL_DO: case ST_OMP_TARGET_PARALLEL_DO_SIMD: + case ST_OMP_TARGET_SIMD: case ST_OMP_TARGET_TEAMS_DISTRIBUTE: case ST_OMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_DO: case ST_OMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_DO_SIMD: diff --git a/gcc/testsuite/gfortran.dg/gomp/target-simd.f90 b/gcc/testsuite/gfortran.dg/gomp/target-simd.f90 new file mode 100644 index 00000000000..733420f4cc7 --- /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