[Patch, Fortran] Add caf_runtime_error to libgfortran/caf/single.c
This patch adds a caf_runtime_error function to the non-MPI
implementation of Coarray Fortran. It is based on the MPI function of
the same name in mpi.c.
Ok to commit?
ChangeLog:
2011-07-14 Daniel Carrera dcarr...@gmail.com
* caf/single.c: Include stdarg.h header.
(caf_runtime_error): New function based on the function in
mpi.c with the same name.
(_gfortran_caf_init): Use caf_runtime_error.
* caf/mpi.c (caf_runtime_error): Add a note to keep in sync
with the function in single.c.
--
I'm not overweight, I'm undertall.
Index: libgfortran/caf/single.c
===
--- libgfortran/caf/single.c (revision 176230)
+++ libgfortran/caf/single.c (working copy)
@@ -28,6 +28,7 @@ see the files COPYING3 and COPYING.RUNTI
#include stdio.h /* For fputs and fprintf. */
#include stdlib.h /* For exit and malloc. */
#include string.h /* For memcpy and memset. */
+#include stdarg.h /* For variadic arguments. */
/* Define GFC_CAF_CHECK to enable run-time checking. */
/* #define GFC_CAF_CHECK 1 */
@@ -40,6 +41,21 @@ see the files COPYING3 and COPYING.RUNTI
caf_static_t *caf_static_list = NULL;
+/* Keep in sync with mpi.c. */
+static void
+caf_runtime_error (int error, const char *message, ...)
+{
+ va_list ap;
+ fprintf (stderr, Fortran runtime error.);
+ va_start (ap, message);
+ fprintf (stderr, message, ap);
+ va_end (ap);
+ fprintf (stderr, \n);
+
+ /* FIXME: Shutdown the Fortran RTL to flush the buffer. PR 43849. */
+ exit (error);
+}
+
void
_gfortran_caf_init (int *argc __attribute__ ((unused)),
char ***argv __attribute__ ((unused)),
@@ -73,12 +89,12 @@ _gfortran_caf_register (ptrdiff_t size,
if (unlikely (local == NULL || token == NULL))
{
+ const char msg[] = Failed to allocate coarray;
if (stat)
{
*stat = 1;
if (errmsg_len 0)
{
- const char msg[] = Failed to allocate coarray;
int len = ((int) sizeof (msg) errmsg_len) ? errmsg_len
: (int) sizeof (msg);
memcpy (errmsg, msg, len);
@@ -88,10 +104,7 @@ _gfortran_caf_register (ptrdiff_t size,
return NULL;
}
else
- {
- fprintf (stderr, ERROR: Failed to allocate coarray);
- exit (1);
- }
+ caf_runtime_error (1, msg);
}
if (stat)
Index: libgfortran/caf/mpi.c
===
--- libgfortran/caf/mpi.c (revision 176230)
+++ libgfortran/caf/mpi.c (working copy)
@@ -47,6 +47,7 @@ static int caf_is_finalized;
caf_static_t *caf_static_list = NULL;
+/* Keep in sync with single.c. */
static void
caf_runtime_error (int error, const char *message, ...)
{
@@ -62,7 +63,7 @@ caf_runtime_error (int error, const char
MPI_Abort (MPI_COMM_WORLD, error);
/* Should be unreachable, but to make sure also call exit. */
- exit (2);
+ exit (error);
}
Re: [Patch, Fortran] Add caf_runtime_error to libgfortran/caf/single.c
On 07/14/2011 11:48 AM, Daniel Carrera wrote: This patch adds a caf_runtime_error function to the non-MPI implementation of Coarray Fortran. It is based on the MPI function of the same name in mpi.c. Ok to commit? I was wondering - based on the discussion - whether one should remove the int error argument from caf_runtime_error and simply use exit (EXIT_FAILURE) for all exit() calls in mpi.c/single.c, cf. http://gcc.gnu.org/ml/fortran/2011-07/msg00140.html But one can also do so as follow up patch. Thus, OK for the patch as is - or with replacing all exit(...) by exit(EXIT_FAILURE), which uses stdlib.h's EXIT_FAILURE. One then can also drop the int error argument to the caf_runtime_error function. Thanks for the patch! Tobias ChangeLog: 2011-07-14 Daniel Carrera dcarr...@gmail.com * caf/single.c: Include stdarg.h header. (caf_runtime_error): New function based on the function in mpi.c with the same name. (_gfortran_caf_init): Use caf_runtime_error. * caf/mpi.c (caf_runtime_error): Add a note to keep in sync with the function in single.c.
Re: [Patch, Fortran] Add caf_runtime_error to libgfortran/caf/single.c
On 07/14/2011 12:04 PM, Tobias Burnus wrote: I was wondering - based on the discussion - whether one should remove the int error argument from caf_runtime_error and simply use exit (EXIT_FAILURE) for all exit() calls in mpi.c/single.c, cf. http://gcc.gnu.org/ml/fortran/2011-07/msg00140.html But one can also do so as follow up patch. You are the boss. The message I got from Nick's post is that it doesn't matter much and that you could even get surprising behaviour because EXIT_SUCCESS is not required to be zero and EXIT_FAILURE is not required to be non-zero. But maybe I missed the point. So it's up to you. Cheers, Daniel. -- I'm not overweight, I'm undertall.
Re: [Patch, Fortran] Add caf_runtime_error to libgfortran/caf/single.c
On 07/14/2011 12:14 PM, Daniel Carrera wrote: On 07/14/2011 12:04 PM, Tobias Burnus wrote: I was wondering - based on the discussion - whether one should remove the int error argument from caf_runtime_error and simply use exit (EXIT_FAILURE) for all exit() calls in mpi.c/single.c, cf. http://gcc.gnu.org/ml/fortran/2011-07/msg00140.html But one can also do so as follow up patch. You are the boss. The message I got from Nick's post is that it doesn't matter much and that you could even get surprising behaviour because EXIT_SUCCESS is not required to be zero and EXIT_FAILURE is not required to be non-zero. But maybe I missed the point. So it's up to you. Well, for exit(3) one knows that one will get a surprising in Windows: An abort. While EXIT_FAILURE [but also exit(1)] have a very high change to be working (nearly) everywhere. At the end, as long as the operating system does not react in a completely odd way, the exit status does not matter - it's the problem of the caller (e.g. some script) to make use of the result. If EXIT_FAILURE is 0 and the script assumes that it is a success, it's either a fault of the script writer, or of the one putting 0 for EXIT_FAILURE into the header, or a fault of the OS designer. Thus, I think the chance that one avoids surprises is higher with exit(EXIT_FAILURE) and one avoids having some random number (for the exit status) in the code. Users usually do not check the exact exit status value but only its 0- vs non-0-ness. Hence, I think exit(EXIT_FAILURE) is better than the current version. Tobias
Re: [Patch, Fortran] Add caf_runtime_error to libgfortran/caf/single.c
Hi Tobias,
As per your suggestion, I'm copying to gfotran and gcc-patches. I've
made the change your asked and I'm going to commit the patch.
Cheers,
Daniel.
On 07/14/2011 05:31 PM, Tobias Burnus wrote:
On 07/14/2011 05:05 PM, Daniel Carrera wrote:
+caf_runtime_error (const char *message, ...)
+{
+ va_list ap;
+ fprintf (stderr, Fortran runtime error.);
Could you replace . by : (colon, space), I think Fortran runtime
error: Could not ... looks better than Fortran runtime error.Could not
Otherwise, the patch is OK. Could you then send the patch as to fortran@
and gcc-patches@?
Tobias
PS: You could directly commit the modified patch, simply reply to this
email, add a CC to fortran@/gcc-patches@ and attach the patch and
changelog. There is no need that I approve the patch again.
--
I'm not overweight, I'm undertall.
Re: [Patch, Fortran] Add caf_runtime_error to libgfortran/caf/single.c
And of course, here is the patch and ChangeLog.
2011-07-14 Daniel Carrera dcarr...@gmail.com
* caf/single.c: Include stdarg.h header.
(caf_runtime_error): New function. Use exit(EXIT_FAILURE).
(_gfortran_caf_register): Use caf_runtime_error.
(_gfortran_caf_sync_images): Use exit(EXIT_FAILURE).
* caf/mpi.c (caf_runtime_error): Remove error parameter.
Return EXIT_FAILURE instead.
(_gfortran_caf_register): Update call to caf_runtime_error.
(_gfortran_caf_sync_all): Ditto.
(_gfortran_caf_sync_images): Ditto.
(_gfortran_caf_error_stop_str): Use exit(EXIT_FAILURE).
Now I'm just waiting for SVN update before I commit...
On 07/14/2011 05:34 PM, Daniel Carrera wrote:
Hi Tobias,
As per your suggestion, I'm copying to gfotran and gcc-patches. I've
made the change your asked and I'm going to commit the patch.
Cheers,
Daniel.
On 07/14/2011 05:31 PM, Tobias Burnus wrote:
On 07/14/2011 05:05 PM, Daniel Carrera wrote:
+caf_runtime_error (const char *message, ...)
+{
+ va_list ap;
+ fprintf (stderr, Fortran runtime error.);
Could you replace . by : (colon, space), I think Fortran runtime
error: Could not ... looks better than Fortran runtime error.Could not
Otherwise, the patch is OK. Could you then send the patch as to fortran@
and gcc-patches@?
Tobias
PS: You could directly commit the modified patch, simply reply to this
email, add a CC to fortran@/gcc-patches@ and attach the patch and
changelog. There is no need that I approve the patch again.
--
I'm not overweight, I'm undertall.
Index: libgfortran/caf/single.c
===
--- libgfortran/caf/single.c (revision 176230)
+++ libgfortran/caf/single.c (working copy)
@@ -28,6 +28,7 @@ see the files COPYING3 and COPYING.RUNTI
#include stdio.h /* For fputs and fprintf. */
#include stdlib.h /* For exit and malloc. */
#include string.h /* For memcpy and memset. */
+#include stdarg.h /* For variadic arguments. */
/* Define GFC_CAF_CHECK to enable run-time checking. */
/* #define GFC_CAF_CHECK 1 */
@@ -40,6 +41,21 @@ see the files COPYING3 and COPYING.RUNTI
caf_static_t *caf_static_list = NULL;
+/* Keep in sync with mpi.c. */
+static void
+caf_runtime_error (const char *message, ...)
+{
+ va_list ap;
+ fprintf (stderr, Fortran runtime error: );
+ va_start (ap, message);
+ fprintf (stderr, message, ap);
+ va_end (ap);
+ fprintf (stderr, \n);
+
+ /* FIXME: Shutdown the Fortran RTL to flush the buffer. PR 43849. */
+ exit (EXIT_FAILURE);
+}
+
void
_gfortran_caf_init (int *argc __attribute__ ((unused)),
char ***argv __attribute__ ((unused)),
@@ -73,12 +89,12 @@ _gfortran_caf_register (ptrdiff_t size,
if (unlikely (local == NULL || token == NULL))
{
+ const char msg[] = Failed to allocate coarray;
if (stat)
{
*stat = 1;
if (errmsg_len 0)
{
- const char msg[] = Failed to allocate coarray;
int len = ((int) sizeof (msg) errmsg_len) ? errmsg_len
: (int) sizeof (msg);
memcpy (errmsg, msg, len);
@@ -88,10 +104,7 @@ _gfortran_caf_register (ptrdiff_t size,
return NULL;
}
else
- {
- fprintf (stderr, ERROR: Failed to allocate coarray);
- exit (1);
- }
+ caf_runtime_error (msg);
}
if (stat)
@@ -140,7 +153,7 @@ _gfortran_caf_sync_images (int count __a
{
fprintf (stderr, COARRAY ERROR: Invalid image index %d to SYNC
IMAGES, images[i]);
- exit (1);
+ exit (EXIT_FAILURE);
}
#endif
Index: libgfortran/caf/mpi.c
===
--- libgfortran/caf/mpi.c (revision 176230)
+++ libgfortran/caf/mpi.c (working copy)
@@ -47,8 +47,9 @@ static int caf_is_finalized;
caf_static_t *caf_static_list = NULL;
+/* Keep in sync with single.c. */
static void
-caf_runtime_error (int error, const char *message, ...)
+caf_runtime_error (const char *message, ...)
{
va_list ap;
fprintf (stderr, Fortran runtime error on image %d: , caf_this_image);
@@ -59,10 +60,10 @@ caf_runtime_error (int error, const char
/* FIXME: Shutdown the Fortran RTL to flush the buffer. PR 43849. */
/* FIXME: Do some more effort than just MPI_ABORT. */
- MPI_Abort (MPI_COMM_WORLD, error);
+ MPI_Abort (MPI_COMM_WORLD, EXIT_FAILURE);
/* Should be unreachable, but to make sure also call exit. */
- exit (2);
+ exit (EXIT_FAILURE);
}
@@ -179,7 +180,7 @@ error:
}
}
else
- caf_runtime_error (caf_is_finalized ? STAT_STOPPED_IMAGE : 1, msg);
+ caf_runtime_error (msg);
}
return NULL;
@@ -223,7 +224,7 @@ _gfortran_caf_sync_all (int *stat, char
memset (errmsg[len], ' ', errmsg_len-len);
}
else
- caf_runtime_error (caf_is_finalized ? STAT_STOPPED_IMAGE : ierr, msg);
+ caf_runtime_error (msg);
}
}
@@ -291,7 +292,7 @@ _gfortran_caf_sync_images (int count, in
memset (errmsg[len],
Re: [Patch, Fortran] Add caf_runtime_error to libgfortran/caf/single.c
On Thu, Jul 14, 2011 at 18:40, Daniel Carrera dcarr...@gmail.com wrote: And of course, here is the patch and ChangeLog. 2011-07-14 Daniel Carrera dcarr...@gmail.com * caf/mpi.c (caf_runtime_error): Remove error parameter. Return EXIT_FAILURE instead. Well, this changelog entry is incorrect; you're calling exit(EXIT_FAILURE) and not returning a value. From the patch: /* FIXME: Shutdown the Fortran RTL to flush the buffer. PR 43849. */ This is unnecessary, as the call to exit() will call the libgfortran destructor which will close all units, as explained in comment #3 in the PR. -- Janne Blomqvist
Re: [Patch, Fortran] Add caf_runtime_error to libgfortran/caf/single.c
On 07/14/2011 09:48 PM, Janne Blomqvist wrote: 2011-07-14 Daniel Carreradcarr...@gmail.com * caf/mpi.c (caf_runtime_error): Remove error parameter. Return EXIT_FAILURE instead. Well, this changelog entry is incorrect; you're calling exit(EXIT_FAILURE) and not returning a value. Ok. What term should I have used? I was thinking that the program itself returns a value, but I can see that what I wrote was not optimal. From the patch: /* FIXME: Shutdown the Fortran RTL to flush the buffer. PR 43849. */ This is unnecessary, as the call to exit() will call the libgfortran destructor which will close all units, as explained in comment #3 in the PR. Ok. I didn't want to mess with comments that I didn't fully understand. -- I'm not overweight, I'm undertall.
Re: [Patch, Fortran] Add caf_runtime_error to libgfortran/caf/single.c
Janne Blomqvist wrote: * caf/mpi.c (caf_runtime_error): Remove error parameter. Return EXIT_FAILURE instead. From the patch: /* FIXME: Shutdown the Fortran RTL to flush the buffer. PR 43849. */ This is unnecessary, as the call to exit() will call the libgfortran destructor which will close all units, as explained in comment #3 in the PR. While I think it should be sufficient for single-processor usage, I am not sure that that way all I/O buffers gets written before one calls MPI_Finalize - nor am I sure how one would handle ERROR STOP with regards to I/O. In terms of I/O, there are three kinds of I/O, which might need to be treated differently: * I/O to stdout (OUTPUT_UNIT): Here, all output should be collected by MPI - I am not sure whether it will come to later in a destructor * I/O to (local) files * I/O via the communication library: Here, I see the greatest problems, but that's not in F2008's coarrays, but might well be added with the Technical Report. I somehow would feel better if I could ensure that the buffers are flushed and the files closed before I pull the MPI plug (MPI_Finalize, MPI_Abort). For reference, the comment Janne is referring to is the one at the bottom (comment 3) of http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43849 Tobias
Re: [Patch, Fortran] Add caf_runtime_error to libgfortran/caf/single.c
On Thu, Jul 14, 2011 at 23:34, Tobias Burnus bur...@net-b.de wrote: Janne Blomqvist wrote: * caf/mpi.c (caf_runtime_error): Remove error parameter. Return EXIT_FAILURE instead. From the patch: /* FIXME: Shutdown the Fortran RTL to flush the buffer. PR 43849. */ This is unnecessary, as the call to exit() will call the libgfortran destructor which will close all units, as explained in comment #3 in the PR. While I think it should be sufficient for single-processor usage, I am not sure that that way all I/O buffers gets written before one calls MPI_Finalize - nor am I sure how one would handle ERROR STOP with regards to I/O. Ah, I read that comment from caf/single.c, but now I see it in the caf/mpi.c part of the patch as well. Yeah, in that case it might matter, depending on how the MPI library implements MPI_Abort(). That is, does it call exit() (in which case the destructor will be called) or will it call abort() or raise some other fatal signal. At least Open MPI seems to implement it by sending SIGTERM http://www.open-mpi.org/doc/v1.4/man3/MPI_Abort.3.php Come to think of it, in this case, if you want to use MPI_Abort() rather than MPI_Finalize() + exit(), you should probably reset the fatal signal handlers to the default one unless you want the backtrace to be printed. -- Janne Blomqvist
