Re: [Patch, Fortran] PR25829: Asynchronous I/O (v2)

Re: [Patch, Fortran] PR25829: Asynchronous I/O (v2)

[Dominique d'Humières]

Hi Nicolas,

I have applied your patch on top of revision r261130 on x86_64-apple-darwin17 (SSD with APFS file system).

The only remaining failure on my own tests is for the test (pr35840)

write(10,*, asynchronous="Y"//"E"//trim("S  "))
end

giving at run time

At line 1 of file pr35840.f90 (unit = 10, file = 'fort.10')
Fortran runtime error: ASYNCHRONOUS transfer without ASYHCRONOUS='YES' in OPEN

I also see two regressions

FAIL: gfortran.dg/f2003_inquire_1.f03   -O1  execution test

only with -m32 and -O1 (STOP 5), and

FAIL: gfortran.dg/f2003_io_1.f03   -O*

with both -m32 and -m64 (STOP 1).

The is also typos for the added tests

s/libgfomp/libgomp/

Why do the tests start at asynchronous_6.f90?

"Treat asynchronous variables the same as volatile, for now." could probably simplified as
"Treat asynchronous variables as volatile, for now."

I also wonder if

+    wrap_scalar_transfer (dtp, BT_INTEGER, p, kind, kind, 1);

is correct without a cast to size_t for the last two arguments (and for the last argument in other instances). Note that I am C challenged, so forgive the question if it is stupid.

Thanks for the nice work.

Dominique

Re: [Patch, Fortran] PR25829: Asynchronous I/O (v2)

[Rainer Orth]

Hi Dominique, Nicolas,

> I have applied your patch on top of revision r261130 on
> x86_64-apple-darwin17 (SSD with APFS file system).

I've tried it on i386-pc-solaris2.11 and sparc-sun-solaris2.11.

> I also see two regressions
>
> FAIL: gfortran.dg/f2003_inquire_1.f03   -O1  execution test
>
> only with -m32 and -O1 (STOP 5), and

It fails for me at -O[0s] (i386) resp. -O[01] (sparc), 64-bit only.

> FAIL: gfortran.dg/f2003_io_1.f03   -O*
>
> with both -m32 and -m64 (STOP 1).

Same here: FAILs at -O[0-3s] for both 32 and 64-bit.

> The is also typos for the added tests
>
> s/libgfomp/libgomp/
>
> Why do the tests start at asynchronous_6.f90?

... and asynchronous_9.f90 is missing from the ChangeLog, which
..._7.f90 is missing from the sequence.

Besides, I see

+FAIL: libgomp.fortran/asynchronous_6.f90   -O1  execution test

STOP 2

32-bit i386 only.

+FAIL: libgomp.fortran/asynchronous_9.f90   -O  execution test

32 and 64-bit i386 and sparc, no error message.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [Patch, Fortran] PR25829: Asynchronous I/O (v2)

[Nicolas Koenig]

Hi Dominique and Rainer,

First of all thanks for testing!

> Hi Dominique, Nicolas,
> 
>> I have applied your patch on top of revision r261130 on
>> x86_64-apple-darwin17 (SSD with APFS file system).
> 
> I've tried it on i386-pc-solaris2.11 and sparc-sun-solaris2.11.
> 
>> I also see two regressions
>>
>> FAIL: gfortran.dg/f2003_inquire_1.f03   -O1  execution test
>>
>> only with -m32 and -O1 (STOP 5), and
> 
> It fails for me at -O[0s] (i386) resp. -O[01] (sparc), 64-bit only.

This seems to be a bug in the test suite. It tries to find out whether 
an id is pending that is never initialized.

> 
>> FAIL: gfortran.dg/f2003_io_1.f03   -O*
>>
>> with both -m32 and -m64 (STOP 1).
> 
> Same here: FAILs at -O[0-3s] for both 32 and 64-bit.

And another bug in the test suite. This time the wait after the read is 
missing.

> 
>> The is also typos for the added tests
>>
>> s/libgfomp/libgomp/

Will fix.

>>
>> Why do the tests start at asynchronous_6.f90?

Because they were originally intended for the gfortran test suite, but I 
couldn't run it there because of libpthread. I will change the numbering 
scheme.

> 
> ... and asynchronous_9.f90 is missing from the ChangeLog, which
> ..._7.f90 is missing from the sequence.
> 

asynchronous_7.f90 is a test for an error, but dg-shouldfail is not 
working in libgomp. Dominique is looking into this.

> Besides, I see
> 
> +FAIL: libgomp.fortran/asynchronous_6.f90   -O1  execution test
> 
> STOP 2
> 
> 32-bit i386 only.
> 

I have trouble replicating this bug even with -m32. Could you get some 
more debugging info for the test on your machine?

> +FAIL: libgomp.fortran/asynchronous_9.f90   -O  execution test
> 
> 32 and 64-bit i386 and sparc, no error message.
> 

This file wasn't supposed to be a test case, that's why it is not in the 
ChangeLog. It is a benchmark program, so it takes some time. Maybe a 
time out? Could you maybe try running it outside the test suite?

> 	Rainer
> 

Dominique wrote:
 > "Treat asynchronous variables the same as volatile, for now." could 
probably simplified as
 > "Treat asynchronous variables as volatile, for now."

Will do.

 >
 > I also wonder if
 >
 > +    wrap_scalar_transfer (dtp, BT_INTEGER, p, kind, kind, 1);
 >
 > is correct without a cast to size_t for the last two arguments (and 
for the last argument in other instances). Note that I am C challenged, 
so forgive the question if it is stupid.

It atomatically casts based on the type information in the prototype in 
io.h.

 >
 > Thanks for the nice work.

With pleasure! :)

 >
 > Dominique

Re: [Patch, Fortran] PR25829: Asynchronous I/O (v2)

[Rainer Orth]

Hi Nicolas,

> Because they were originally intended for the gfortran test suite, but I
> couldn't run it there because of libpthread. I will change the numbering
> scheme.

the way that libpthread dependency is currently handled seems weird to
me: right now it is only dragged in via -fopenmp, although libgomp isn't
otherwise used AFAICS.  Is this really supposed to work this way?  And
what about targets that don't have pthreads?  Isn't <gthr.h> supposed to
abstract away from the details of the underlying threading library?

If async io needs libpthread (or whatever else), the gfortran driver
needs to drag that in, maybe under the control of some option, instead
of relying on libgomp that happens to use it already.  Maybe have a look
at what libobjc and libstdc++ (the other consumers of gthr.h) do here.

>> ... and asynchronous_9.f90 is missing from the ChangeLog, which
>> ..._7.f90 is missing from the sequence.
>
> asynchronous_7.f90 is a test for an error, but dg-shouldfail is not working
> in libgomp. Dominique is looking into this.

Weird: it's already being used in many places in libgomp.oacc*.

>> Besides, I see
>>
>> +FAIL: libgomp.fortran/asynchronous_6.f90   -O1  execution test
>>
>> STOP 2
>>
>> 32-bit i386 only.
>>
>
> I have trouble replicating this bug even with -m32. Could you get some more
> debugging info for the test on your machine?

I'll try.  It seems pretty random, though: I've just run another test
where only the 32-bit i386

FAIL: libgomp.fortran/asynchronous_6.f90   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test

FAILs.  Maybe different runs of the test with different options (under
make -j<N> testing) tread on each other's toes?

>> +FAIL: libgomp.fortran/asynchronous_9.f90   -O  execution test
>>
>> 32 and 64-bit i386 and sparc, no error message.
>>
>
> This file wasn't supposed to be a test case, that's why it is not in the
> ChangeLog. It is a benchmark program, so it takes some time. Maybe a time
> out? Could you maybe try running it outside the test suite?

Sure: when rebuilt with -g3, I get

Thread 2 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1 (LWP 1)]
MAIN__ ()
    at /vol/gcc/src/hg/trunk/solaris/libgomp/testsuite/libgomp.fortran/asynchronous_9.f90:7
7         call random_number(a)
(gdb) where
#0  MAIN__ ()
    at /vol/gcc/src/hg/trunk/solaris/libgomp/testsuite/libgomp.fortran/asynchronous_9.f90:7
(gdb) display/i $pc
1: x/i $pc
=> 0x8051679 <MAIN__+12>:       movl   $0x0,-0x7270f60(%ebp)
(gdb) p/x $ebp
$1 = 0xfeffda78

That address (FEFFDA78-7270F60 = F7D8CB18) isn't even mapped:

9698:   /var/gcc/gcc-9.0.0-20180604/11.4-gcc/i386-pc-solaris2.11/libgomp/tests
 Address Kbytes  RSS Anon Lock Mode     Mapped File
08050000     40   40    -    - r-x----  [ text ] asynchronous_9.exe
08069000      8    8    8    - rwx----  [ data ] asynchronous_9.exe
0806B000      8    8    8    - rwx----  [ heap ]
FDB90000    352  288    -    - r-x----  [ text ] libquadmath.so.0.0.0
FDBF7000      4    4    4    - rwx----  [ data ] libquadmath.so.0.0.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [Patch, Fortran] PR25829: Asynchronous I/O (v2)

[Jerry DeLisle]

On 06/05/2018 06:58 AM, Rainer Orth wrote:
> Hi Nicolas,
> 
>> Because they were originally intended for the gfortran test suite, but I
>> couldn't run it there because of libpthread. I will change the numbering
>> scheme.
> 
> the way that libpthread dependency is currently handled seems weird to
> me: right now it is only dragged in via -fopenmp, although libgomp isn't
> otherwise used AFAICS.  Is this really supposed to work this way?  And
> what about targets that don't have pthreads?  Isn't <gthr.h> supposed to
> abstract away from the details of the underlying threading library?

 From my perspective, since async is a feature of the language it should 
not require any special flags, just link to pthread always.

If a user does not use it, it will most likely be optimized out.

Jerry

Re: [Patch, Fortran] PR25829: Asynchronous I/O (v2)

[Janne Blomqvist]

On Wed, Jun 6, 2018 at 3:43 AM, Jerry DeLisle <jvdelisle@charter.net> wrote:

> On 06/05/2018 06:58 AM, Rainer Orth wrote:
>
>> Hi Nicolas,
>>
>> Because they were originally intended for the gfortran test suite, but I
>>> couldn't run it there because of libpthread. I will change the numbering
>>> scheme.
>>>
>>
>> the way that libpthread dependency is currently handled seems weird to
>> me: right now it is only dragged in via -fopenmp, although libgomp isn't
>> otherwise used AFAICS.  Is this really supposed to work this way?  And
>> what about targets that don't have pthreads?  Isn't <gthr.h> supposed to
>> abstract away from the details of the underlying threading library?
>>
>
> From my perspective, since async is a feature of the language it should
> not require any special flags, just link to pthread always.
>
> If a user does not use it, it will most likely be optimized out.
>
> Jerry
>

This has the disadvantage that then the library will always use pthread
stuff also for single-threaded programs, where we now use the stubbed-out
dummy implementations which are faster.



-- 
Janne Blomqvist

Re: [Patch, Fortran] PR25829: Asynchronous I/O (v2)

[Steve Kargl]

On Wed, Jun 06, 2018 at 08:34:57AM +0300, Janne Blomqvist wrote:
> On Wed, Jun 6, 2018 at 3:43 AM, Jerry DeLisle <jvdelisle@charter.net> wrote:
> 
> > On 06/05/2018 06:58 AM, Rainer Orth wrote:
> >
> >> Hi Nicolas,
> >>
> >> Because they were originally intended for the gfortran test suite, but I
> >>> couldn't run it there because of libpthread. I will change the numbering
> >>> scheme.
> >>>
> >>
> >> the way that libpthread dependency is currently handled seems weird to
> >> me: right now it is only dragged in via -fopenmp, although libgomp isn't
> >> otherwise used AFAICS.  Is this really supposed to work this way?  And
> >> what about targets that don't have pthreads?  Isn't <gthr.h> supposed to
> >> abstract away from the details of the underlying threading library?
> >>
> >
> > From my perspective, since async is a feature of the language it should
> > not require any special flags, just link to pthread always.
> >
> > If a user does not use it, it will most likely be optimized out.
> >
> 
> This has the disadvantage that then the library will always use pthread
> stuff also for single-threaded programs, where we now use the stubbed-out
> dummy implementations which are faster.
> 

I have not looked at the source code, but do have a question.
With -fopenmp, gfortran automatically adds -lpthread to
the command line.  Is it possible during the parsing 
phase to detect an async structure and on the fly add
-lpthread to the loader options?

I do note that if one has, say, 8 files and only one
file uses async IO, then during linking of the 8 *.o
files to make the final execute -lpthread must be added.
How doesn't gfortran communicate that to the loader?

PS: Nice work Nicolas.  We'll figure out the details.

-- 
Steve

Re: [Patch, Fortran] PR25829: Asynchronous I/O (v2)

[Jakub Jelinek]

On Tue, Jun 05, 2018 at 11:47:21PM -0700, Steve Kargl wrote:
> I have not looked at the source code, but do have a question.
> With -fopenmp, gfortran automatically adds -lpthread to
> the command line.  Is it possible during the parsing 

Even if it wouldn't, libgomp.so.1 depends on libpthread.so.0 and thus
it will be around.

> phase to detect an async structure and on the fly add
> -lpthread to the loader options?
> 
> I do note that if one has, say, 8 files and only one
> file uses async IO, then during linking of the 8 *.o
> files to make the final execute -lpthread must be added.
> How doesn't gfortran communicate that to the loader?

ELF doesn't a way to do this, you'd need to add a special linker plugin for
that.  But is that really necessary?
Simply let the users link with -lpthread or -pthread if they want real
asynchronous IO, or without it if they are ok with the gcc <= 8
behavior and document it.

	Jakub

Re: [Patch, Fortran] PR25829: Asynchronous I/O (v2)

[Rainer Orth]

Hi Jakub,

>> I do note that if one has, say, 8 files and only one
>> file uses async IO, then during linking of the 8 *.o
>> files to make the final execute -lpthread must be added.
>> How doesn't gfortran communicate that to the loader?
>
> ELF doesn't a way to do this, you'd need to add a special linker plugin for
> that.  But is that really necessary?
> Simply let the users link with -lpthread or -pthread if they want real
> asynchronous IO, or without it if they are ok with the gcc <= 8
> behavior and document it.

wouldn't it be better to use a special gfortran flag for that?
-pthread/-lpthread is an implementation detail on some platforms, but
others may not need it (Solaris has folded libpthread into libc and
there are no more single-threaded processes, for example) or need
something different for async io (Windows, ...).  IMO it would be better
to abstract this low-level detail away and let the driver DTRT, under
the control of a flag if need be.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [Patch, Fortran] PR25829: Asynchronous I/O (v2)

[Rainer Orth]

Hi Nicolas,

a few other nits:

* The current patch has a large number of GNU Coding Style violations,
  many catched by contrib/check_GNU_style.{sh,py}.  

* Others are partially pre-existing (additional blank before formal
  paramater name as in

+destroy_adv_cond (struct adv_cond * ac)

  and many many more.

* Many of the new functions lack comments.

* Hardcoded escape codes in async.h.

* Several indentation and line length errors, like

+  if (au->tail)
+	internal_error(NULL, "Trying to free nonempty unit");

  TAB instead of 4 spaces.

* Stuff like this in async.c at the very least needs a comment
  explaining what's going on:

+#ifndef GTHREAD_USE_WEAK
+#ifdef SUPPORTS_WEAK
+#define GTHREAD_USE_WEAK 1
+#endif
+#endif

  This should already be handled in gthr.h.  Why not use that?

+#define _GTHREAD_USE_COND_INIT_FUNC
+#include "../../libgcc/gthr-posix.h"

  Again: gthr.h would include that (via gthr-default.h) already.

As I stated, those are mostly nits, but should be fixed before this
patch goes in.

Thanks.
        Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [Patch, Fortran] PR25829: Asynchronous I/O (v2)

[Dominique d'Humières]

>>> FAIL: gfortran.dg/f2003_inquire_1.f03   -O1  execution test
> 
> This seems to be a bug in the test suite. It tries to find out whether an id is pending that is never initialized.
> 
>>> FAIL: gfortran.dg/f2003_io_1.f03   -O*
> 
> And another bug in the test suite. This time the wait after the read is missing.

Do you have any fix for them ?

> asynchronous_7.f90 is a test for an error, but dg-shouldfail is not working in libgomp. Dominique is looking into this.

I have this in my working tree

! { dg-do run }
program main
  integer :: i
  open (10,file="tst.dat")
  write (10,'(A4)') 'asdf'
  close(10)
  i = 234
  open(10,file="tst.dat", asynchronous="yes")
  read (10,'(I4)',asynchronous="yes") i
  wait(10)
end program main
! { dg-output "Fortran runtime error: Bad value during integer read" }
! { dg-final { remote_file build delete "tst.dat" } }

> 
>> Besides, I see
>> +FAIL: libgomp.fortran/asynchronous_6.f90   -O1  execution test
>> STOP 2
>> 32-bit i386 only.
> 
> I have trouble replicating this bug even with -m32. Could you get some more debugging info for the test on your machine?

I have copied the asynchronous tests from libgomp.fortran to gfortran.dg and ran both

make -k check-gfortran RUNTESTFLAGS="dg.exp=asynchronous_* --target_board=unix'{-m32,-m64}’"

in gcc and

make -k check RUNTESTFLAGS="fortran.exp=asynchronous_*.f90 --target_board=unix'{-m32,-m64}’"

in x86_64-apple-darwin17.6.0/libgomp/testsuite 10 times and I see ~10 failures in each cases (mostly STOP 2, but also STOP 4). I some cases I had to kill the process.

Note that these tests should probably protected by something such as

! { dg-require-effective-target pthread }

or

! { dg-require-effective-target tls }

Dominique