[patch, fortran] fixes for the EXIT intrinsic function (PR30933)

[patch, fortran] fixes for the EXIT intrinsic function (PR30933)

[Tobias Burnus]

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

:ADDPATCH fortran:

The following patch does the following

a) Correct the dummy argument name; for "exit(status)" (is was "code"
instead of "status") and for "new_line(A)" (it was "i").

b) It allows any integer kind for the exit subroutine. This is in the
spirit of Fortran 2003 which also allows any kind for most of the
intrinsic procedures. Status quo is to give a link-time error that
exit_i1 etc. does not exist.

For (b) there are several options:

i. Add i1, i2, and i16 versions to libgfortran
ii. Convert kind > 8 to integer(8) and kind < 4 to integer(4)
iii. As POSIX defines "exit(int)" and sizeof(int) is on most systems "4"
convert everything to integer(4)
iv. Convert everything to the default integer, i.e. integer(4) or
integer(8) depending whether -fdefault-integer-8 is used.

I have chosen i, but I don't know which is the best, cleanest and most
sensible choice. For option ii, I would really like to have INTEGER_4
defined in gcc/fortran/* as otherwise one has to work with magic
numbers: "if (kind < 4)".
(Option iii would change the ABI of the library - at least exit_i8 would
cease to exist.)

Build & regression tested on x86_64-unknown-linux-gnu.
Ok for the trunk and 4.2?

Tobias

PS: Thanks to Daniel Franke for finding the two bugs.

[-- Attachment #2: exit.diff --]
[-- Type: text/x-patch, Size: 4463 bytes --]

fortran/
2007-02-23  Tobias Burnus  <burnus@net-b.de>

	PR fortran/30933
	* intrinsic.c (add_functions): Correct name of dummy argument.

libgfortran/
2007-02-23  Tobias Burnus  <burnus@net-b.de>

	PR fortran/30933
	* intrinsics/exit.c (exit_i1,exit_i2,exit_i16): New functions.

testcase/
2007-02-23  Daniel Franke  <franke.daniel@gmail.com>

	PR fortran/30933
	* gfortran.dg/subroutine-exit-link.f90: New test.
	* gfortran.dg/subroutine-exit-compile.f90: New test.

Index: gcc/fortran/intrinsic.c
===================================================================
--- gcc/fortran/intrinsic.c	(revision 122263)
+++ gcc/fortran/intrinsic.c	(working copy)
@@ -1911,7 +1911,7 @@
 
   add_sym_1 ("new_line", NOT_ELEMENTAL, ACTUAL_NO, BT_CHARACTER, dc,
 	     GFC_STD_F2003, gfc_check_new_line, gfc_simplify_new_line, NULL,
-	     i, BT_CHARACTER, dc, REQUIRED);
+	     a, BT_CHARACTER, dc, REQUIRED);
 
   add_sym_2 ("nint", ELEMENTAL, ACTUAL_YES, BT_INTEGER, di, GFC_STD_F77,
 	     gfc_check_a_ikind, gfc_simplify_nint, gfc_resolve_nint,
@@ -2457,7 +2457,7 @@
 
   add_sym_1s ("exit", 0, BT_UNKNOWN, 0, GFC_STD_GNU,
 	      gfc_check_exit, NULL, gfc_resolve_exit,
-	      c, BT_INTEGER, di, OPTIONAL);
+	      st, BT_INTEGER, di, OPTIONAL);
 
   if ((gfc_option.allow_std & GFC_STD_GNU) || gfc_option.flag_all_intrinsics)
     make_noreturn();
Index: libgfortran/intrinsics/exit.c
===================================================================
--- libgfortran/intrinsics/exit.c	(revision 122263)
+++ libgfortran/intrinsics/exit.c	(working copy)
@@ -39,6 +39,24 @@
 /* SUBROUTINE EXIT(STATUS)
    INTEGER, INTENT(IN), OPTIONAL :: STATUS  */
 
+extern void exit_i1 (GFC_INTEGER_1 *);
+export_proto(exit_i1);
+
+void
+exit_i1 (GFC_INTEGER_1 * status)
+{
+  exit (status ? *status : 0);
+}
+
+extern void exit_i2 (GFC_INTEGER_2 *);
+export_proto(exit_i2);
+
+void
+exit_i2 (GFC_INTEGER_2 * status)
+{
+  exit (status ? *status : 0);
+}
+
 extern void exit_i4 (GFC_INTEGER_4 *);
 export_proto(exit_i4);
 
@@ -56,3 +74,14 @@
 {
   exit (status ? *status : 0);
 }
+
+#ifdef HAVE_GFC_INTEGER_16
+extern void exit_i16 (GFC_INTEGER_16 *);
+export_proto(exit_i16);
+
+void
+exit_i16 (GFC_INTEGER_16 * status)
+{
+  exit (status ? *status : 0);
+}
+#endif
Index: gcc/testsuite/gfortran.dg/subroutine-exit-compile.f90
===================================================================
--- gcc/testsuite/gfortran.dg/subroutine-exit-compile.f90	(revision 0)
+++ gcc/testsuite/gfortran.dg/subroutine-exit-compile.f90	(revision 0)
@@ -0,0 +1,33 @@
+! { dg-do compile }
+! PR fortran/13094
+! Contributed by Daniel Franke.
+program subroutine_exit_compile
+  implicit none
+  INTEGER(kind=1) :: int_1
+  INTEGER(kind=2) :: int_2
+  INTEGER(kind=4) :: int_4
+  INTEGER(kind=8) :: int_8
+  LOGICAL(kind=1) :: logical_1
+  LOGICAL(kind=2) :: logical_2
+  LOGICAL(kind=4) :: logical_4
+  LOGICAL(kind=8) :: logical_8
+  REAL(kind=4) :: real_4
+  REAL(kind=8) :: real_8
+  COMPLEX(kind=4) :: cmplx_4
+  COMPLEX(kind=8) :: cmplx_8
+  CHARACTER(len=100) :: string
+  CALL exit(status = int_1) ! valid
+  CALL exit(status = int_2) ! valid
+  CALL exit(status = int_4) ! valid
+  CALL exit(status = int_8) ! valid
+  CALL exit(status = logical_1) ! { dg-error "must be INTEGER" }
+  CALL exit(status = logical_2) ! { dg-error "must be INTEGER" }
+  CALL exit(status = logical_4) ! { dg-error "must be INTEGER" }
+  CALL exit(status = logical_8) ! { dg-error "must be INTEGER" }
+  CALL exit(status = real_4) ! { dg-error "must be INTEGER" }
+  CALL exit(status = real_8) ! { dg-error "must be INTEGER" }
+  CALL exit(status = cmplx_4) ! { dg-error "must be INTEGER" }
+  CALL exit(status = cmplx_8) ! { dg-error "must be INTEGER" }
+  CALL exit(status = string) ! { dg-error "must be INTEGER" }
+  CALL exit() ! valid
+end program
Index: gcc/testsuite/gfortran.dg/subroutine-exit-link.f90
===================================================================
--- gcc/testsuite/gfortran.dg/subroutine-exit-link.f90	(revision 0)
+++ gcc/testsuite/gfortran.dg/subroutine-exit-link.f90	(revision 0)
@@ -0,0 +1,15 @@
+! { dg-do link }
+! PR fortran/13094
+! Contributed by Daniel Franke.
+program subroutine_exit_link
+  implicit none
+  INTEGER(kind=1) :: int_1
+  INTEGER(kind=2) :: int_2
+  INTEGER(kind=4) :: int_4
+  INTEGER(kind=8) :: int_8
+  CALL exit()
+  CALL exit(status = int_1)
+  CALL exit(status = int_2)
+  CALL exit(status = int_4)
+  CALL exit(status = int_8)
+end program

Re: [patch, fortran] fixes for the EXIT intrinsic function (PR30933)

[Daniel Franke]

On Friday 23 February 2007 19:03:22 Tobias Burnus wrote:
> b) [The patch] allows any integer kind for the exit subroutine. This is 
> in the spirit of Fortran 2003 which also allows any kind for most of the
> intrinsic procedures. Status quo is to give a link-time error that
> exit_i1 etc. does not exist.
>
> For (b) there are several options:
>
> i. Add i1, i2, and i16 versions to libgfortran
> ii. Convert kind > 8 to integer(8) and kind < 4 to integer(4)
> iii. As POSIX defines "exit(int)" and sizeof(int) is on most systems "4"
> convert everything to integer(4)
> iv. Convert everything to the default integer, i.e. integer(4) or
> integer(8) depending whether -fdefault-integer-8 is used.
>
> I have chosen i
>

FLUSH implements (iii), see also PR30941.

Re: [patch, fortran] fixes for the EXIT intrinsic function (PR30933)

[Tobias Burnus]

Daniel Franke schrieb:
>> For (b) there are several options:
>>
>> i. Add i1, i2, and i16 versions to libgfortran
>> ii. Convert kind > 8 to integer(8) and kind < 4 to integer(4)
>> iii. As POSIX defines "exit(int)" and sizeof(int) is on most systems "4"
>> convert everything to integer(4)
>> iv. Convert everything to the default integer, i.e. integer(4) or
>> integer(8) depending whether -fdefault-integer-8 is used.
>>     
> FLUSH implements (iii)
>   
No, FLUSH implements (iv):

  ts.type = BT_INTEGER;
  ts.kind = gfc_default_integer_kind; /* <<<< can be 4 or 8 */
  n = c->ext.actual->expr;
  if (n != NULL && n->ts.kind != ts.kind)
    gfc_convert_type (n, &ts, 2);

That is also the reason that there is not flush() but flush_i4() and
flush_i8() in libgfortran/io/intrinsic.c.

Tobias

Re: [patch, fortran] fixes for the EXIT intrinsic function (PR30933)

[Daniel Franke]

On Friday 23 February 2007 23:43:38 Tobias Burnus wrote:
> Daniel Franke schrieb:
> >> For (b) there are several options:
> >>
> >> i. Add i1, i2, and i16 versions to libgfortran
> >> ii. Convert kind > 8 to integer(8) and kind < 4 to integer(4)
> >> iii. As POSIX defines "exit(int)" and sizeof(int) is on most systems "4"
> >> convert everything to integer(4)
> >> iv. Convert everything to the default integer, i.e. integer(4) or
> >> integer(8) depending whether -fdefault-integer-8 is used.
> >
> > FLUSH implements (iii)
>
> No, FLUSH implements (iv):
>
>   ts.type = BT_INTEGER;
>   ts.kind = gfc_default_integer_kind; /* <<<< can be 4 or 8 */
>   n = c->ext.actual->expr;
>   if (n != NULL && n->ts.kind != ts.kind)
>     gfc_convert_type (n, &ts, 2);
>
> That is also the reason that there is not flush() but flush_i4() and
> flush_i8() in libgfortran/io/intrinsic.c.

Standing corrected. Thanks.
Nevertheless, at least, it does not implement the suggested option.

Is stuff like this documented somewhere or is it obvious for those more 
familiar with the code than I am? Just glancing over it, checking the dump(s) 
and the output of 'nm' is obviously not enough ...

Re: [patch, fortran] fixes for the EXIT intrinsic function (PR30933)

[Steve Kargl]

On Fri, Feb 23, 2007 at 11:55:28PM +0100, Daniel Franke wrote:
> 
> Is stuff like this documented somewhere or is it obvious for those more 
> familiar with the code than I am? Just glancing over it, checking the dump(s) 
> and the output of 'nm' is obviously not enough ...

Unfortunately, no.  You're looking at a merging of different
novice contributions to gfortran.  I wrote some of the g77
intrinsics to cut my teeth, and then FX wrote several more g77
intrinsics as his intrduction to gfortran.  Others contributed
intrinsics in the dark ages when gfortran only worried about
integer(4) and (8).

A nice newbie (or Google SOC) project would be to audit 
the implementation details of libgfortran.  I wonder if
we have any fortran@ lurkers looking for something to do.

-- 
Steve

Re: [patch, fortran] fixes for the EXIT intrinsic function (PR30933)

[Tobias Burnus]

Hi,

Steve Kargl wrote:
> Unfortunately, no.  You're looking at a merging of different
> novice contributions to gfortran.  I wrote some of the g77
> intrinsics to cut my teeth, and then FX wrote several more g77
> intrinsics as his intrduction to gfortran.  Others contributed
> intrinsics in the dark ages when gfortran only worried about
> integer(4) and (8).
>   
In any case which of the following option should we choose for those
non-time critical functions which call C library functions? Currently,
we have a mixure of (i), and (iv).

i. Add i1, i2, and i16 versions to libgfortran

ii. Convert kind > 8 to integer(8) and kind < 4 to integer(4)

iii. As POSIX defines "exit(int)" and sizeof(int) is on most systems "4"
convert everything to integer(4)

iv. Convert everything to the default integer, i.e. integer(4) or
integer(8) depending whether -fdefault-integer-8 is used.


While (iv) is the easiest, it somehow looks a bit arbitrary. I therefore
would prefer either of (i) to (iii), with (iii) seeming to be the
cleanest - at least if one uses the same kind as the C library does.
(How does one ensure this?)

Tobias

Re: [patch, fortran] fixes for the EXIT intrinsic function (PR30933)

[Daniel Franke]

On Saturday 24 February 2007 13:29:36 Tobias Burnus wrote:
> Hi,
>
> Steve Kargl wrote:
> > Unfortunately, no.  You're looking at a merging of different
> > novice contributions to gfortran.  I wrote some of the g77
> > intrinsics to cut my teeth, and then FX wrote several more g77
> > intrinsics as his intrduction to gfortran.  Others contributed
> > intrinsics in the dark ages when gfortran only worried about
> > integer(4) and (8).
>
> In any case which of the following option should we choose for those
> non-time critical functions which call C library functions? Currently,
> we have a mixure of (i), and (iv).
>
> i. Add i1, i2, and i16 versions to libgfortran
>
> ii. Convert kind > 8 to integer(8) and kind < 4 to integer(4)
>
> iii. As POSIX defines "exit(int)" and sizeof(int) is on most systems "4"
> convert everything to integer(4)
>
> iv. Convert everything to the default integer, i.e. integer(4) or
> integer(8) depending whether -fdefault-integer-8 is used.
>
>
> While (iv) is the easiest, it somehow looks a bit arbitrary. I therefore
> would prefer either of (i) to (iii), with (iii) seeming to be the
> cleanest - at least if one uses the same kind as the C library does.
> (How does one ensure this?)

Additional question:

If a subroutine/function takes multiple integer arguments, how many calls 
should be considered valid?

 a) all possible combinations (i1/i1, i1/i2, i1/i4, ..., i8/i4, i8/i8)
 b) only those with matching types (i1/i1, i2/i2, i4/i4, i8/i8)


	Daniel


P.S. Removed gcc-patches as recipient, this is fortran-only.

Re: [patch, fortran] fixes for the EXIT intrinsic function (PR30933)

[Steve Kargl]

On Sat, Feb 24, 2007 at 07:19:55PM +0100, Daniel Franke wrote:
> On Saturday 24 February 2007 13:29:36 Tobias Burnus wrote:
> >
> > In any case which of the following option should we choose for those
> > non-time critical functions which call C library functions? Currently,
> > we have a mixure of (i), and (iv).
> >
> > i. Add i1, i2, and i16 versions to libgfortran
> >
> > ii. Convert kind > 8 to integer(8) and kind < 4 to integer(4)
> >
> > iii. As POSIX defines "exit(int)" and sizeof(int) is on most systems "4"
> > convert everything to integer(4)
> >
> > iv. Convert everything to the default integer, i.e. integer(4) or
> > integer(8) depending whether -fdefault-integer-8 is used.
> >
> > While (iv) is the easiest, it somehow looks a bit arbitrary. I therefore
> > would prefer either of (i) to (iii), with (iii) seeming to be the
> > cleanest - at least if one uses the same kind as the C library does.

I think it depends on the INTENT of the arguments.  If the args are
INTENT(IN), then (iv) is probably best.  A user calling FUNC(1_1)
will see that FUNC works, but under the hood _gfortran_func_i4 or
_gfortran_func_i8 is called depending on -fdefault-integer-8. 
For INTENT(INOUT) or INTENT(OUT) args, we'll probably need to go
with (i).  This may be accomplished by wrappers around the current
implementations for func_i4 and/or func_i8.

> > (How does one ensure this?)
> 
> Additional question:
> 
> If a subroutine/function takes multiple integer arguments, how many calls 
> should be considered valid?
> 
>  a) all possible combinations (i1/i1, i1/i2, i1/i4, ..., i8/i4, i8/i8)
>  b) only those with matching types (i1/i1, i2/i2, i4/i4, i8/i8)
> 

I prefer (b).  For many F95 intrinsics, this is the specified behavior,
and so we'll be consistent with the standard.

-- 
Steve

Re: [patch, fortran] fixes for the EXIT intrinsic function (PR30933)

[Thomas Koenig]

On Sat, Feb 24, 2007 at 01:29:36PM +0100, Tobias Burnus wrote:

> In any case which of the following option should we choose for those
> non-time critical functions which call C library functions?

Those are mostly non-standard intrinsics, which we support (with
some pain) out of a desire to remain compatible with g77.

For these, we could reject everything that g77 rejects (which
means to reject non-default integer types with an explicit
error message).

	Thomas

Re: [patch, fortran] fixes for the EXIT intrinsic function (PR30933)

[Brooks Moses]

:REVIEWMAIL:

Tobias Burnus wrote:
> The following patch does the following
> 
> a) Correct the dummy argument name; for "exit(status)" (is was "code"
> instead of "status") and for "new_line(A)" (it was "i").
> 
> b) It allows any integer kind for the exit subroutine. This is in the
> spirit of Fortran 2003 which also allows any kind for most of the
> intrinsic procedures. Status quo is to give a link-time error that
> exit_i1 etc. does not exist.
[...]

The patch for part (a) -- which should be just the intrinsic.c parts I 
quoted below, yes? -- is ok for trunk, although the changelog should 
mention the "new_line" and "exit" intrinsics.

My feeling at this point is that part (b) is something that should be 
solved in a consistent and systemic fashion, as was discussed in other 
threads after you posted this patch; thus, I don't think it's useful to 
fix it in a piecemeal manner now.

Thanks for working on this!

- Brooks


> fortran/
> 2007-02-23  Tobias Burnus  <burnus@net-b.de>
> 
> 	PR fortran/30933
> 	* intrinsic.c (add_functions): Correct name of dummy argument.
> 
> Index: gcc/fortran/intrinsic.c
> ===================================================================
> --- gcc/fortran/intrinsic.c	(revision 122263)
> +++ gcc/fortran/intrinsic.c	(working copy)
> @@ -1911,7 +1911,7 @@
>  
>    add_sym_1 ("new_line", NOT_ELEMENTAL, ACTUAL_NO, BT_CHARACTER, dc,
>  	     GFC_STD_F2003, gfc_check_new_line, gfc_simplify_new_line, NULL,
> -	     i, BT_CHARACTER, dc, REQUIRED);
> +	     a, BT_CHARACTER, dc, REQUIRED);
>  
>    add_sym_2 ("nint", ELEMENTAL, ACTUAL_YES, BT_INTEGER, di, GFC_STD_F77,
>  	     gfc_check_a_ikind, gfc_simplify_nint, gfc_resolve_nint,
> @@ -2457,7 +2457,7 @@
>  
>    add_sym_1s ("exit", 0, BT_UNKNOWN, 0, GFC_STD_GNU,
>  	      gfc_check_exit, NULL, gfc_resolve_exit,
> -	      c, BT_INTEGER, di, OPTIONAL);
> +	      st, BT_INTEGER, di, OPTIONAL);
>  
>    if ((gfc_option.allow_std & GFC_STD_GNU) || gfc_option.flag_all_intrinsics)
>      make_noreturn();