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

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

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

Tobias Burnus-5
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.)


omp-target-simd.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [patch][Fortran] Actually permit OpenMP's 'target simd'

Bernhard Reutner-Fischer
On 8 October 2019 14:12:47 CEST, Jakub Jelinek <[hidden email]> 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,
Reply | Threaded
Open this post in threaded view
|

Re: [patch][Fortran] Actually permit OpenMP's 'target simd'

Thomas Schwinge-8
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 <[hidden email]> wrote:
> On 8 October 2019 14:12:47 CEST, Jakub Jelinek <[hidden email]> 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

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

Re: [patch][Fortran] Actually permit OpenMP's 'target simd'

Bernhard Reutner-Fischer
In reply to this post by Tobias Burnus-5
On 14 October 2019 10:50:55 CEST, Jakub Jelinek <[hidden email]> 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.
Reply | Threaded
Open this post in threaded view
|

Re: [patch][Fortran] Actually permit OpenMP's 'target simd'

Thomas König
Hi Bernhard,

> Am 14.10.2019 um 11:58 schrieb Bernhard Reutner-Fischer <[hidden email]>:
>
> On 14 October 2019 10:50:55 CEST, Jakub Jelinek <[hidden email]> 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 😉
Reply | Threaded
Open this post in threaded view
|

Re: [patch][Fortran] Actually permit OpenMP's 'target simd'

Bernhard Reutner-Fischer
On Mon, 14 Oct 2019 14:18:58 +0200
Thomas König <[hidden email]> wrote:

> Hi Bernhard,
>
> > Am 14.10.2019 um 11:58 schrieb Bernhard Reutner-Fischer <[hidden email]>:
> >
> > On 14 October 2019 10:50:55 CEST, Jakub Jelinek <[hidden email]> 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.

I didn't look but i'd have have expected to know the [min,max) range of
the allocate "index" which we would then use the check if access is
within bounds, for starters.
>
> Anyway, I have just submitted PR 92087 for this. Feel free to comment or, even better, participate 😉

I'd love to but i'm completely swamped at work.

Thanks for filing the bug as a reminder though!
cheers,