Re: Build fail on gthr-simple.h targets (Re: AsyncI/O patch committed)

Re: Build fail on gthr-simple.h targets (Re: AsyncI/O patch committed)

[David Edelsohn]

>>>>> Ulrich Weigand wrote:

> Nicholas Koenig wrote:
>
>> 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.

Yes, this broke bootstrap for AIX as well.

- David

Re: Build fail on gthr-simple.h targets (Re: AsyncI/O patch committed)

[Thomas König]

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

> Am 31.07.2018 um 09:45 schrieb Andre Vieira (lists) <Andre.SimoesDiasVieira@arm.com>:
> 
>> On 27/07/18 13:25, David Edelsohn wrote:
>> 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
>>> On Fri, Jul 27, 2018 at 3:30 AM Thomas Koenig <tkoenig@netcologne.de> wrote:
>>> 
>>>> Am 26.07.2018 um 22:54 schrieb Thomas Koenig:
>>>> Hi Ulrich,
>>>> 
>>>>> 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).
>>>> 
>>> 
>>> 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  <andre.simoesdiasvieira@arm.com>
> 
>    Reverted 'AsyncI/O patch committed'
>    2018-07-25  Nicolas Koenig  <koenigni@gcc.gnu.org>
>        Thomas Koenig <tkoenig@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  <koenigni@gcc.gnu.org>
>        Thomas Koenig <tkoenig@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  <koenigni@gcc.gnu.org>
>        Thomas Koenig <tkoenig@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.
> 

Re: Build fail on gthr-simple.h targets (Re: AsyncI/O patch committed)

[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
On Fri, Jul 27, 2018 at 3:30 AM Thomas Koenig <tkoenig@netcologne.de> wrote:
>
> Am 26.07.2018 um 22:54 schrieb Thomas Koenig:
> > Hi Ulrich,
> >
> >> 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).
> >
>
> 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

Re: Build fail on gthr-simple.h targets (Re: AsyncI/O patch committed)

[Thomas Koenig]

Am 26.07.2018 um 22:54 schrieb Thomas Koenig:
> Hi Ulrich,
> 
>> 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).
> 

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

Re: Build fail on gthr-simple.h targets (Re: AsyncI/O patch committed)

[Thomas Koenig]

[-- Attachment #1: Type: text/plain, Size: 2068 bytes --]

Hi Ulrich,

> 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  <tkoenig@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.

[-- Attachment #2: p-rep-1.diff --]
[-- Type: text/x-patch, Size: 3551 bytes --]

Index: io/async.h
===================================================================
--- io/async.h	(revision 262978)
+++ io/async.h	(working copy)
@@ -25,6 +25,16 @@
 #ifndef ASYNC_H
 #define ASYNC_H
 
+/* Async I/O will not work on targets which do not support
+   __gthread_cond_t and __gthread_equal / __gthread_self.  Check
+   this.  */
+
+#if defined(__GTHREAD_HAS_COND) && defined(__GTHREADS_CXX0X)
+#define ASYNC_IO 1
+#else
+#undef ASYNC_IO
+#endif
+
 /* Defining DEBUG_ASYNC will enable somewhat verbose debugging
    output for async I/O.  */
 
@@ -217,6 +227,8 @@
 
 #define INTERN_UNLOCK(mutex) T_ERROR (__gthread_mutex_unlock, mutex);
 
+#if ASYNC_IO
+
 #define SIGNAL(advcond) do{						\
     INTERN_LOCK (&(advcond)->lock);					\
     (advcond)->pending = 1;						\
@@ -257,6 +269,15 @@
     INTERN_UNLOCK (&(advcond)->lock);		\
   } while (0)
 
+#else
+
+#define SIGNAL(advcond) do{} while(0)
+#define WAIT_SIGNAL_MUTEX(advcond, condition, mutex) do{} while(0)
+#define REVOKE_SIGNAL(advcond) do{} while(0)
+
+#endif
+
+#if ASYNC_IO
 DEBUG_LINE (extern __thread const char *aio_prefix);
 
 DEBUG_LINE (typedef struct aio_lock_debug{
@@ -274,6 +295,7 @@ DEBUG_LINE (extern __gthread_mutex_t debug_queue_l
    error reporting.  */
 
 extern __thread gfc_unit *thread_unit;
+#endif
 
 enum aio_do {
   AIO_INVALID = 0,
@@ -285,6 +307,8 @@ enum aio_do {
   AIO_CLOSE
 };
 
+#if ASYNC_IO
+
 typedef union transfer_args
 {
   struct
@@ -342,6 +366,23 @@ typedef struct async_unit
 
 } async_unit;
 
+#else
+typedef union transfer_args
+{
+  int x;
+};
+
+struct adv_cond
+{
+  int x;
+};
+
+typedef struct async_unit
+{
+  int x;
+};
+#endif
+
 void init_async_unit (gfc_unit *);
 internal_proto (init_async_unit);
 
Index: io/async.c
===================================================================
--- io/async.c	(revision 262978)
+++ io/async.c	(working copy)
@@ -36,6 +36,7 @@
 #include <sys/types.h>
 
 #include "async.h"
+#if ASYNC_IO
 
 DEBUG_LINE (__thread const char *aio_prefix = MPREFIX);
 
@@ -481,3 +482,88 @@ async_close (async_unit *au)
   T_ERROR (__gthread_join, au->thread, NULL);
   free_async_unit (au);
 }
+
+#else
+
+/* Do-nothing function, which will not be called.  */
+
+void
+init_async_unit (gfc_unit *u)
+{
+  u->au = NULL;
+  return;
+}
+
+/* Do-nothing function, which will not be called.  */
+
+void
+enqueue_transfer (async_unit *au, transfer_args *arg, enum aio_do type)
+{
+  return;
+}
+
+/* Do-nothing function, which will not be called.  */
+
+int
+enqueue_done_id (async_unit *au, enum aio_do type)
+{
+  return 0;
+}
+
+/* Do-nothing function, which will not be called.  */
+
+void
+enqueue_done (async_unit *au, enum aio_do type)
+{
+  return;
+}
+
+/* Do-nothing function, which will not be called.  */
+
+void
+enqueue_close (async_unit *au)
+{
+  return;
+}
+
+/* Do-nothing function, which will not be called.  */
+
+void
+enqueue_data_transfer_init (async_unit *au, st_parameter_dt *dt, int read_flag)
+{
+  return;
+}
+
+/* Do-nothing function, which will not be called.  */
+
+bool
+collect_async_errors (st_parameter_common *cmp, async_unit *au)
+{
+  return false;
+}
+
+/* Do-nothing function, which will not be called.  */
+
+bool
+async_wait_id (st_parameter_common *cmp, async_unit *au, int i)
+{
+  return false;
+}
+
+/* Do-nothing function, which will not be called.  */
+
+bool
+async_wait (st_parameter_common *cmp, async_unit *au)
+{
+  return false;
+}
+
+/* Do-nothing function, which will not be called.  */
+
+void
+async_close (async_unit *au)
+{
+  return;
+}
+
+#endif

Build fail on gthr-simple.h targets (Re: AsyncI/O patch committed)

[Ulrich Weigand]

Nicholas Koenig wrote:

> 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
  Ulrich.Weigand@de.ibm.com