AsyncI/O patch committed

Post by Nicolas Koenig
Hello everyone,
I have committed the async I/O patch as r262978.
The test cases are in libgomp.fortran for now, maybe that can be changed
later.

It looks like this broke building libgfortran on spu, and presumably
any platform that uses gthr-simple instead of gthr-posix.

The problem is that io/asynch.h unconditionally uses a couple of
features that are not provided by gthr-simplex, in particular
__gthread_cond_t
and
__gthread_equal / __gthread_self

According to the documentation in gthr.h, the former is only available
if __GTHREAD_HAS_COND is defined, and the latter are only available if
__GTHREADS_CXX0X is defined. Neither is true for gthr-simple.h.

To fix the build error, either libgfortran should only use those features
conditionally on those defines, or else the gthr.h logic needs to be
changed and (stubs for) those features provided in gthr-simple.h as well.

Bye,
Ulrich

--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
***@de.ibm.com

Thomas Koenig

2018-07-26 20:54:16 UTC

Hi Ulrich,

Post by Ulrich Weigand
The problem is that io/asynch.h unconditionally uses a couple of
features that are not provided by gthr-simplex, in particular
__gthread_cond_t
and
__gthread_equal / __gthread_self
According to the documentation in gthr.h, the former is only available
if __GTHREAD_HAS_COND is defined, and the latter are only available if
__GTHREADS_CXX0X is defined. Neither is true for gthr-simple.h.

Thanks for the analysis, and the pointer to the macros.

Because the functionality depends on these features, it is best to
remove them if it is not present. So, here is a patch which does
just that.

This was reg-tested on Linux, which showed that the functionality is
still there. I tried bootstrapping on AIX on gcc119, but this failed
due to an unrelated issue (problem with compiling the inline
libraries).

Would it be possible to check if this restores bootstrap in the next
10 hours or so? If so, I would like to commit this. Otherwise, Nicolas
and I will not be able to fix this for a week or so, and it would be
best to revert the async I/O patch :-(

Regards

Thomas

2018-07-25 Thomas Koenig <***@gcc.gnu.org>

* io/async.h: Test for feature macros for __gthread_cond_t and
__gthread_equal. Define ASYNC_IO if both are present.
(SIGNAL): Define as no-op if ASYNC_IO is not defined.
(WAIT_SIGNAL_MUTEX): Likewise.
(REVOLE_SIGNAL): Likewise.
(transfer_args): Define as useless struct if ASYNC_IO is not
defined.
(adv_cond): Likewise.
(async_unit): Likewise.
* io/async.c (init_async_unit): If ASYNC_IO is not defined,
define alternate function which does nothing.
(enqueue_transfer): Likewise.
(enqueue_done_id): Likewise.
(enqueue_done): Likewise.
(enqueue_close): Likewise.
(enqueue_data_transfer_init): Likewise.
(collect_async_errors): Likewise.

(async_wait_id): Likewise.

(async_wait): Likewise.

(async_close): Likewise.

Thomas Koenig

2018-07-27 07:29:53 UTC

Post by Thomas Koenig
Hi Ulrich,

OK, this does not work.

We have found a method of checking on Linux, and this does not work.
We have also found a way of working in the next couple of days, so
expect an update in one or two days.

If that is too much time, feel free to revert the async patch
in the meantime.

Regards

Thomas

David Edelsohn

2018-07-27 12:25:46 UTC

Thomas,

Correct, the proposed patch does not fix the build failure on AIX.

Please see the information on the GCC Compile Farm wiki page for
instructions to bootstrap on gcc119.
https://gcc.gnu.org/wiki/CompileFarm#Projects_Ideas - at the bottom of
Project ideas.

Thanks, David

Post by Thomas Koenig
Hi Ulrich,

OK, this does not work.
We have found a method of checking on Linux, and this does not work.
We have also found a way of working in the next couple of days, so
expect an update in one or two days.
If that is too much time, feel free to revert the async patch
in the meantime.
Regards
Thomas

Andre Vieira (lists)

2018-07-31 08:45:52 UTC

Post by David Edelsohn
Thomas,
Correct, the proposed patch does not fix the build failure on AIX.
Please see the information on the GCC Compile Farm wiki page for
instructions to bootstrap on gcc119.
https://gcc.gnu.org/wiki/CompileFarm#Projects_Ideas - at the bottom of
Project ideas.
Thanks, David

Post by Thomas Koenig
Hi Ulrich,

OK, this does not work.
We have found a method of checking on Linux, and this does not work.
We have also found a way of working in the next couple of days, so
expect an update in one or two days.
If that is too much time, feel free to revert the async patch
in the meantime.
Regards
Thomas

Hi Thomas,

Sorry but I had to revert your patch as it has been breaking our
bare-metal builds for over 5 days now.

Comitted as r263082.

2018-07-31 Andre Vieira <***@arm.com>

Reverted 'AsyncI/O patch committed'
2018-07-25 Nicolas Koenig <***@gcc.gnu.org>
Thomas Koenig <***@gcc.gnu.org>

PR fortran/25829
* gfortran.texi: Add description of asynchronous I/O.
* trans-decl.c (gfc_finish_var_decl): Treat asynchronous variables
as volatile.
* trans-io.c (gfc_build_io_library_fndecls): Rename st_wait to
st_wait_async and change argument spec from ".X" to ".w".
(gfc_trans_wait): Pass ID argument via reference.

Reverted 'AsyncI/O patch committed'
2018-07-25 Nicolas Koenig <***@gcc.gnu.org>
Thomas Koenig <***@gcc.gnu.org>

PR fortran/25829
* gfortran.dg/f2003_inquire_1.f03: Add write statement.
* gfortran.dg/f2003_io_1.f03: Add wait statement.

Reverted 'AsyncI/O patch committed'
2018-07-25 Nicolas Koenig <***@gcc.gnu.org>
Thomas Koenig <***@gcc.gnu.org>

PR fortran/25829
* Makefile.am: Add async.c to gfor_io_src.
Add async.h to gfor_io_headers.
* Makefile.in: Regenerated.
* gfortran.map: Add _gfortran_st_wait_async.
* io/async.c: New file.
* io/async.h: New file.
* io/close.c: Include async.h.
(st_close): Call async_wait for an asynchronous unit.
* io/file_pos.c (st_backspace): Likewise.
(st_endfile): Likewise.
(st_rewind): Likewise.
(st_flush): Likewise.
* io/inquire.c: Add handling for asynchronous PENDING
and ID arguments.
* io/io.h (st_parameter_dt): Add async bit.
(st_parameter_wait): Correct.
(gfc_unit): Add au pointer.
(st_wait_async): Add prototype.
(transfer_array_inner): Likewise.
(st_write_done_worker): Likewise.
* io/open.c: Include async.h.
(new_unit): Initialize asynchronous unit.
* io/transfer.c (async_opt): New struct.
(wrap_scalar_transfer): New function.
(transfer_integer): Call wrap_scalar_transfer to do the work.
(transfer_real): Likewise.
(transfer_real_write): Likewise.
(transfer_character): Likewise.
(transfer_character_wide): Likewise.
(transfer_complex): Likewise.
(transfer_array_inner): New function.
(transfer_array): Call transfer_array_inner.
(transfer_derived): Call wrap_scalar_transfer.
(data_transfer_init): Check for asynchronous I/O.
Perform a wait operation on any pending asynchronous I/O
if the data transfer is synchronous. Copy PDT and enqueue
thread for data transfer.

Thomas König

2018-07-31 10:29:53 UTC

Post by Andre Vieira (lists)

Hi Andre,

sorry that it took so long for a fix. I am on holiday at the moment, so my hacking environment is somewhat restricted.

Expect a combined patch for async I/O which we expect to work for pthreads targets and which should not disturb others today or tomorrow.

I would appreciate testing on critical systems.

Regards

Thomas

Post by Thomas Koenig
Hi Ulrich,

OK, this does not work.
We have found a method of checking on Linux, and this does not work.
We have also found a way of working in the next couple of days, so
expect an update in one or two days.
If that is too much time, feel free to revert the async patch
in the meantime.
Regards
Thomas

Hi Thomas,
Sorry but I had to revert your patch as it has been breaking our
bare-metal builds for over 5 days now.
Comitted as r263082.
Reverted 'AsyncI/O patch committed'
PR fortran/25829
* gfortran.texi: Add description of asynchronous I/O.
* trans-decl.c (gfc_finish_var_decl): Treat asynchronous variables
as volatile.
* trans-io.c (gfc_build_io_library_fndecls): Rename st_wait to
st_wait_async and change argument spec from ".X" to ".w".
(gfc_trans_wait): Pass ID argument via reference.
Reverted 'AsyncI/O patch committed'
PR fortran/25829
* gfortran.dg/f2003_inquire_1.f03: Add write statement.
* gfortran.dg/f2003_io_1.f03: Add wait statement.
Reverted 'AsyncI/O patch committed'
PR fortran/25829
* Makefile.am: Add async.c to gfor_io_src.
Add async.h to gfor_io_headers.
* Makefile.in: Regenerated.
* gfortran.map: Add _gfortran_st_wait_async.
* io/async.c: New file.
* io/async.h: New file.
* io/close.c: Include async.h.
(st_close): Call async_wait for an asynchronous unit.
* io/file_pos.c (st_backspace): Likewise.
(st_endfile): Likewise.
(st_rewind): Likewise.
(st_flush): Likewise.
* io/inquire.c: Add handling for asynchronous PENDING
and ID arguments.
* io/io.h (st_parameter_dt): Add async bit.
(st_parameter_wait): Correct.
(gfc_unit): Add au pointer.
(st_wait_async): Add prototype.
(transfer_array_inner): Likewise.
(st_write_done_worker): Likewise.
* io/open.c: Include async.h.
(new_unit): Initialize asynchronous unit.
* io/transfer.c (async_opt): New struct.
(wrap_scalar_transfer): New function.
(transfer_integer): Call wrap_scalar_transfer to do the work.
(transfer_real): Likewise.
(transfer_real_write): Likewise.
(transfer_character): Likewise.
(transfer_character_wide): Likewise.
(transfer_complex): Likewise.
(transfer_array_inner): New function.
(transfer_array): Call transfer_array_inner.
(transfer_derived): Call wrap_scalar_transfer.
(data_transfer_init): Check for asynchronous I/O.
Perform a wait operation on any pending asynchronous I/O
if the data transfer is synchronous. Copy PDT and enqueue
thread for data transfer.

Nicolas Koenig

2018-08-02 11:31:36 UTC

Hello everyone,

Here is an updated version of the patch that hopefully fixes the compilation
problems by disabling async I/O if conditions are not supported by the target.

I would appreciate if people could test it on systems on which it failed
before. As for the array_constructor_8.f90 failure reported in the PR, why
it fails is beyond me, it doesn't even use I/O. Maybe/Probably something
unrelated?

Nicolas

2018-08-02 Nicolas Koenig <***@gcc.gnu.org>
Thomas Koenig <***@gcc.gnu.org>

PR fortran/25829
* gfortran.texi: Add description of asynchronous I/O.
* trans-decl.c (gfc_finish_var_decl): Treat asynchronous variables
as volatile.
* trans-io.c (gfc_build_io_library_fndecls): Rename st_wait to
st_wait_async and change argument spec from ".X" to ".w".
(gfc_trans_wait): Pass ID argument via reference.

2018-08-02 Nicolas Koenig <***@gcc.gnu.org>
Thomas Koenig <***@gcc.gnu.org>

PR fortran/25829
* gfortran.dg/f2003_inquire_1.f03: Add write statement.
* gfortran.dg/f2003_io_1.f03: Add wait statement.

2018-08-02 Nicolas Koenig <***@gcc.gnu.org>
Thomas Koenig <***@gcc.gnu.org>

PR fortran/25829
* Makefile.am: Add async.c to gfor_io_src.
Add async.h to gfor_io_headers.
* Makefile.in: Regenerated.
* gfortran.map: Add _gfortran_st_wait_async.
* io/async.c: New file.
* io/async.h: New file.
* io/close.c: Include async.h.
(st_close): Call async_wait for an asynchronous unit.
* io/file_pos.c (st_backspace): Likewise.
(st_endfile): Likewise.
(st_rewind): Likewise.
(st_flush): Likewise.
* io/inquire.c: Add handling for asynchronous PENDING
and ID arguments.
* io/io.h (st_parameter_dt): Add async bit.
(st_parameter_wait): Correct.
(gfc_unit): Add au pointer.
(st_wait_async): Add prototype.
(transfer_array_inner): Likewise.
(st_write_done_worker): Likewise.
* io/open.c: Include async.h.
(new_unit): Initialize asynchronous unit.
* io/transfer.c (async_opt): New struct.
(wrap_scalar_transfer): New function.
(transfer_integer): Call wrap_scalar_transfer to do the work.
(transfer_real): Likewise.
(transfer_real_write): Likewise.
(transfer_character): Likewise.
(transfer_character_wide): Likewise.
(transfer_complex): Likewise.
(transfer_array_inner): New function.
(transfer_array): Call transfer_array_inner.
(transfer_derived): Call wrap_scalar_transfer.
(data_transfer_init): Check for asynchronous I/O.
Perform a wait operation on any pending asynchronous I/O
if the data transfer is synchronous. Copy PDT and enqueue
thread for data transfer.
(st_read_done_worker): New function.
(st_read_done): Enqueue transfer or call st_read_done_worker.
(st_write_done_worker): New function.
(st_write_done): Enqueue transfer or call st_read_done_worker.
(st_wait): Document as no-op for compatibility reasons.
(st_wait_async): New function.
* io/unit.c (insert_unit): Use macros LOCK, UNLOCK and TRYLOCK;
add NOTE where necessary.
(get_gfc_unit): Likewise.
(init_units): Likewise.
(close_unit_1): Likewise. Call async_close if asynchronous.
(close_unit): Use macros LOCK and UNLOCK.
(finish_last_advance_record): Likewise.
(newunit_alloc): Likewise.
* io/unix.c (find_file): Likewise.
(flush_all_units_1): Likewise.
(flush_all_units): Likewise.
* libgfortran.h (generate_error_common): Add prototype.
* runtime/error.c: Include io.h and async.h.
(generate_error_common): New function.

2018-08-02 Nicolas Koenig <***@gcc.gnu.org>
Thomas Koenig <***@gcc.gnu.org>

PR fortran/25829
* testsuite/libgomp.fortran/async_io_1.f90: New test.
* testsuite/libgomp.fortran/async_io_2.f90: New test.
* testsuite/libgomp.fortran/async_io_3.f90: New test.
* testsuite/libgomp.fortran/async_io_4.f90: New test.
* testsuite/libgomp.fortran/async_io_5.f90: New test.
* testsuite/libgomp.fortran/async_io_6.f90: New test.
* testsuite/libgomp.fortran/async_io_7.f90: New test.

Christophe Lyon

2018-08-02 15:42:46 UTC

Post by Nicolas Koenig
Hello everyone,
Here is an updated version of the patch that hopefully fixes the compilation
problems by disabling async I/O if conditions are not supported by the target.
I would appreciate if people could test it on systems on which it failed
before. As for the array_constructor_8.f90 failure reported in the PR, why
it fails is beyond me, it doesn't even use I/O. Maybe/Probably something
unrelated?

Hi,
I'm probably missing something obvious, but after applying this patch
on top of r263136, the builds fail while building libgfortran:
/tmp/9271913_1.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgfortran/runtime/error.c:28:10:
fatal error: async.h: No such file or directory
#include "async.h"
^~~~~~~~~
compilation terminated.
make[3]: *** [error.lo] Error 1

Post by Nicolas Koenig
Nicolas
PR fortran/25829
* gfortran.texi: Add description of asynchronous I/O.
* trans-decl.c (gfc_finish_var_decl): Treat asynchronous variables
as volatile.
* trans-io.c (gfc_build_io_library_fndecls): Rename st_wait to
st_wait_async and change argument spec from ".X" to ".w".
(gfc_trans_wait): Pass ID argument via reference.
PR fortran/25829
* gfortran.dg/f2003_inquire_1.f03: Add write statement.
* gfortran.dg/f2003_io_1.f03: Add wait statement.
PR fortran/25829
* Makefile.am: Add async.c to gfor_io_src.
Add async.h to gfor_io_headers.
* Makefile.in: Regenerated.
* gfortran.map: Add _gfortran_st_wait_async.
* io/async.c: New file.
* io/async.h: New file.
* io/close.c: Include async.h.
(st_close): Call async_wait for an asynchronous unit.
* io/file_pos.c (st_backspace): Likewise.
(st_endfile): Likewise.
(st_rewind): Likewise.
(st_flush): Likewise.
* io/inquire.c: Add handling for asynchronous PENDING
and ID arguments.
* io/io.h (st_parameter_dt): Add async bit.
(st_parameter_wait): Correct.
(gfc_unit): Add au pointer.
(st_wait_async): Add prototype.
(transfer_array_inner): Likewise.
(st_write_done_worker): Likewise.
* io/open.c: Include async.h.
(new_unit): Initialize asynchronous unit.
* io/transfer.c (async_opt): New struct.
(wrap_scalar_transfer): New function.
(transfer_integer): Call wrap_scalar_transfer to do the work.
(transfer_real): Likewise.
(transfer_real_write): Likewise.
(transfer_character): Likewise.
(transfer_character_wide): Likewise.
(transfer_complex): Likewise.
(transfer_array_inner): New function.
(transfer_array): Call transfer_array_inner.
(transfer_derived): Call wrap_scalar_transfer.
(data_transfer_init): Check for asynchronous I/O.
Perform a wait operation on any pending asynchronous I/O
if the data transfer is synchronous. Copy PDT and enqueue
thread for data transfer.
(st_read_done_worker): New function.
(st_read_done): Enqueue transfer or call st_read_done_worker.
(st_write_done_worker): New function.
(st_write_done): Enqueue transfer or call st_read_done_worker.
(st_wait): Document as no-op for compatibility reasons.
(st_wait_async): New function.
* io/unit.c (insert_unit): Use macros LOCK, UNLOCK and TRYLOCK;
add NOTE where necessary.
(get_gfc_unit): Likewise.
(init_units): Likewise.
(close_unit_1): Likewise. Call async_close if asynchronous.
(close_unit): Use macros LOCK and UNLOCK.
(finish_last_advance_record): Likewise.
(newunit_alloc): Likewise.
* io/unix.c (find_file): Likewise.
(flush_all_units_1): Likewise.
(flush_all_units): Likewise.
* libgfortran.h (generate_error_common): Add prototype.
* runtime/error.c: Include io.h and async.h.
(generate_error_common): New function.
PR fortran/25829
* testsuite/libgomp.fortran/async_io_1.f90: New test.
* testsuite/libgomp.fortran/async_io_2.f90: New test.
* testsuite/libgomp.fortran/async_io_3.f90: New test.
* testsuite/libgomp.fortran/async_io_4.f90: New test.
* testsuite/libgomp.fortran/async_io_5.f90: New test.
* testsuite/libgomp.fortran/async_io_6.f90: New test.
* testsuite/libgomp.fortran/async_io_7.f90: New test.

Nicolas Koenig

2018-08-02 17:04:43 UTC

Hi,
I'm probably missing something obvious, but after applying this patch
fatal error: async.h: No such file or directory
#include "async.h"
^~~~~~~~~
compilation terminated.
make[3]: *** [error.lo] Error 1

Hi,

It wasn't you who missed something obvious. Typing `svn add` is hard.
Here is a version of the patch with the two new files.

Nicolas

Christophe Lyon

2018-08-03 08:45:30 UTC

Post by Nicolas Koenig

Hi,
It wasn't you who missed something obvious. Typing `svn add` is hard.
Here is a version of the patch with the two new files.

OK,

I applied this patch, and again I still see regressions on
armeb-none-linux-gnueabihf
--with-cpu cortex-a9
--with-fpu neon-fp16

FAIL: gfortran.dg/array_constructor_8.f90 -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions execution
test
FAIL: gfortran.dg/array_constructor_8.f90 -O3 -g execution test

gfortran.log contains:
STOP 2
STOP 2

Christophe

Post by Nicolas Koenig
Nicolas

Thomas König

2018-08-03 22:42:54 UTC

Hi Cristophe,

this is seriously weird - there is not even an I/O statement in that test case.

One question: Is this real hardware or an emulator?

Also, Could you try a few things?

Run the test case manually. Do you still fail?

Is there an error if the executable is run under valgrind?

If you have two compilers, one with the patch and one without: Is there a difference in the generated files for

-dump-tree-original, -fdump-tree-optimized and -S?

Regards, Thomas

Christophe Lyon

2018-08-06 11:32:57 UTC

Post by Thomas KÃ¶nig
Hi Cristophe,
this is seriously weird - there is not even an I/O statement in that test case.
One question: Is this real hardware or an emulator?

I'm using QEMU

Post by Thomas KÃ¶nig
Also, Could you try a few things?
Run the test case manually. Do you still fail?

Yes.

Post by Thomas KÃ¶nig
Is there an error if the executable is run under valgrind?

I don't know how to do that with qemu, nor if valgrind supports armeb?

Post by Thomas KÃ¶nig
If you have two compilers, one with the patch and one without: Is there a difference in the generated files for
-dump-tree-original, -fdump-tree-optimized and -S?

I posted a few comments in the associated PR:
- the .s files are the same with /without the patch, so I suppose the
problem comes from the runtime libraries
- I've attached both execution traces and output from objdump on the
statically linked executable, so as to hopefully include all the code
executed

Post by Thomas KÃ¶nig
Regards, Thomas

Thomas Koenig

2018-08-17 15:41:08 UTC

Hi Christophe,

sorry that this took so long, but a holiday followed by a
business trip seven timezones away can do that :-)

Post by Christophe Lyon
I applied this patch, and again I still see regressions on
armeb-none-linux-gnueabihf
--with-cpu cortex-a9
--with-fpu neon-fp16

The info that you supplied in the PR indicates some sort of library
problem exposed by the patch, possibly by including gthr.h.

All Nicolas and I could come up with was to remove the async I/O
functionality from armeb-* and by xfailing the tests.

This is done by

+#if defined(__GTHREAD_HAS_COND) && defined(__GTHREADS_CXX0X) &&
!defined(__ARMEB__)
+#define ASYNC_IO 1
+#else
+#define ASYNC_IO 0
+#endif

If somebody comes up with something more fine-grained for the
feature test, we can put this in now or later.

Regression-tested on x86_64-pc-linux-gnu (which showed that
xfail lines in the testsuite aren't wildly inaccurate).

So, I'd appreciate testing. If this passes, this will be
committed ASAP.

Regards

Thomas

Christophe Lyon

2018-08-18 22:43:48 UTC

Post by Thomas Koenig
Hi Christophe,

Hi,

Post by Thomas Koenig
sorry that this took so long, but a holiday followed by a
business trip seven timezones away can do that :-)

Sorry, I am on holidays too, and not back yet :)

Post by Christophe Lyon
I applied this patch, and again I still see regressions on
armeb-none-linux-gnueabihf
--with-cpu cortex-a9
--with-fpu neon-fp16

The info that you supplied in the PR indicates some sort of library
problem exposed by the patch, possibly by including gthr.h.
All Nicolas and I could come up with was to remove the async I/O
functionality from armeb-* and by xfailing the tests.
This is done by
+#if defined(__GTHREAD_HAS_COND) && defined(__GTHREADS_CXX0X) &&
!defined(__ARMEB__)
+#define ASYNC_IO 1
+#else
+#define ASYNC_IO 0
+#endif
If somebody comes up with something more fine-grained for the
feature test, we can put this in now or later.
Regression-tested on x86_64-pc-linux-gnu (which showed that
xfail lines in the testsuite aren't wildly inaccurate).
So, I'd appreciate testing. If this passes, this will be
committed ASAP.

I tried this version of the patch, and I'm still seeing the regression
on array_constructor_8.f90.
I didn't try to run the new tests (I only applied the patch part)

I'll try to investigate the PR a bit more when I'm back at the office
(e/o August)

Christophe

Post by Thomas Koenig
Regards
Thomas

Thomas Koenig

2018-08-19 13:39:54 UTC

Hi Christophe,

Post by Christophe Lyon
Hi,

Post by Thomas Koenig
This is done by
+#if defined(__GTHREAD_HAS_COND) && defined(__GTHREADS_CXX0X) &&
!defined(__ARMEB__)
+#define ASYNC_IO 1
+#else
+#define ASYNC_IO 0
+#endif

I tried this version of the patch, and I'm still seeing the regression
on array_constructor_8.f90.

Urgh...

Could you run

gcc -dM foo.h

with an empty foo.h in your target environment to see if
__ARMEB__ is actually defined? If it is not, what other
macro or combination of macros could be used?

At the moment, and with the data you posted in the PR, it seems that
this might be a library problem. Calling different library functions
with or without the patch seems to be an indication of that.

Currently, this patch holds up other major work :-(

What I would propose is to commit the patch as given in
https://gcc.gnu.org/ml/fortran/2018-08/msg00007.html
and open a PR for the strange armeb failure (if the test
for the macros above does not a promising approach).
Let us then fix this PR before the 9.0 release.

Regards

Thomas

Thomas Koenig

2018-08-21 19:42:48 UTC

Hi everybody,

Nicolas has committed the patch as r263750.

PR 87048 now traces the armeb regression, which is
assumed to resurface now. Let's really try and fix
that one before 9.0 :-)

Regards

Thomas

Thomas Koenig

2018-08-21 20:08:42 UTC

Hi everybody,

apparently, this didn't go to ***@.

Nicolas has committed the patch as r263750.

PR 87048 now traces the armeb regression, which is
assumed to resurface now. Let's really try and fix
that one before 9.0 :-)

Regards

Thomas

David Edelsohn

2018-08-22 18:49:17 UTC

Thomas,

This patch broke bootstrap on AIX again. This is completely unacceptable.

In file included from
*/nasfarm/edelsohn/src/src/libgfortran/runtime/error.c:28*:

*/nasfarm/edelsohn/src/src/libgfortran/io/async.h:333:3:* *error: *unknown
type name '*__gthread_cond_t*'

333 | *__gthread_cond_t* signal;

| *^~~~~~~~~~~~~~~~*

make[1]: *** [Makefile:2594: error.lo] Error 1

- David

Post by Thomas Koenig
Hi everybody,
Nicolas has committed the patch as r263750.
PR 87048 now traces the armeb regression, which is
assumed to resurface now. Let's really try and fix
that one before 9.0 :-)
Regards
Thomas

Thomas Koenig

2018-08-22 21:30:18 UTC

Hi David,

Post by David Edelsohn
This patch broke bootstrap on AIX again. This is completely unacceptable.

Again, sorry for the breakage.

I faced quite some challenges trying to get bootstrap to
work on gcc119. Despite quite a few e-mails (plus hints in a PR)
that I received, none of the hints for bootstrap I got actually got
it to work. I finally gave up after four or five different failures,
and the patch was committed because every test
that _could_ be run did show no failure.

Had we received instructions that work for bootstrapping on AIX,
we would have tested the patch there.

If it were possible to add instructions that do work for AIX
bootstrapping to the compile farm wiki, that would be great.

If you (or somebody else who has the requisite AIX fu) could test
patches that are known to be difficult, that would also be
great.

As long as we have no other solution, it is probably best to #ifdef out
AIX any additional pthread-related functionality for libgfortran that
might be coming along. That can always be integrated later, if somebody
can re-implement POSIX condition variables for libgfortran from what
AIX provides.

Let's talk about how to proceed at the GNU cauldron, over a beer.
Both Nicolas and I will be there.

In the meantime, I have committed the following patch (r263788) as
obvious after regression-testing on Linux both with ASYNC_IO set
to 1 and to 0. Let me know how that works out.

2018-08-22 Thomas Koenig <***@gcc.gnu.org>

* async.h: Set ASYNC_IO to zero if _AIX is defined.
(struct adv_cond): If ASYNC_IO is zero, the struct has no members.
(async_unit): If ASYNC_IO is zero, remove unneeded members.

2018-08-22 Thomas Koenig <***@gcc.gnu.org>

* gfortran.texi: Mention that asynchronous I/O does
not work on systems which lack condition variables, such
as AIX.

Regards

Thomas

David Edelsohn

2018-08-23 12:23:53 UTC

Thomas,

After your complaint about bootstrap on gcc119, I tried it again. My
bootstrap worked correctly. I used exactly the public instructions.
Others have been able to bootstrap on gcc119. The problem is local to
you. Does your environment have any special variables?

You can see the bootstrap in /scratch/dje/GCC

Second, the patch that you applied is unacceptable. ASYNC_IO already is
set to zero for AIX. I refuse to accept this patch as correct or complete.

Third, the problem from the beginning has been __gthread_cond_t. The
earlier patch had

struct adv_cond
{
int pending;
__gthread_mutex_t lock;
__gthread_cond_t signal;
};

with __gthread_cond_t clearly not protected by #if ASYNC_IO. This is
visible by inspection. Without a bootstrap, this patch should have been
visually checked that all uses of __gthread_cond_t were protected.

Similarly one can test this / confirm this on any system by setting
ASYNC_IO to 0 and ensuring that __gthread_cond_t is not declared. This
simply is sloppy work.

Please fix this correctly. The current kludge is not acceptable.

Thanks, David

Post by Thomas Koenig
Hi David,

Post by David Edelsohn
This patch broke bootstrap on AIX again. This is completely

unacceptable.
Again, sorry for the breakage.
I faced quite some challenges trying to get bootstrap to
work on gcc119. Despite quite a few e-mails (plus hints in a PR)
that I received, none of the hints for bootstrap I got actually got
it to work. I finally gave up after four or five different failures,
and the patch was committed because every test
that _could_ be run did show no failure.
Had we received instructions that work for bootstrapping on AIX,
we would have tested the patch there.
If it were possible to add instructions that do work for AIX
bootstrapping to the compile farm wiki, that would be great.
If you (or somebody else who has the requisite AIX fu) could test
patches that are known to be difficult, that would also be
great.
As long as we have no other solution, it is probably best to #ifdef out
AIX any additional pthread-related functionality for libgfortran that
might be coming along. That can always be integrated later, if somebody
can re-implement POSIX condition variables for libgfortran from what
AIX provides.
Let's talk about how to proceed at the GNU cauldron, over a beer.
Both Nicolas and I will be there.
In the meantime, I have committed the following patch (r263788) as
obvious after regression-testing on Linux both with ASYNC_IO set
to 1 and to 0. Let me know how that works out.
* async.h: Set ASYNC_IO to zero if _AIX is defined.
(struct adv_cond): If ASYNC_IO is zero, the struct has no members.
(async_unit): If ASYNC_IO is zero, remove unneeded members.
* gfortran.texi: Mention that asynchronous I/O does
not work on systems which lack condition variables, such
as AIX.
Regards
Thomas

Thomas Koenig

2018-08-23 17:42:08 UTC

David,

I will take the "thank you for restoring bootstrap for AIX" as implied
in your e-mail.

Second, the patch that you applied is unacceptable. ASYNC_IO already is
set to zero for AIX.

There is a clear procedure for this something that you deem
"unacceptable": Submit a patch (for which I promise a fast
review) or, if you feel that the patch is simple and obvious,
commit it yourself and announce it to the fortran and gcc-patches
mailing list.

Thomas

David Edelsohn

2018-08-23 12:52:52 UTC

Thomas,

Once the plural.c file has been re-generated incorrectly using the local
Yacc, it must be deleted and checked out again. Did you pull it fresh from
the repository in your AIX tree after the incorrect checkout?

Thanks, David

Post by Thomas Koenig
Hi David,

Post by David Edelsohn
This patch broke bootstrap on AIX again. This is completely

Ulrich Weigand

2018-07-26 13:40:46 UTC

Post by Ulrich Weigand

Post by Nicolas Koenig
Hello everyone,
I have committed the async I/O patch as r262978.
The test cases are in libgomp.fortran for now, maybe that can be changed
later.

It looks like this broke building libgfortran on spu, and presumably
any platform that uses gthr-simple instead of gthr-posix.

The file is called gthr-single.h, not gthr-simple.h ... sorry for the typo.

Bye,
Ulrich

--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
***@de.ibm.com

David Edelsohn

2018-07-26 15:18:17 UTC