public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fortran -- clean up KILL
@ 2018-03-11 16:52 Steve Kargl
  2018-03-11 20:16 ` Janne Blomqvist
  0 siblings, 1 reply; 20+ messages in thread
From: Steve Kargl @ 2018-03-11 16:52 UTC (permalink / raw)
  To: fortran, gcc-patches

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

The attach patch cleans up KILL to match its
documentation.  In doing so, I have changed 
the argument keywords to consistently use 
pid and sig.  If no one objects, I intend to
commit this tomorrow.

2018-03-11  Steven G. Kargl  <kargls@gcc.gnu.org>

	* check.c (gfc_check_kill):  Check pid and sig are scalar.
	(gfc_check_kill_sub): Restrict kind to 4 and 8.
	* intrinsic.c (add_function): Sort keyword list.  Add pid and sig
	keywords for KILL.  Remove redundant *back="back" in favor of the
	original *bck="back".
	(add_subroutines): Sort keyword list.  Add pid and sig keywords
	for KILL.
	* intrinsic.texi: Fix documentation to consistently use pid and sig.
	* iresolve.c (gfc_resolve_kill): Kind can only be 4 or 8.  Choose the
	correct function.
	(gfc_resolve_rename_sub): Add comment.

-- 
Steve

[-- Attachment #2: kill.diff --]
[-- Type: text/x-diff, Size: 10215 bytes --]

Index: gcc/fortran/check.c
===================================================================
--- gcc/fortran/check.c	(revision 258433)
+++ gcc/fortran/check.c	(working copy)
@@ -2755,9 +2755,15 @@ gfc_check_kill (gfc_expr *pid, gfc_expr *sig)
   if (!type_check (pid, 0, BT_INTEGER))
     return false;
 
+  if (!scalar_check (pid, 0))
+    return false;
+
   if (!type_check (sig, 1, BT_INTEGER))
     return false;
 
+  if (!scalar_check (sig, 1))
+    return false;
+
   return true;
 }
 
@@ -2785,6 +2791,13 @@ gfc_check_kill_sub (gfc_expr *pid, gfc_expr *sig, gfc_
 
   if (!scalar_check (status, 2))
     return false;
+
+  if (status->ts.kind != 4 && status->ts.kind != 8)
+    {
+      gfc_error ("Invalid kind type parameter for STATUS at %L",
+		 &status->where);
+      return false;
+    }
 
   return true;
 }
Index: gcc/fortran/intrinsic.c
===================================================================
--- gcc/fortran/intrinsic.c	(revision 258433)
+++ gcc/fortran/intrinsic.c	(working copy)
@@ -1229,25 +1229,26 @@ set_attr_value (int n, ...)
 static void
 add_functions (void)
 {
-  /* Argument names as in the standard (to be used as argument keywords).  */
+  /* Argument names.  These are used as argument keywords and so need to
+    match the documentation.  Please keep this list in sorted order.  */
   const char
-    *a = "a", *f = "field", *pt = "pointer", *tg = "target",
-    *b = "b", *m = "matrix", *ma = "matrix_a", *mb = "matrix_b",
-    *c = "c", *n = "n", *ncopies= "ncopies", *pos = "pos", *bck = "back",
-    *i = "i", *v = "vector", *va = "vector_a", *vb = "vector_b",
-    *j = "j", *a1 = "a1", *fs = "fsource", *ts = "tsource",
-    *l = "l", *a2 = "a2", *mo = "mold", *ord = "order",
-    *p = "p", *ar = "array", *shp = "shape", *src = "source",
-    *r = "r", *bd = "boundary", *pad = "pad", *set = "set",
-    *s = "s", *dm = "dim", *kind = "kind", *msk = "mask",
-    *x = "x", *sh = "shift", *stg = "string", *ssg = "substring",
-    *y = "y", *sz = "size", *sta = "string_a", *stb = "string_b",
-    *z = "z", *ln = "len", *ut = "unit", *han = "handler",
-    *num = "number", *tm = "time", *nm = "name", *md = "mode",
-    *vl = "values", *p1 = "path1", *p2 = "path2", *com = "command",
-    *ca = "coarray", *sub = "sub", *dist = "distance", *failed="failed",
-    *c_ptr_1 = "c_ptr_1", *c_ptr_2 = "c_ptr_2", *back = "back",
-    *team = "team", *image = "image", *level = "level";
+    *a = "a", *a1 = "a1", *a2 = "a2", *ar = "array", *b = "b",
+    *bck = "back", *bd = "boundary", *c = "c", *c_ptr_1 = "c_ptr_1",
+    *c_ptr_2 = "c_ptr_2", *ca = "coarray", *com = "command",
+    *dist = "distance", *dm = "dim", *f = "field", *failed="failed",
+    *fs = "fsource", *han = "handler", *i = "i",
+    *image = "image", *j = "j", *kind = "kind",
+    *l = "l", *ln = "len", *level = "level", *m = "matrix", *ma = "matrix_a",
+    *mb = "matrix_b", *md = "mode", *mo = "mold", *msk = "mask",
+    *n = "n", *ncopies= "ncopies", *nm = "name", *num = "number",
+    *ord = "order", *p = "p", *p1 = "path1", *p2 = "path2",
+    *pad = "pad", *pid = "pid", *pos = "pos", *pt = "pointer",
+    *r = "r", *s = "s", *set = "set", *sh = "shift", *shp = "shape",
+    *sig = "sig", *src = "source", *ssg = "substring",
+    *sta = "string_a", *stb = "string_b", *stg = "string",
+    *sub = "sub", *sz = "size", *tg = "target", *team = "team", *tm = "time",
+    *ts = "tsource", *ut = "unit", *v = "vector", *va = "vector_a",
+    *vb = "vector_b", *vl = "values", *x = "x", *y = "y", *z = "z";
 
   int di, dr, dd, dl, dc, dz, ii;
 
@@ -2255,7 +2256,7 @@ add_functions (void)
 
   add_sym_2 ("kill", GFC_ISYM_KILL, CLASS_IMPURE, ACTUAL_NO, BT_INTEGER,
 	     di, GFC_STD_GNU, gfc_check_kill, NULL, gfc_resolve_kill,
-	     a, BT_INTEGER, di, REQUIRED, b, BT_INTEGER, di, REQUIRED);
+	     pid, BT_INTEGER, di, REQUIRED, sig, BT_INTEGER, di, REQUIRED);
 
   make_generic ("kill", GFC_ISYM_KILL, GFC_STD_GNU);
 
@@ -2471,7 +2472,7 @@ add_functions (void)
 	       gfc_check_minloc_maxloc, gfc_simplify_maxloc, gfc_resolve_maxloc,
 	       ar, BT_REAL, dr, REQUIRED, dm, BT_INTEGER, ii, OPTIONAL,
 	       msk, BT_LOGICAL, dl, OPTIONAL, kind, BT_INTEGER, di, OPTIONAL,
-	       back, BT_LOGICAL, dl, OPTIONAL);
+	       bck, BT_LOGICAL, dl, OPTIONAL);
 
   make_generic ("maxloc", GFC_ISYM_MAXLOC, GFC_STD_F95);
 
@@ -2548,7 +2549,7 @@ add_functions (void)
 	       gfc_check_minloc_maxloc, gfc_simplify_minloc, gfc_resolve_minloc,
 	       ar, BT_REAL, dr, REQUIRED, dm, BT_INTEGER, ii, OPTIONAL,
 	       msk, BT_LOGICAL, dl, OPTIONAL, kind, BT_INTEGER, di, OPTIONAL,
-	       back, BT_LOGICAL, dl, OPTIONAL);
+	       bck, BT_LOGICAL, dl, OPTIONAL);
 
   make_generic ("minloc", GFC_ISYM_MINLOC, GFC_STD_F95);
 
@@ -3301,20 +3302,21 @@ add_functions (void)
 static void
 add_subroutines (void)
 {
-  /* Argument names as in the standard (to be used as argument keywords).  */
-  const char
-    *a = "a", *h = "harvest", *dt = "date", *vl = "values", *pt = "put",
-    *c = "count", *tm = "time", *tp = "topos", *gt = "get",
-    *t = "to", *zn = "zone", *fp = "frompos", *cm = "count_max",
-    *f = "from", *sz = "size", *ln = "len", *cr = "count_rate",
-    *com = "command", *length = "length", *st = "status",
-    *val = "value", *num = "number", *name = "name",
-    *trim_name = "trim_name", *ut = "unit", *han = "handler",
-    *sec = "seconds", *res = "result", *of = "offset", *md = "mode",
-    *whence = "whence", *pos = "pos", *ptr = "ptr", *p1 = "path1",
-    *p2 = "path2", *msk = "mask", *old = "old", *result_image = "result_image",
-    *stat = "stat", *errmsg = "errmsg";
-
+  /* Argument names.  These are used as argument keywords and so need to
+     match the documentation.  Please keep this list in sorted order.  */
+  static const char
+    *a = "a", *c = "count", *cm = "count_max", *com = "command",
+    *cr = "count_rate", *dt = "date", *errmsg = "errmsg", *f = "from",
+    *fp = "frompos", *gt = "get", *h = "harvest", *han = "handler",
+    *length = "length", *ln = "len", *md = "mode", *msk = "mask",
+    *name = "name", *num = "number", *of = "offset", *old = "old",
+    *p1 = "path1", *p2 = "path2", *pid = "pid", *pos = "pos",
+    *pt = "put", *ptr = "ptr", *res = "result",
+    *result_image = "result_image", *sec = "seconds", *sig = "sig",
+    *st = "status", *stat = "stat", *sz = "size", *t = "to",
+    *tm = "time", *tp = "topos", *trim_name = "trim_name", *ut = "unit",
+    *val = "value", *vl = "values", *whence = "whence", *zn = "zone";
+ 
   int di, dr, dc, dl, ii;
 
   di = gfc_default_integer_kind;
@@ -3723,8 +3725,8 @@ add_subroutines (void)
 
   add_sym_3s ("kill", GFC_ISYM_KILL, CLASS_IMPURE, BT_UNKNOWN, 0, GFC_STD_GNU,
 	      gfc_check_kill_sub, NULL, gfc_resolve_kill_sub,
-	      c, BT_INTEGER, di, REQUIRED, INTENT_IN,
-	      val, BT_INTEGER, di, REQUIRED, INTENT_IN,
+	      pid, BT_INTEGER, di, REQUIRED, INTENT_IN,
+	      sig, BT_INTEGER, di, REQUIRED, INTENT_IN,
 	      st, BT_INTEGER, di, OPTIONAL, INTENT_OUT);
 
   add_sym_3s ("link", GFC_ISYM_LINK, CLASS_IMPURE, BT_UNKNOWN, 0, GFC_STD_GNU,
Index: gcc/fortran/intrinsic.texi
===================================================================
--- gcc/fortran/intrinsic.texi	(revision 258433)
+++ gcc/fortran/intrinsic.texi	(working copy)
@@ -8715,36 +8715,39 @@ end program test_itime
 @table @asis
 @item @emph{Description}:
 @item @emph{Standard}:
-Sends the signal specified by @var{SIGNAL} to the process @var{PID}.
+Sends the signal specified by @var{SIG} to the process @var{PID}.
 See @code{kill(2)}.
 
-This intrinsic is provided in both subroutine and function forms; however,
-only one form can be used in any given program unit.
+This intrinsic is provided in both subroutine and function forms;
+however,  only one form can be used in any given program unit.
 
 @item @emph{Class}:
 Subroutine, function
 
 @item @emph{Syntax}:
 @multitable @columnfractions .80
-@item @code{CALL KILL(C, VALUE [, STATUS])}
-@item @code{STATUS = KILL(C, VALUE)}
+@item @code{CALL KILL(PID, SIG [, STATUS])}
+@item @code{STATUS = KILL(PID, SIG)}
 @end multitable
 
 @item @emph{Arguments}:
 @multitable @columnfractions .15 .70
-@item @var{C} @tab Shall be a scalar @code{INTEGER}, with
+@item @var{PID} @tab Shall be a scalar @code{INTEGER} with
 @code{INTENT(IN)}
-@item @var{VALUE} @tab Shall be a scalar @code{INTEGER}, with
+@item @var{SIG} @tab Shall be a scalar @code{INTEGER} with
 @code{INTENT(IN)}
-@item @var{STATUS} @tab (Optional) status flag of type @code{INTEGER(4)} or
-@code{INTEGER(8)}. Returns 0 on success, or a system-specific error code
-otherwise.
+@item @var{STATUS} @tab [Subroutine](Optional) status flag of type
+@code{INTEGER(4)} or @code{INTEGER(8)}.
+Returns 0 on success; otherwise a system-specific error code is returned.
+@item @var{STATUS} @tab [Function] The kind type parameter is that of
+@code{pid} if @code{pid} is of type @code{INTEGER(4)} or @code{INTEGER(8)};
+otherwise, it is default integer kind.
+Returns 0 on success; otherwise a system-specific error code is returned.
 @end multitable
 
 @item @emph{See also}:
 @ref{ABORT}, @ref{EXIT}
 @end table
-
 
 
 @node KIND
Index: gcc/fortran/iresolve.c
===================================================================
--- gcc/fortran/iresolve.c	(revision 258433)
+++ gcc/fortran/iresolve.c	(working copy)
@@ -1492,11 +1492,14 @@ gfc_resolve_ishftc (gfc_expr *f, gfc_expr *i, gfc_expr
 
 
 void
-gfc_resolve_kill (gfc_expr *f, gfc_expr *p ATTRIBUTE_UNUSED,
-		  gfc_expr *s ATTRIBUTE_UNUSED)
+gfc_resolve_kill (gfc_expr *f, gfc_expr *pid,
+		  gfc_expr *sig ATTRIBUTE_UNUSED)
 {
   f->ts.type = BT_INTEGER;
-  f->ts.kind = gfc_default_integer_kind;
+  if (pid->ts.kind == 4 || pid->ts.kind == 8)
+    f->ts.kind = pid->ts.kind;
+  else
+    f->ts.kind = gfc_default_integer_kind;
   f->value.function.name = gfc_get_string (PREFIX ("kill_i%d"), f->ts.kind);
 }
 
@@ -3446,6 +3449,7 @@ gfc_resolve_rename_sub (gfc_code *c)
   const char *name;
   int kind;
 
+  /* Find the type of status.  If not present use default integer kind.  */
   if (c->ext.actual->next->next->expr != NULL)
     kind = c->ext.actual->next->next->expr->ts.kind;
   else

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-11 16:52 [PATCH] Fortran -- clean up KILL Steve Kargl
@ 2018-03-11 20:16 ` Janne Blomqvist
  2018-03-11 20:42   ` Steve Kargl
  0 siblings, 1 reply; 20+ messages in thread
From: Janne Blomqvist @ 2018-03-11 20:16 UTC (permalink / raw)
  To: Steve Kargl; +Cc: Fortran List, GCC Patches

On Sun, Mar 11, 2018 at 6:52 PM, Steve Kargl
<sgk@troutmask.apl.washington.edu> wrote:
> The attach patch cleans up KILL to match its
> documentation.  In doing so, I have changed
> the argument keywords to consistently use
> pid and sig.  If no one objects, I intend to
> commit this tomorrow.
>
> 2018-03-11  Steven G. Kargl  <kargls@gcc.gnu.org>
>
>         * check.c (gfc_check_kill):  Check pid and sig are scalar.
>         (gfc_check_kill_sub): Restrict kind to 4 and 8.
>         * intrinsic.c (add_function): Sort keyword list.  Add pid and sig
>         keywords for KILL.  Remove redundant *back="back" in favor of the
>         original *bck="back".
>         (add_subroutines): Sort keyword list.  Add pid and sig keywords
>         for KILL.
>         * intrinsic.texi: Fix documentation to consistently use pid and sig.
>         * iresolve.c (gfc_resolve_kill): Kind can only be 4 or 8.  Choose the
>         correct function.
>         (gfc_resolve_rename_sub): Add comment.

The patch per se looks fine, but while you're at it, it would be nice
to get rid of all but one of the libgfortran entry points and do the
typecasting etc. in the frontend instead..

-- 
Janne Blomqvist

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-11 20:16 ` Janne Blomqvist
@ 2018-03-11 20:42   ` Steve Kargl
  2018-03-12 16:56     ` Janne Blomqvist
  0 siblings, 1 reply; 20+ messages in thread
From: Steve Kargl @ 2018-03-11 20:42 UTC (permalink / raw)
  To: Janne Blomqvist; +Cc: Fortran List, GCC Patches

On Sun, Mar 11, 2018 at 10:16:01PM +0200, Janne Blomqvist wrote:
> On Sun, Mar 11, 2018 at 6:52 PM, Steve Kargl
> <sgk@troutmask.apl.washington.edu> wrote:
> > The attach patch cleans up KILL to match its
> > documentation.  In doing so, I have changed
> > the argument keywords to consistently use
> > pid and sig.  If no one objects, I intend to
> > commit this tomorrow.
> >
> > 2018-03-11  Steven G. Kargl  <kargls@gcc.gnu.org>
> >
> >         * check.c (gfc_check_kill):  Check pid and sig are scalar.
> >         (gfc_check_kill_sub): Restrict kind to 4 and 8.
> >         * intrinsic.c (add_function): Sort keyword list.  Add pid and sig
> >         keywords for KILL.  Remove redundant *back="back" in favor of the
> >         original *bck="back".
> >         (add_subroutines): Sort keyword list.  Add pid and sig keywords
> >         for KILL.
> >         * intrinsic.texi: Fix documentation to consistently use pid and sig.
> >         * iresolve.c (gfc_resolve_kill): Kind can only be 4 or 8.  Choose the
> >         correct function.
> >         (gfc_resolve_rename_sub): Add comment.
> 
> The patch per se looks fine, but while you're at it, it would be nice
> to get rid of all but one of the libgfortran entry points and do the
> typecasting etc. in the frontend instead..
> 

Thanks.  Note, the documentation specifically states that
only INTEGER(4) and INTEGER(8) are supported, so there is
only 2 entry points for the function and 4(?) for the 
subroutine version.  nm shows 

00000000 T _gfortran_kill_i4
00000000 T _gfortran_kill_i4_sub
00000000 T _gfortran_kill_i8
00000000 T _gfortran_kill_i8_sub
00000000 T _gfortrani_kill_i4_sub
00000000 T _gfortrani_kill_i8_sub

I don't recall what the difference is between _gfortran_
and _gfortrani_. 

I suppose we could remove the restriction to INTEGER(4) and
INTEGER(8), but I don't see any reason to do so.  INTEGER(1)
is too limited given that at least on FreeBSD the lower pid's
correspond to system processes.  Using 'ps ax' suggests that
the first 1000 or so processes are from system start up.
INTEGER(16) (if supported) seems to be overkill in that I
doubt any OS uses a 64-bit pid_t.  

-- 
Steve

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-11 20:42   ` Steve Kargl
@ 2018-03-12 16:56     ` Janne Blomqvist
  2018-03-12 17:37       ` Steve Kargl
  0 siblings, 1 reply; 20+ messages in thread
From: Janne Blomqvist @ 2018-03-12 16:56 UTC (permalink / raw)
  To: Steve Kargl; +Cc: Fortran List, GCC Patches

On Sun, Mar 11, 2018 at 10:42 PM, Steve Kargl
<sgk@troutmask.apl.washington.edu> wrote:
> On Sun, Mar 11, 2018 at 10:16:01PM +0200, Janne Blomqvist wrote:
>> On Sun, Mar 11, 2018 at 6:52 PM, Steve Kargl
>> <sgk@troutmask.apl.washington.edu> wrote:
>> > The attach patch cleans up KILL to match its
>> > documentation.  In doing so, I have changed
>> > the argument keywords to consistently use
>> > pid and sig.  If no one objects, I intend to
>> > commit this tomorrow.
>> >
>> > 2018-03-11  Steven G. Kargl  <kargls@gcc.gnu.org>
>> >
>> >         * check.c (gfc_check_kill):  Check pid and sig are scalar.
>> >         (gfc_check_kill_sub): Restrict kind to 4 and 8.
>> >         * intrinsic.c (add_function): Sort keyword list.  Add pid and sig
>> >         keywords for KILL.  Remove redundant *back="back" in favor of the
>> >         original *bck="back".
>> >         (add_subroutines): Sort keyword list.  Add pid and sig keywords
>> >         for KILL.
>> >         * intrinsic.texi: Fix documentation to consistently use pid and sig.
>> >         * iresolve.c (gfc_resolve_kill): Kind can only be 4 or 8.  Choose the
>> >         correct function.
>> >         (gfc_resolve_rename_sub): Add comment.
>>
>> The patch per se looks fine, but while you're at it, it would be nice
>> to get rid of all but one of the libgfortran entry points and do the
>> typecasting etc. in the frontend instead..
>>
>
> Thanks.  Note, the documentation specifically states that
> only INTEGER(4) and INTEGER(8) are supported, so there is
> only 2 entry points for the function and 4(?) for the
> subroutine version.  nm shows
>
> 00000000 T _gfortran_kill_i4
> 00000000 T _gfortran_kill_i4_sub
> 00000000 T _gfortran_kill_i8
> 00000000 T _gfortran_kill_i8_sub
> 00000000 T _gfortrani_kill_i4_sub
> 00000000 T _gfortrani_kill_i8_sub
>
> I don't recall what the difference is between _gfortran_
> and _gfortrani_.

IIRC, the "i" stands for internal, it's symbols called internally in
gfortran and are not externally visible in the .so (though you can
still see them in the .a). So we have 4 external symbols that are part
of the libgfortran API.

> I suppose we could remove the restriction to INTEGER(4) and
> INTEGER(8), but I don't see any reason to do so.  INTEGER(1)
> is too limited given that at least on FreeBSD the lower pid's
> correspond to system processes.  Using 'ps ax' suggests that
> the first 1000 or so processes are from system start up.
> INTEGER(16) (if supported) seems to be overkill in that I
> doubt any OS uses a 64-bit pid_t.

I'm not sure there's any particular reason for the kind=4,8
restriction except that "it's the same restriction g77 had". And AFAIK
there are no systems with a 64-bit pid_t, so a plain int should be
enough. So it should suffice to have a single libgfortran function,
say with the prototype

int _gfortran_kill (int pid, int sig);

and the frontend would take care of whatever massaging to handle
whether it's called as a function or subroutine, and whatever
typecasting is necessary. Whether we want to limit it to kind=4,8 or
allow arbitrary kinds I don't have any particularly strong opinion on.



-- 
Janne Blomqvist

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-12 16:56     ` Janne Blomqvist
@ 2018-03-12 17:37       ` Steve Kargl
  2018-03-12 19:05         ` Janne Blomqvist
  0 siblings, 1 reply; 20+ messages in thread
From: Steve Kargl @ 2018-03-12 17:37 UTC (permalink / raw)
  To: Janne Blomqvist; +Cc: Fortran List, GCC Patches

On Mon, Mar 12, 2018 at 06:56:24PM +0200, Janne Blomqvist wrote:
> On Sun, Mar 11, 2018 at 10:42 PM, Steve Kargl
> > I suppose we could remove the restriction to INTEGER(4) and
> > INTEGER(8), but I don't see any reason to do so.  INTEGER(1)
> > is too limited given that at least on FreeBSD the lower pid's
> > correspond to system processes.  Using 'ps ax' suggests that
> > the first 1000 or so processes are from system start up.
> > INTEGER(16) (if supported) seems to be overkill in that I
> > doubt any OS uses a 64-bit pid_t.
> 
> I'm not sure there's any particular reason for the kind=4,8
> restriction except that "it's the same restriction g77 had". And AFAIK
> there are no systems with a 64-bit pid_t, so a plain int should be
> enough. So it should suffice to have a single libgfortran function,
> say with the prototype
> 
> int _gfortran_kill (int pid, int sig);
> 
> and the frontend would take care of whatever massaging to handle
> whether it's called as a function or subroutine, and whatever
> typecasting is necessary. Whether we want to limit it to kind=4,8 or
> allow arbitrary kinds I don't have any particularly strong opinion on.
> 

Keeping kind=4,8 simply makes life easier for people who
use -fdefault-integer-8.  It also makes our life's easier.
Where do you want the conversion to occur?  We don't have
a conv_intrinsic_kill in trans-intrinsic.c.  The return type
is documented to be that of pid, so we need to convert pid
and sig to int and then convert the presumably returned int
to type of pid. 

BTW, I'm not too familiar with all the nuances of symbol
versioning, but I thought we needed to drag the 6 exported
library symbols along forever.

-- 
Steve

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-12 17:37       ` Steve Kargl
@ 2018-03-12 19:05         ` Janne Blomqvist
  2018-03-13  4:08           ` Steve Kargl
  0 siblings, 1 reply; 20+ messages in thread
From: Janne Blomqvist @ 2018-03-12 19:05 UTC (permalink / raw)
  To: Steve Kargl; +Cc: Fortran List, GCC Patches

On Mon, Mar 12, 2018 at 7:37 PM, Steve Kargl
<sgk@troutmask.apl.washington.edu> wrote:
> On Mon, Mar 12, 2018 at 06:56:24PM +0200, Janne Blomqvist wrote:
>> On Sun, Mar 11, 2018 at 10:42 PM, Steve Kargl
>> > I suppose we could remove the restriction to INTEGER(4) and
>> > INTEGER(8), but I don't see any reason to do so.  INTEGER(1)
>> > is too limited given that at least on FreeBSD the lower pid's
>> > correspond to system processes.  Using 'ps ax' suggests that
>> > the first 1000 or so processes are from system start up.
>> > INTEGER(16) (if supported) seems to be overkill in that I
>> > doubt any OS uses a 64-bit pid_t.
>>
>> I'm not sure there's any particular reason for the kind=4,8
>> restriction except that "it's the same restriction g77 had". And AFAIK
>> there are no systems with a 64-bit pid_t, so a plain int should be
>> enough. So it should suffice to have a single libgfortran function,
>> say with the prototype
>>
>> int _gfortran_kill (int pid, int sig);
>>
>> and the frontend would take care of whatever massaging to handle
>> whether it's called as a function or subroutine, and whatever
>> typecasting is necessary. Whether we want to limit it to kind=4,8 or
>> allow arbitrary kinds I don't have any particularly strong opinion on.
>>
>
> Keeping kind=4,8 simply makes life easier for people who
> use -fdefault-integer-8.

Yes, I understand that -fdefault-integer-8 (or whatever the equivalent
option was called on g77) is the original motivation. Like I said, I
don't have any particular opinion on whether we should keep that
restriction or not. On one hand, more recent versions of the standard
has lifted restrictions that integer intrinsic arguments have to be of
default kind in many cases, OTOH KILL is not a standard intrinsic but
something inherited from g77. So, meh.

> BTW, I'm not too familiar with all the nuances of symbol
> versioning, but I thought we needed to drag the 6 exported
> library symbols along forever.

No, when we bump the major .so version number, as we have already done
for GCC 8, the clock resets and we can remove unnecessary stuff. The
symbol versioning stuff is only useful as long as we keep the same
major .so version number.


-- 
Janne Blomqvist

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-12 19:05         ` Janne Blomqvist
@ 2018-03-13  4:08           ` Steve Kargl
  2018-03-13 19:49             ` Janne Blomqvist
  0 siblings, 1 reply; 20+ messages in thread
From: Steve Kargl @ 2018-03-13  4:08 UTC (permalink / raw)
  To: Janne Blomqvist; +Cc: Fortran List, GCC Patches

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

On Mon, Mar 12, 2018 at 09:05:09PM +0200, Janne Blomqvist wrote:
> 
> Yes, I understand that -fdefault-integer-8 (or whatever the equivalent
> option was called on g77) is the original motivation. Like I said, I
> don't have any particular opinion on whether we should keep that
> restriction or not. On one hand, more recent versions of the standard
> has lifted restrictions that integer intrinsic arguments have to be of
> default kind in many cases, OTOH KILL is not a standard intrinsic but
> something inherited from g77. So, meh.

The Fortran standard specifically permits a Fortran processor to 
supply additional subprograms not contained in the Fortran standard.
I personally can't any person person using INTEGER(1) or even 
INTEGER(2) with KILL as pid_t on typical modern OS's exceeds HUGE()
in those types.  My original patch simply fixed KILL to actually
conform to its documentation.  But is this what you want

2018-03-12  Steven G. Kargl  <kargl@gcc.gnu.org>

	* check.c (gfc_check_kill_sub):  Remove check for INTEGER(4) or (8).
	* intrinsic.c (add_functions): Remove reference to gfc_resolve_kill.
	(add_subroutines): Remove reference to gfc_resolve_kill_sub.
	* intrinsic.texi: Update documentation.
	* iresolve.c (gfc_resolve_kill, gfc_resolve_kill_sub): Remove.
	* trans-decl.c (gfc_build_intrinsic_function_decls):  Add gfor_fndecl_kill
	and gfor_fndecl_kill_sub
	* trans-intrinsic.c (conv_intrinsic_kill, conv_intrinsic_kill_sub): new
	functions.
	(gfc_conv_intrinsic_function): Use conv_intrinsic_kill.
        (gfc_conv_intrinsic_subroutine): Use conv_intrinsic_kill_sub.
	* trans.h: Declare gfor_fndecl_kill and gfor_fndecl_kill_sub.
 
2018-03-12  Steven G. Kargl  <kargl@gcc.gnu.org>

	* libgfortran/gfortran.map: Remove _gfortran_kill_i4, _gfortran_kill_i4_sub,
	_gfortran_kill_i8, and _gfortran_kill_i8_sub.  Add _gfortran_kill and
	_gfortran_kill_sub.
	* libgfortran/intrinsics/kill.c: Eliminate _gfortran_kill_i4,
	_gfortran_kill_i4_sub, _gfortran_kill_i8, and _gfortran_kill_i8_sub.
	Add _gfortran_kill and _gfortran_kill_sub.

-- 
Steve

[-- Attachment #2: kill.diff --]
[-- Type: text/x-diff, Size: 12608 bytes --]

Index: gcc/fortran/check.c
===================================================================
--- gcc/fortran/check.c	(revision 258468)
+++ gcc/fortran/check.c	(working copy)
@@ -2783,20 +2783,13 @@ gfc_check_kill_sub (gfc_expr *pid, gfc_expr *sig, gfc_
   if (!scalar_check (sig, 1))
     return false;
 
-  if (status == NULL)
-    return true;
-
-  if (!type_check (status, 2, BT_INTEGER))
-    return false;
-
-  if (!scalar_check (status, 2))
-    return false;
-
-  if (status->ts.kind != 4 && status->ts.kind != 8)
+  if (status)
     {
-      gfc_error ("Invalid kind type parameter for STATUS at %L",
-		 &status->where);
-      return false;
+      if (!type_check (status, 2, BT_INTEGER))
+	return false;
+
+      if (!scalar_check (status, 2))
+	return false;
     }
 
   return true;
Index: gcc/fortran/intrinsic.c
===================================================================
--- gcc/fortran/intrinsic.c	(revision 258468)
+++ gcc/fortran/intrinsic.c	(working copy)
@@ -2255,7 +2255,7 @@ add_functions (void)
   make_generic ("ishftc", GFC_ISYM_ISHFTC, GFC_STD_F95);
 
   add_sym_2 ("kill", GFC_ISYM_KILL, CLASS_IMPURE, ACTUAL_NO, BT_INTEGER,
-	     di, GFC_STD_GNU, gfc_check_kill, NULL, gfc_resolve_kill,
+	     di, GFC_STD_GNU, gfc_check_kill, NULL, NULL,
 	     pid, BT_INTEGER, di, REQUIRED, sig, BT_INTEGER, di, REQUIRED);
 
   make_generic ("kill", GFC_ISYM_KILL, GFC_STD_GNU);
@@ -3724,7 +3724,7 @@ add_subroutines (void)
 	      st, BT_INTEGER, di, OPTIONAL, INTENT_OUT);
 
   add_sym_3s ("kill", GFC_ISYM_KILL, CLASS_IMPURE, BT_UNKNOWN, 0, GFC_STD_GNU,
-	      gfc_check_kill_sub, NULL, gfc_resolve_kill_sub,
+	      gfc_check_kill_sub, NULL, NULL,
 	      pid, BT_INTEGER, di, REQUIRED, INTENT_IN,
 	      sig, BT_INTEGER, di, REQUIRED, INTENT_IN,
 	      st, BT_INTEGER, di, OPTIONAL, INTENT_OUT);
Index: gcc/fortran/intrinsic.texi
===================================================================
--- gcc/fortran/intrinsic.texi	(revision 258468)
+++ gcc/fortran/intrinsic.texi	(working copy)
@@ -8719,7 +8719,7 @@ Sends the signal specified by @var{SIG} to the process
 See @code{kill(2)}.
 
 This intrinsic is provided in both subroutine and function forms;
-however,  only one form can be used in any given program unit.
+however, only one form can be used in any given program unit.
 
 @item @emph{Class}:
 Subroutine, function
@@ -8732,16 +8732,13 @@ Subroutine, function
 
 @item @emph{Arguments}:
 @multitable @columnfractions .15 .70
-@item @var{PID} @tab Shall be a scalar @code{INTEGER} with
-@code{INTENT(IN)}
-@item @var{SIG} @tab Shall be a scalar @code{INTEGER} with
-@code{INTENT(IN)}
-@item @var{STATUS} @tab [Subroutine](Optional) status flag of type
-@code{INTEGER(4)} or @code{INTEGER(8)}.
+@item @var{PID} @tab Shall be a scalar @code{INTEGER} with @code{INTENT(IN)}.
+@item @var{SIG} @tab Shall be a scalar @code{INTEGER} with @code{INTENT(IN)}.
+@item @var{STATUS} @tab [Subroutine](Optional)
+Shall be a scalar @code{INTEGER}.
 Returns 0 on success; otherwise a system-specific error code is returned.
 @item @var{STATUS} @tab [Function] The kind type parameter is that of
-@code{pid} if @code{pid} is of type @code{INTEGER(4)} or @code{INTEGER(8)};
-otherwise, it is default integer kind.
+@code{pid}.
 Returns 0 on success; otherwise a system-specific error code is returned.
 @end multitable
 
Index: gcc/fortran/iresolve.c
===================================================================
--- gcc/fortran/iresolve.c	(revision 258468)
+++ gcc/fortran/iresolve.c	(working copy)
@@ -1492,19 +1492,6 @@ gfc_resolve_ishftc (gfc_expr *f, gfc_expr *i, gfc_expr
 
 
 void
-gfc_resolve_kill (gfc_expr *f, gfc_expr *pid,
-		  gfc_expr *sig ATTRIBUTE_UNUSED)
-{
-  f->ts.type = BT_INTEGER;
-  if (pid->ts.kind == 4 || pid->ts.kind == 8)
-    f->ts.kind = pid->ts.kind;
-  else
-    f->ts.kind = gfc_default_integer_kind;
-  f->value.function.name = gfc_get_string (PREFIX ("kill_i%d"), f->ts.kind);
-}
-
-
-void
 gfc_resolve_lbound (gfc_expr *f, gfc_expr *array, gfc_expr *dim, gfc_expr *kind)
 {
   resolve_bound (f, array, dim, kind, "__lbound", false);
@@ -3456,22 +3443,6 @@ gfc_resolve_rename_sub (gfc_code *c)
     kind = gfc_default_integer_kind;
 
   name = gfc_get_string (PREFIX ("rename_i%d_sub"), kind);
-  c->resolved_sym = gfc_get_intrinsic_sub_symbol (name);
-}
-
-
-void
-gfc_resolve_kill_sub (gfc_code *c)
-{
-  const char *name;
-  int kind;
-
-  if (c->ext.actual->next->next->expr != NULL)
-    kind = c->ext.actual->next->next->expr->ts.kind;
-  else
-    kind = gfc_default_integer_kind;
-
-  name = gfc_get_string (PREFIX ("kill_i%d_sub"), kind);
   c->resolved_sym = gfc_get_intrinsic_sub_symbol (name);
 }
 
Index: gcc/fortran/trans-decl.c
===================================================================
--- gcc/fortran/trans-decl.c	(revision 258468)
+++ gcc/fortran/trans-decl.c	(working copy)
@@ -123,7 +123,6 @@ tree gfor_fndecl_system_clock8;
 tree gfor_fndecl_ieee_procedure_entry;
 tree gfor_fndecl_ieee_procedure_exit;
 
-
 /* Coarray run-time library function decls.  */
 tree gfor_fndecl_caf_init;
 tree gfor_fndecl_caf_finalize;
@@ -215,7 +214,10 @@ tree gfor_fndecl_convert_char4_to_char1;
 tree gfor_fndecl_size0;
 tree gfor_fndecl_size1;
 tree gfor_fndecl_iargc;
+tree gfor_fndecl_kill;
+tree gfor_fndecl_kill_sub;
 
+
 /* Intrinsic functions implemented in Fortran.  */
 tree gfor_fndecl_sc_kind;
 tree gfor_fndecl_si_kind;
@@ -3494,6 +3496,14 @@ gfc_build_intrinsic_function_decls (void)
   gfor_fndecl_iargc = gfc_build_library_function_decl (
 	get_identifier (PREFIX ("iargc")), gfc_int4_type_node, 0);
   TREE_NOTHROW (gfor_fndecl_iargc) = 1;
+
+  gfor_fndecl_kill_sub = gfc_build_library_function_decl (
+	get_identifier (PREFIX ("kill_sub")), void_type_node,
+	3, gfc_int4_type_node, gfc_int4_type_node, gfc_pint4_type_node);
+
+  gfor_fndecl_kill = gfc_build_library_function_decl (
+	get_identifier (PREFIX ("kill")), gfc_int4_type_node,
+	2, gfc_int4_type_node, gfc_int4_type_node);
 }
 
 
Index: gcc/fortran/trans-intrinsic.c
===================================================================
--- gcc/fortran/trans-intrinsic.c	(revision 258468)
+++ gcc/fortran/trans-intrinsic.c	(working copy)
@@ -8115,6 +8115,85 @@ gfc_conv_intrinsic_iargc (gfc_se * se, gfc_expr * expr
 }
 
 
+/* Generate code for the KILL intrinsic.  */
+
+static void
+conv_intrinsic_kill (gfc_se *se, gfc_expr *expr)
+{
+  tree *args;
+  tree int4_type_node = gfc_get_int_type (4);
+  tree pid;
+  tree sig;
+  tree tmp;
+  unsigned int num_args;
+
+  num_args = gfc_intrinsic_argument_list_length (expr);
+  args = XALLOCAVEC (tree, num_args);
+  gfc_conv_intrinsic_function_args (se, expr, args, num_args);
+
+  /* Convert PID to a INTEGER(4) entity.  */
+  pid = convert (int4_type_node, args[0]);
+
+  /* Convert SIG to a INTEGER(4) entity.  */
+  sig = convert (int4_type_node, args[1]);
+
+  tmp = build_call_expr_loc (input_location, gfor_fndecl_kill, 2, pid, sig);
+
+  se->expr = fold_convert (TREE_TYPE (args[0]), tmp);
+}
+
+
+static tree
+conv_intrinsic_kill_sub (gfc_code *code)
+{
+  stmtblock_t block;
+  gfc_se se, se_stat;
+  tree int4_type_node = gfc_get_int_type (4);
+  tree pid;
+  tree sig;
+  tree statp;
+  tree tmp;
+
+  /* Make the function call.  */
+  gfc_init_block (&block);
+  gfc_init_se (&se, NULL);
+
+  /* Convert PID to a INTEGER(4) entity.  */
+  gfc_conv_expr (&se, code->ext.actual->expr);
+  gfc_add_block_to_block (&block, &se.pre);
+  pid = fold_convert (int4_type_node, gfc_evaluate_now (se.expr, &block));
+  gfc_add_block_to_block (&block, &se.post);
+
+  /* Convert SIG to a INTEGER(4) entity.  */
+  gfc_conv_expr (&se, code->ext.actual->next->expr);
+  gfc_add_block_to_block (&block, &se.pre);
+  sig = fold_convert (int4_type_node, gfc_evaluate_now (se.expr, &block));
+  gfc_add_block_to_block (&block, &se.post);
+
+  /* Deal with an optional STATUS.  */
+  if (code->ext.actual->next->next->expr)
+    {
+      gfc_init_se (&se_stat, NULL);
+      gfc_conv_expr (&se_stat, code->ext.actual->next->next->expr);
+      statp = gfc_create_var (gfc_get_int_type (4), "_statp");
+    }
+  else
+    statp = NULL_TREE;
+
+  tmp = build_call_expr_loc (input_location, gfor_fndecl_kill_sub, 3, pid, sig,
+	statp ? gfc_build_addr_expr (NULL_TREE, statp) : null_pointer_node);
+
+  gfc_add_expr_to_block (&block, tmp);
+
+  if (statp && statp != se_stat.expr)
+    gfc_add_modify (&block, se_stat.expr,
+		    fold_convert (TREE_TYPE (se_stat.expr), statp));
+
+  return gfc_finish_block (&block);
+}
+
+
+
 /* The loc intrinsic returns the address of its argument as
    gfc_index_integer_kind integer.  */
 
@@ -8962,6 +9041,10 @@ gfc_conv_intrinsic_function (gfc_se * se, gfc_expr * e
       gfc_conv_intrinsic_conjg (se, expr);
       break;
 
+    case GFC_ISYM_KILL:
+      conv_intrinsic_kill (se, expr);
+      break;
+
     case GFC_ISYM_COUNT:
       gfc_conv_intrinsic_count (se, expr);
       break;
@@ -9344,7 +9427,6 @@ gfc_conv_intrinsic_function (gfc_se * se, gfc_expr * e
     case GFC_ISYM_GETPID:
     case GFC_ISYM_GETUID:
     case GFC_ISYM_HOSTNM:
-    case GFC_ISYM_KILL:
     case GFC_ISYM_IERRNO:
     case GFC_ISYM_IRAND:
     case GFC_ISYM_ISATTY:
@@ -10819,6 +10901,10 @@ gfc_conv_intrinsic_subroutine (gfc_code *code)
 
     case GFC_ISYM_FREE:
       res = conv_intrinsic_free (code);
+      break;
+
+    case GFC_ISYM_KILL:
+      res = conv_intrinsic_kill_sub (code);
       break;
 
     case GFC_ISYM_SYSTEM_CLOCK:
Index: gcc/fortran/trans.h
===================================================================
--- gcc/fortran/trans.h	(revision 258468)
+++ gcc/fortran/trans.h	(working copy)
@@ -903,6 +903,8 @@ extern GTY(()) tree gfor_fndecl_convert_char4_to_char1
 extern GTY(()) tree gfor_fndecl_size0;
 extern GTY(()) tree gfor_fndecl_size1;
 extern GTY(()) tree gfor_fndecl_iargc;
+extern GTY(()) tree gfor_fndecl_kill;
+extern GTY(()) tree gfor_fndecl_kill_sub;
 
 /* Implemented in Fortran.  */
 extern GTY(()) tree gfor_fndecl_sc_kind;
Index: libgfortran/gfortran.map
===================================================================
--- libgfortran/gfortran.map	(revision 258468)
+++ libgfortran/gfortran.map	(working copy)
@@ -318,10 +318,8 @@ GFORTRAN_8 {
     _gfortran_ishftc8;
     _gfortran_itime_i4;
     _gfortran_itime_i8;
-    _gfortran_kill_i4;
-    _gfortran_kill_i4_sub;
-    _gfortran_kill_i8;
-    _gfortran_kill_i8_sub;
+    _gfortran_kill;
+    _gfortran_kill_sub;
     _gfortran_link_i4;
     _gfortran_link_i4_sub;
     _gfortran_link_i8;
Index: libgfortran/intrinsics/kill.c
===================================================================
--- libgfortran/intrinsics/kill.c	(revision 258468)
+++ libgfortran/intrinsics/kill.c	(working copy)
@@ -32,61 +32,32 @@ see the files COPYING3 and COPYING.RUNTIME respectivel
    INTEGER, INTENT(IN) :: PID, SIGNAL
    INTEGER(KIND=1), INTENT(OUT), OPTIONAL :: STATUS
 
-   INTEGER(KIND=1) FUNCTION KILL(PID, SIGNAL)
+   INTEGER FUNCTION KILL(PID, SIGNAL)
    INTEGER, INTENT(IN) :: PID, SIGNAL */
 
 #ifdef HAVE_KILL
-extern void kill_i4_sub (GFC_INTEGER_4 *, GFC_INTEGER_4 *, GFC_INTEGER_4 *);
-iexport_proto(kill_i4_sub);
+extern void kill_sub (GFC_INTEGER_4, GFC_INTEGER_4, GFC_INTEGER_4 *);
+iexport_proto(kill_sub);
 
 void
-kill_i4_sub (GFC_INTEGER_4 *pid, GFC_INTEGER_4 *signal,
-	     GFC_INTEGER_4 *status)
+kill_sub (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal, GFC_INTEGER_4 *status)
 {
   int val;
 
-  val = kill (*pid, *signal);
+  val = kill (pid, signal);
 
   if (status != NULL) 
     *status = (val == 0) ? 0 : errno;
 }
-iexport(kill_i4_sub);
+iexport(kill_sub);
 
-extern void kill_i8_sub (GFC_INTEGER_8 *, GFC_INTEGER_8 *, GFC_INTEGER_8 *);
-iexport_proto(kill_i8_sub);
+extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
+export_proto(kill);
 
-void
-kill_i8_sub (GFC_INTEGER_8 *pid, GFC_INTEGER_8 *signal,
-	     GFC_INTEGER_8 *status)
-{
-  int val;
-
-  val = kill (*pid, *signal);
-
-  if (status != NULL) 
-    *status = (val == 0) ? 0 : errno;
-}
-iexport(kill_i8_sub);
-
-extern GFC_INTEGER_4 kill_i4 (GFC_INTEGER_4 *, GFC_INTEGER_4 *);
-export_proto(kill_i4);
-
 GFC_INTEGER_4
-kill_i4 (GFC_INTEGER_4 *pid, GFC_INTEGER_4 *signal)
+kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal)
 {
-  GFC_INTEGER_4 val;
-  kill_i4_sub (pid, signal, &val);
-  return val;
+  return (int)kill (pid, signal);
 }
 
-extern GFC_INTEGER_8 kill_i8 (GFC_INTEGER_8 *, GFC_INTEGER_8 *);
-export_proto(kill_i8);
-
-GFC_INTEGER_8
-kill_i8 (GFC_INTEGER_8 *pid, GFC_INTEGER_8 *signal)
-{
-  GFC_INTEGER_8 val;
-  kill_i8_sub (pid, signal, &val);
-  return val;
-}
 #endif

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-13  4:08           ` Steve Kargl
@ 2018-03-13 19:49             ` Janne Blomqvist
  2018-03-14  0:57               ` Steve Kargl
  0 siblings, 1 reply; 20+ messages in thread
From: Janne Blomqvist @ 2018-03-13 19:49 UTC (permalink / raw)
  To: Steve Kargl; +Cc: Fortran List, GCC Patches

On Tue, Mar 13, 2018 at 6:08 AM, Steve Kargl
<sgk@troutmask.apl.washington.edu> wrote:
> On Mon, Mar 12, 2018 at 09:05:09PM +0200, Janne Blomqvist wrote:
>>
>> Yes, I understand that -fdefault-integer-8 (or whatever the equivalent
>> option was called on g77) is the original motivation. Like I said, I
>> don't have any particular opinion on whether we should keep that
>> restriction or not. On one hand, more recent versions of the standard
>> has lifted restrictions that integer intrinsic arguments have to be of
>> default kind in many cases, OTOH KILL is not a standard intrinsic but
>> something inherited from g77. So, meh.
>
> The Fortran standard specifically permits a Fortran processor to
> supply additional subprograms not contained in the Fortran standard.
> I personally can't any person person using INTEGER(1) or even
> INTEGER(2) with KILL as pid_t on typical modern OS's exceeds HUGE()
> in those types.  My original patch simply fixed KILL to actually
> conform to its documentation.  But is this what you want

Yes, very much so! Thanks!

One little nit, I think in libgfortran/intrinsics/kill.c

+kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal)
 {
-  GFC_INTEGER_4 val;
-  kill_i4_sub (pid, signal, &val);
-  return val;
+  return (int)kill (pid, signal);
 }

the implementation should be something like

int val = kill (pid, signal);
return (val == 0): 0 ? errno;

like it already does for the optional status argument for kill_sub.

-- 
Janne Blomqvist

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-13 19:49             ` Janne Blomqvist
@ 2018-03-14  0:57               ` Steve Kargl
  2018-03-15 10:18                 ` Bin.Cheng
  0 siblings, 1 reply; 20+ messages in thread
From: Steve Kargl @ 2018-03-14  0:57 UTC (permalink / raw)
  To: Janne Blomqvist; +Cc: Fortran List, GCC Patches

On Tue, Mar 13, 2018 at 09:49:10PM +0200, Janne Blomqvist wrote:
> 
> int val = kill (pid, signal);
> return (val == 0): 0 ? errno;
> 
> like it already does for the optional status argument for kill_sub.
> 

Committed as r258511 with your suggested change.

-- 
Steve

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-14  0:57               ` Steve Kargl
@ 2018-03-15 10:18                 ` Bin.Cheng
  2018-03-15 12:11                   ` Bin.Cheng
  0 siblings, 1 reply; 20+ messages in thread
From: Bin.Cheng @ 2018-03-15 10:18 UTC (permalink / raw)
  To: sgk; +Cc: Janne Blomqvist, Fortran List, GCC Patches

On Wed, Mar 14, 2018 at 12:57 AM, Steve Kargl
<sgk@troutmask.apl.washington.edu> wrote:
> On Tue, Mar 13, 2018 at 09:49:10PM +0200, Janne Blomqvist wrote:
>>
>> int val = kill (pid, signal);
>> return (val == 0): 0 ? errno;
>>
>> like it already does for the optional status argument for kill_sub.
>>
>
> Committed as r258511 with your suggested change.
Hi Steve,

After this change, AArch64/arm bare-metal cross (fortran) toolchain
fail to build with below error message:

/.../obj/gcc2/./gcc/xgcc -B/.../obj/gcc2/./gcc/
-B/.../install/aarch64-none-elf/bin/
-B/.../install/aarch64-none-elf/lib/ -isystem
/.../install/aarch64-none-elf/include -isystem
/.../install/aarch64-none-elf/sys-include -DHAVE_CONFIG_H -I.
-I/.../gcc/libgfortran -iquote/.../gcc/libgfortran/io
-I/.../gcc/libgfortran/../gcc -I/.../gcc/libgfortran/../gcc/config
-I../../.././gcc -I/.../gcc/libgfortran/../libgcc -I../../libgcc
-I/.../gcc/libgfortran/../libbacktrace -I../../libbacktrace
-I../libbacktrace -std=gnu11 -Wall -Wstrict-prototypes
-Wmissing-prototypes -Wold-style-definition -Wextra -Wwrite-strings
-Werror=implicit-function-declaration -Werror=vla -fcx-fortran-rules
-ffunction-sections -fdata-sections -g -ffunction-sections
-fdata-sections -O2 -mabi=ilp32 -MT kill.lo -MD -MP -MF .deps/kill.Tpo
-c /.../gcc/libgfortran/intrinsics/kill.c -o kill.o
/.../gcc/libgfortran/intrinsics/kill.c:54:22: error: conflicting types
for 'kill'
 extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
                      ^~~~
In file included from /.../install/aarch64-none-elf/include/signal.h:6,
                 from /.../gcc/libgfortran/intrinsics/kill.c:28:
/.../install/aarch64-none-elf/include/sys/signal.h:176:5: note:
previous declaration of 'kill' was here
 int kill (pid_t, int);
     ^~~~
In file included from /.../gcc/libgfortran/intrinsics/kill.c:26:
/.../gcc/libgfortran/intrinsics/kill.c:55:14: error: conflicting types
for 'kill'
 export_proto(kill);
              ^~~~
/.../gcc/libgfortran/libgfortran.h:150:57: note: in definition of
macro 'sym_rename2'
 #define sym_rename2(old, ulp, new) extern __typeof(old) old __asm__(#ulp #new)
                                                         ^~~
/.../gcc/libgfortran/libgfortran.h:148:30: note: in expansion of macro
'sym_rename1'
 #define sym_rename(old, new) sym_rename1(old, __USER_LABEL_PREFIX__, new)
                              ^~~~~~~~~~~
/.../gcc/libgfortran/libgfortran.h:195:26: note: in expansion of macro
'sym_rename'
 # define export_proto(x) sym_rename(x, PREFIX(x))
                          ^~~~~~~~~~
/.../gcc/libgfortran/intrinsics/kill.c:55:1: note: in expansion of
macro 'export_proto'
 export_proto(kill);
 ^~~~~~~~~~~~
In file included from /.../install/aarch64-none-elf/include/signal.h:6,
                 from /.../gcc/libgfortran/intrinsics/kill.c:28:
/.../install/aarch64-none-elf/include/sys/signal.h:176:5: note:
previous declaration of 'kill' was here
 int kill (pid_t, int);
     ^~~~
/.../gcc/libgfortran/intrinsics/kill.c:58:1: error: conflicting types for 'kill'
 kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal)
 ^~~~
In file included from /.../install/aarch64-none-elf/include/signal.h:6,
                 from /.../gcc/libgfortran/intrinsics/kill.c:28:
/.../install/aarch64-none-elf/include/sys/signal.h:176:5: note:
previous declaration of 'kill' was here
 int kill (pid_t, int);
     ^~~~

The gcc is configured with:
gcc/configure --target=aarch64-none-elf --prefix=...
--with-gmp=.../host-tools --with-mpfr=.../host-tools
--with-mpc=.../host-tools --with-isl=.../host-tools
--with-pkgversion=unknown --disable-shared --disable-nls
--disable-threads --disable-tls --enable-checking=yes
--enable-languages=c,c++ --with-newlib
--enable-languages=c,c++,fortran

I don't know fortran, so any suggestion how to fix this?
Note that -mabi=ilp32 is required to reproduce the breakage.

Thanks,
bin
>
> --
> Steve

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-15 10:18                 ` Bin.Cheng
@ 2018-03-15 12:11                   ` Bin.Cheng
  2018-03-15 12:20                     ` Bin.Cheng
  2018-03-15 12:35                     ` Richard Biener
  0 siblings, 2 replies; 20+ messages in thread
From: Bin.Cheng @ 2018-03-15 12:11 UTC (permalink / raw)
  To: sgk; +Cc: Janne Blomqvist, Fortran List, GCC Patches

On Thu, Mar 15, 2018 at 10:18 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Wed, Mar 14, 2018 at 12:57 AM, Steve Kargl
> <sgk@troutmask.apl.washington.edu> wrote:
>> On Tue, Mar 13, 2018 at 09:49:10PM +0200, Janne Blomqvist wrote:
>>>
>>> int val = kill (pid, signal);
>>> return (val == 0): 0 ? errno;
>>>
>>> like it already does for the optional status argument for kill_sub.
>>>
>>
>> Committed as r258511 with your suggested change.
> Hi Steve,
>
> After this change, AArch64/arm bare-metal cross (fortran) toolchain
> fail to build with below error message:
>
> /.../obj/gcc2/./gcc/xgcc -B/.../obj/gcc2/./gcc/
> -B/.../install/aarch64-none-elf/bin/
> -B/.../install/aarch64-none-elf/lib/ -isystem
> /.../install/aarch64-none-elf/include -isystem
> /.../install/aarch64-none-elf/sys-include -DHAVE_CONFIG_H -I.
> -I/.../gcc/libgfortran -iquote/.../gcc/libgfortran/io
> -I/.../gcc/libgfortran/../gcc -I/.../gcc/libgfortran/../gcc/config
> -I../../.././gcc -I/.../gcc/libgfortran/../libgcc -I../../libgcc
> -I/.../gcc/libgfortran/../libbacktrace -I../../libbacktrace
> -I../libbacktrace -std=gnu11 -Wall -Wstrict-prototypes
> -Wmissing-prototypes -Wold-style-definition -Wextra -Wwrite-strings
> -Werror=implicit-function-declaration -Werror=vla -fcx-fortran-rules
> -ffunction-sections -fdata-sections -g -ffunction-sections
> -fdata-sections -O2 -mabi=ilp32 -MT kill.lo -MD -MP -MF .deps/kill.Tpo
> -c /.../gcc/libgfortran/intrinsics/kill.c -o kill.o
> /.../gcc/libgfortran/intrinsics/kill.c:54:22: error: conflicting types
> for 'kill'
>  extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
>                       ^~~~
> In file included from /.../install/aarch64-none-elf/include/signal.h:6,
>                  from /.../gcc/libgfortran/intrinsics/kill.c:28:
> /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note:
> previous declaration of 'kill' was here
>  int kill (pid_t, int);
>      ^~~~
> In file included from /.../gcc/libgfortran/intrinsics/kill.c:26:
> /.../gcc/libgfortran/intrinsics/kill.c:55:14: error: conflicting types
> for 'kill'
>  export_proto(kill);
>               ^~~~
> /.../gcc/libgfortran/libgfortran.h:150:57: note: in definition of
> macro 'sym_rename2'
>  #define sym_rename2(old, ulp, new) extern __typeof(old) old __asm__(#ulp #new)
>                                                          ^~~
> /.../gcc/libgfortran/libgfortran.h:148:30: note: in expansion of macro
> 'sym_rename1'
>  #define sym_rename(old, new) sym_rename1(old, __USER_LABEL_PREFIX__, new)
>                               ^~~~~~~~~~~
> /.../gcc/libgfortran/libgfortran.h:195:26: note: in expansion of macro
> 'sym_rename'
>  # define export_proto(x) sym_rename(x, PREFIX(x))
>                           ^~~~~~~~~~
> /.../gcc/libgfortran/intrinsics/kill.c:55:1: note: in expansion of
> macro 'export_proto'
>  export_proto(kill);
>  ^~~~~~~~~~~~
> In file included from /.../install/aarch64-none-elf/include/signal.h:6,
>                  from /.../gcc/libgfortran/intrinsics/kill.c:28:
> /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note:
> previous declaration of 'kill' was here
>  int kill (pid_t, int);
>      ^~~~
> /.../gcc/libgfortran/intrinsics/kill.c:58:1: error: conflicting types for 'kill'
>  kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal)
>  ^~~~
> In file included from /.../install/aarch64-none-elf/include/signal.h:6,
>                  from /.../gcc/libgfortran/intrinsics/kill.c:28:
> /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note:
> previous declaration of 'kill' was here
>  int kill (pid_t, int);
>      ^~~~
>
> The gcc is configured with:
> gcc/configure --target=aarch64-none-elf --prefix=...
> --with-gmp=.../host-tools --with-mpfr=.../host-tools
> --with-mpc=.../host-tools --with-isl=.../host-tools
> --with-pkgversion=unknown --disable-shared --disable-nls
> --disable-threads --disable-tls --enable-checking=yes
> --enable-languages=c,c++ --with-newlib
> --enable-languages=c,c++,fortran
>
> I don't know fortran, so any suggestion how to fix this?
> Note that -mabi=ilp32 is required to reproduce the breakage.

So the pre-processed file for libgfortran/intrinsics/kill.c is like:


int kill (pid_t, int);

//......

extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
extern __typeof(kill) kill __asm__("" "_gfortran_kill");

GFC_INTEGER_4
kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal)
{
  int val;
  val = (int)kill (pid, signal);
  return ((val == 0) ? 0 :
# 62 "/.../gcc/libgfortran/intrinsics/kill.c" 3 4
                          (*__errno())
# 62 "/.../gcc/libgfortran/intrinsics/kill.c"
                               );
}

I suppose fortran wants to define its own kill returning errorno
directly on failure.
The problem is below declaration:

extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);

could conflict with included c declaration which is:

int kill (pid_t, int);

Even GFC_INTEGER_4 is typedef as int32_t, we can't rely on that is the
same as int type, right?
In this case, int32_t is defined as long int on aarch64_ilp32 or arm
bare-metal toolchains.

Any idea to resolve the inconsistency?

Thanks,
bin
>
> Thanks,
> bin
>>
>> --
>> Steve

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-15 12:11                   ` Bin.Cheng
@ 2018-03-15 12:20                     ` Bin.Cheng
  2018-03-15 15:07                       ` Steve Kargl
  2018-03-15 12:35                     ` Richard Biener
  1 sibling, 1 reply; 20+ messages in thread
From: Bin.Cheng @ 2018-03-15 12:20 UTC (permalink / raw)
  To: sgk; +Cc: Janne Blomqvist, Fortran List, GCC Patches

On Thu, Mar 15, 2018 at 12:11 PM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Thu, Mar 15, 2018 at 10:18 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>> On Wed, Mar 14, 2018 at 12:57 AM, Steve Kargl
>> <sgk@troutmask.apl.washington.edu> wrote:
>>> On Tue, Mar 13, 2018 at 09:49:10PM +0200, Janne Blomqvist wrote:
>>>>
>>>> int val = kill (pid, signal);
>>>> return (val == 0): 0 ? errno;
>>>>
>>>> like it already does for the optional status argument for kill_sub.
>>>>
>>>
>>> Committed as r258511 with your suggested change.
>> Hi Steve,
>>
>> After this change, AArch64/arm bare-metal cross (fortran) toolchain
>> fail to build with below error message:
>>
>> /.../obj/gcc2/./gcc/xgcc -B/.../obj/gcc2/./gcc/
>> -B/.../install/aarch64-none-elf/bin/
>> -B/.../install/aarch64-none-elf/lib/ -isystem
>> /.../install/aarch64-none-elf/include -isystem
>> /.../install/aarch64-none-elf/sys-include -DHAVE_CONFIG_H -I.
>> -I/.../gcc/libgfortran -iquote/.../gcc/libgfortran/io
>> -I/.../gcc/libgfortran/../gcc -I/.../gcc/libgfortran/../gcc/config
>> -I../../.././gcc -I/.../gcc/libgfortran/../libgcc -I../../libgcc
>> -I/.../gcc/libgfortran/../libbacktrace -I../../libbacktrace
>> -I../libbacktrace -std=gnu11 -Wall -Wstrict-prototypes
>> -Wmissing-prototypes -Wold-style-definition -Wextra -Wwrite-strings
>> -Werror=implicit-function-declaration -Werror=vla -fcx-fortran-rules
>> -ffunction-sections -fdata-sections -g -ffunction-sections
>> -fdata-sections -O2 -mabi=ilp32 -MT kill.lo -MD -MP -MF .deps/kill.Tpo
>> -c /.../gcc/libgfortran/intrinsics/kill.c -o kill.o
>> /.../gcc/libgfortran/intrinsics/kill.c:54:22: error: conflicting types
>> for 'kill'
>>  extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
>>                       ^~~~
>> In file included from /.../install/aarch64-none-elf/include/signal.h:6,
>>                  from /.../gcc/libgfortran/intrinsics/kill.c:28:
>> /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note:
>> previous declaration of 'kill' was here
>>  int kill (pid_t, int);
>>      ^~~~
>> In file included from /.../gcc/libgfortran/intrinsics/kill.c:26:
>> /.../gcc/libgfortran/intrinsics/kill.c:55:14: error: conflicting types
>> for 'kill'
>>  export_proto(kill);
>>               ^~~~
>> /.../gcc/libgfortran/libgfortran.h:150:57: note: in definition of
>> macro 'sym_rename2'
>>  #define sym_rename2(old, ulp, new) extern __typeof(old) old __asm__(#ulp #new)
>>                                                          ^~~
>> /.../gcc/libgfortran/libgfortran.h:148:30: note: in expansion of macro
>> 'sym_rename1'
>>  #define sym_rename(old, new) sym_rename1(old, __USER_LABEL_PREFIX__, new)
>>                               ^~~~~~~~~~~
>> /.../gcc/libgfortran/libgfortran.h:195:26: note: in expansion of macro
>> 'sym_rename'
>>  # define export_proto(x) sym_rename(x, PREFIX(x))
>>                           ^~~~~~~~~~
>> /.../gcc/libgfortran/intrinsics/kill.c:55:1: note: in expansion of
>> macro 'export_proto'
>>  export_proto(kill);
>>  ^~~~~~~~~~~~
>> In file included from /.../install/aarch64-none-elf/include/signal.h:6,
>>                  from /.../gcc/libgfortran/intrinsics/kill.c:28:
>> /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note:
>> previous declaration of 'kill' was here
>>  int kill (pid_t, int);
>>      ^~~~
>> /.../gcc/libgfortran/intrinsics/kill.c:58:1: error: conflicting types for 'kill'
>>  kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal)
>>  ^~~~
>> In file included from /.../install/aarch64-none-elf/include/signal.h:6,
>>                  from /.../gcc/libgfortran/intrinsics/kill.c:28:
>> /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note:
>> previous declaration of 'kill' was here
>>  int kill (pid_t, int);
>>      ^~~~
>>
>> The gcc is configured with:
>> gcc/configure --target=aarch64-none-elf --prefix=...
>> --with-gmp=.../host-tools --with-mpfr=.../host-tools
>> --with-mpc=.../host-tools --with-isl=.../host-tools
>> --with-pkgversion=unknown --disable-shared --disable-nls
>> --disable-threads --disable-tls --enable-checking=yes
>> --enable-languages=c,c++ --with-newlib
>> --enable-languages=c,c++,fortran
>>
>> I don't know fortran, so any suggestion how to fix this?
>> Note that -mabi=ilp32 is required to reproduce the breakage.
>
> So the pre-processed file for libgfortran/intrinsics/kill.c is like:
>
>
> int kill (pid_t, int);
>
> //......
>
> extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
> extern __typeof(kill) kill __asm__("" "_gfortran_kill");
>
> GFC_INTEGER_4
> kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal)
> {
>   int val;
>   val = (int)kill (pid, signal);
>   return ((val == 0) ? 0 :
> # 62 "/.../gcc/libgfortran/intrinsics/kill.c" 3 4
>                           (*__errno())
> # 62 "/.../gcc/libgfortran/intrinsics/kill.c"
>                                );
> }
>
> I suppose fortran wants to define its own kill returning errorno
> directly on failure.
> The problem is below declaration:
>
> extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
>
> could conflict with included c declaration which is:
>
> int kill (pid_t, int);
>
> Even GFC_INTEGER_4 is typedef as int32_t, we can't rely on that is the
> same as int type, right?
> In this case, int32_t is defined as long int on aarch64_ilp32 or arm
> bare-metal toolchains.
>
> Any idea to resolve the inconsistency?

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84880
filed.
>
> Thanks,
> bin
>>
>> Thanks,
>> bin
>>>
>>> --
>>> Steve

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-15 12:11                   ` Bin.Cheng
  2018-03-15 12:20                     ` Bin.Cheng
@ 2018-03-15 12:35                     ` Richard Biener
  2018-03-15 14:10                       ` Steve Kargl
  1 sibling, 1 reply; 20+ messages in thread
From: Richard Biener @ 2018-03-15 12:35 UTC (permalink / raw)
  To: Bin.Cheng; +Cc: Steve Kargl, Janne Blomqvist, Fortran List, GCC Patches

On Thu, Mar 15, 2018 at 1:11 PM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Thu, Mar 15, 2018 at 10:18 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>> On Wed, Mar 14, 2018 at 12:57 AM, Steve Kargl
>> <sgk@troutmask.apl.washington.edu> wrote:
>>> On Tue, Mar 13, 2018 at 09:49:10PM +0200, Janne Blomqvist wrote:
>>>>
>>>> int val = kill (pid, signal);
>>>> return (val == 0): 0 ? errno;
>>>>
>>>> like it already does for the optional status argument for kill_sub.
>>>>
>>>
>>> Committed as r258511 with your suggested change.
>> Hi Steve,
>>
>> After this change, AArch64/arm bare-metal cross (fortran) toolchain
>> fail to build with below error message:
>>
>> /.../obj/gcc2/./gcc/xgcc -B/.../obj/gcc2/./gcc/
>> -B/.../install/aarch64-none-elf/bin/
>> -B/.../install/aarch64-none-elf/lib/ -isystem
>> /.../install/aarch64-none-elf/include -isystem
>> /.../install/aarch64-none-elf/sys-include -DHAVE_CONFIG_H -I.
>> -I/.../gcc/libgfortran -iquote/.../gcc/libgfortran/io
>> -I/.../gcc/libgfortran/../gcc -I/.../gcc/libgfortran/../gcc/config
>> -I../../.././gcc -I/.../gcc/libgfortran/../libgcc -I../../libgcc
>> -I/.../gcc/libgfortran/../libbacktrace -I../../libbacktrace
>> -I../libbacktrace -std=gnu11 -Wall -Wstrict-prototypes
>> -Wmissing-prototypes -Wold-style-definition -Wextra -Wwrite-strings
>> -Werror=implicit-function-declaration -Werror=vla -fcx-fortran-rules
>> -ffunction-sections -fdata-sections -g -ffunction-sections
>> -fdata-sections -O2 -mabi=ilp32 -MT kill.lo -MD -MP -MF .deps/kill.Tpo
>> -c /.../gcc/libgfortran/intrinsics/kill.c -o kill.o
>> /.../gcc/libgfortran/intrinsics/kill.c:54:22: error: conflicting types
>> for 'kill'
>>  extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
>>                       ^~~~
>> In file included from /.../install/aarch64-none-elf/include/signal.h:6,
>>                  from /.../gcc/libgfortran/intrinsics/kill.c:28:
>> /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note:
>> previous declaration of 'kill' was here
>>  int kill (pid_t, int);
>>      ^~~~
>> In file included from /.../gcc/libgfortran/intrinsics/kill.c:26:
>> /.../gcc/libgfortran/intrinsics/kill.c:55:14: error: conflicting types
>> for 'kill'
>>  export_proto(kill);
>>               ^~~~
>> /.../gcc/libgfortran/libgfortran.h:150:57: note: in definition of
>> macro 'sym_rename2'
>>  #define sym_rename2(old, ulp, new) extern __typeof(old) old __asm__(#ulp #new)
>>                                                          ^~~
>> /.../gcc/libgfortran/libgfortran.h:148:30: note: in expansion of macro
>> 'sym_rename1'
>>  #define sym_rename(old, new) sym_rename1(old, __USER_LABEL_PREFIX__, new)
>>                               ^~~~~~~~~~~
>> /.../gcc/libgfortran/libgfortran.h:195:26: note: in expansion of macro
>> 'sym_rename'
>>  # define export_proto(x) sym_rename(x, PREFIX(x))
>>                           ^~~~~~~~~~
>> /.../gcc/libgfortran/intrinsics/kill.c:55:1: note: in expansion of
>> macro 'export_proto'
>>  export_proto(kill);
>>  ^~~~~~~~~~~~
>> In file included from /.../install/aarch64-none-elf/include/signal.h:6,
>>                  from /.../gcc/libgfortran/intrinsics/kill.c:28:
>> /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note:
>> previous declaration of 'kill' was here
>>  int kill (pid_t, int);
>>      ^~~~
>> /.../gcc/libgfortran/intrinsics/kill.c:58:1: error: conflicting types for 'kill'
>>  kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal)
>>  ^~~~
>> In file included from /.../install/aarch64-none-elf/include/signal.h:6,
>>                  from /.../gcc/libgfortran/intrinsics/kill.c:28:
>> /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note:
>> previous declaration of 'kill' was here
>>  int kill (pid_t, int);
>>      ^~~~
>>
>> The gcc is configured with:
>> gcc/configure --target=aarch64-none-elf --prefix=...
>> --with-gmp=.../host-tools --with-mpfr=.../host-tools
>> --with-mpc=.../host-tools --with-isl=.../host-tools
>> --with-pkgversion=unknown --disable-shared --disable-nls
>> --disable-threads --disable-tls --enable-checking=yes
>> --enable-languages=c,c++ --with-newlib
>> --enable-languages=c,c++,fortran
>>
>> I don't know fortran, so any suggestion how to fix this?
>> Note that -mabi=ilp32 is required to reproduce the breakage.
>
> So the pre-processed file for libgfortran/intrinsics/kill.c is like:
>
>
> int kill (pid_t, int);
>
> //......
>
> extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
> extern __typeof(kill) kill __asm__("" "_gfortran_kill");

Why do you need to jump through these hoops anyway?  Just do ...

> GFC_INTEGER_4
> kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal)

 _gfortran_kill (...

here.  The kill prototype should come via signal.h already.

I guess the issue is that libgfortran 'kill' (exported with _gfortran_ prefix)
aliases the POSIX kill and you even want to call that here.

So maybe just use export_proto_np or add another variant that works
on function definitions directly so you can write

GFC_INTEGER_4
export_def(kill) (GFC_INTEGER_4 ...)
{
}

> {
>   int val;
>   val = (int)kill (pid, signal);
>   return ((val == 0) ? 0 :
> # 62 "/.../gcc/libgfortran/intrinsics/kill.c" 3 4
>                           (*__errno())
> # 62 "/.../gcc/libgfortran/intrinsics/kill.c"
>                                );
> }
>
> I suppose fortran wants to define its own kill returning errorno
> directly on failure.
> The problem is below declaration:
>
> extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
>
> could conflict with included c declaration which is:
>
> int kill (pid_t, int);
>
> Even GFC_INTEGER_4 is typedef as int32_t, we can't rely on that is the
> same as int type, right?
> In this case, int32_t is defined as long int on aarch64_ilp32 or arm
> bare-metal toolchains.
>
> Any idea to resolve the inconsistency?
>
> Thanks,
> bin
>>
>> Thanks,
>> bin
>>>
>>> --
>>> Steve

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-15 12:35                     ` Richard Biener
@ 2018-03-15 14:10                       ` Steve Kargl
  2018-03-15 16:35                         ` Jakub Jelinek
  0 siblings, 1 reply; 20+ messages in thread
From: Steve Kargl @ 2018-03-15 14:10 UTC (permalink / raw)
  To: Richard Biener; +Cc: Bin.Cheng, Janne Blomqvist, Fortran List, GCC Patches

On Thu, Mar 15, 2018 at 01:35:23PM +0100, Richard Biener wrote:
> >
> > extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
> > extern __typeof(kill) kill __asm__("" "_gfortran_kill");
> 
> Why do you need to jump through these hoops anyway?  Just do ...
> 

Not sure who the "you" refers to.  The easiest 
fix be appending a 4 to kill.  I'll do that 
later.

-- 
Steve

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-15 12:20                     ` Bin.Cheng
@ 2018-03-15 15:07                       ` Steve Kargl
  0 siblings, 0 replies; 20+ messages in thread
From: Steve Kargl @ 2018-03-15 15:07 UTC (permalink / raw)
  To: Bin.Cheng; +Cc: Janne Blomqvist, Fortran List, GCC Patches

On Thu, Mar 15, 2018 at 12:20:08PM +0000, Bin.Cheng wrote:
> >> -fdata-sections -O2 -mabi=ilp32 -MT kill.lo -MD -MP -MF .deps/kill.Tpo
> >> -c /.../gcc/libgfortran/intrinsics/kill.c -o kill.o
> >> /.../gcc/libgfortran/intrinsics/kill.c:54:22: error: conflicting types
> >> for 'kill'
> >>  extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
> >>                       ^~~~
> >> In file included from /.../install/aarch64-none-elf/include/signal.h:6,
> >>                  from /.../gcc/libgfortran/intrinsics/kill.c:28:
> >> /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note:
> >> previous declaration of 'kill' was here
> >>  int kill (pid_t, int);
> >>      ^~~~

Does this fix the issue for you?

Index: libgfortran/intrinsics/kill.c
===================================================================
--- libgfortran/intrinsics/kill.c	(revision 258537)
+++ libgfortran/intrinsics/kill.c	(working copy)
@@ -36,11 +36,9 @@ see the files COPYING3 and COPYING.RUNTIME respectivel
    INTEGER, INTENT(IN) :: PID, SIGNAL */
 
 #ifdef HAVE_KILL
-extern void kill_sub (GFC_INTEGER_4, GFC_INTEGER_4, GFC_INTEGER_4 *);
-iexport_proto(kill_sub);
 
 void
-kill_sub (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal, GFC_INTEGER_4 *status)
+_gfortran_kill_sub (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal, GFC_INTEGER_4 *status)
 {
   int val;
 
@@ -49,13 +47,9 @@ kill_sub (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal, GFC
   if (status != NULL) 
     *status = (val == 0) ? 0 : errno;
 }
-iexport(kill_sub);
 
-extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
-export_proto(kill);
-
 GFC_INTEGER_4
-kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal)
+_gfortran_kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal)
 {
   int val;
   val = (int)kill (pid, signal);

-- 
Steve
20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4
20161221 https://www.youtube.com/watch?v=IbCHE-hONow

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-15 16:35                         ` Jakub Jelinek
@ 2018-03-15 15:45                           ` Bin.Cheng
  2018-03-15 17:35                             ` Jakub Jelinek
  2018-03-15 16:28                           ` Steve Kargl
  1 sibling, 1 reply; 20+ messages in thread
From: Bin.Cheng @ 2018-03-15 15:45 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Steve Kargl, Richard Biener, Janne Blomqvist, Fortran List, GCC Patches

On Thu, Mar 15, 2018 at 3:08 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Mar 15, 2018 at 07:10:26AM -0700, Steve Kargl wrote:
>> On Thu, Mar 15, 2018 at 01:35:23PM +0100, Richard Biener wrote:
>> > >
>> > > extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
>> > > extern __typeof(kill) kill __asm__("" "_gfortran_kill");
>> >
>> > Why do you need to jump through these hoops anyway?  Just do ...
>> >
>>
>> Not sure who the "you" refers to.  The easiest
>> fix be appending a 4 to kill.  I'll do that
>> later.
>
> I think this is even easier, no need to rename anything:
>
> 2018-03-15  Jakub Jelinek  <jakub@redhat.com>
>
>         PR libgfortran/84880
>         * intrinsics/kill.c (kill): Rename to...
>         (PREFIX (kill)): ... this.  Use export_proto_np instead of export_proto.
>
> --- libgfortran/intrinsics/kill.c.jj    2018-03-14 09:44:57.988975360 +0100
> +++ libgfortran/intrinsics/kill.c       2018-03-15 16:01:02.725668658 +0100
> @@ -51,11 +51,11 @@ kill_sub (GFC_INTEGER_4 pid, GFC_INTEGER
>  }
>  iexport(kill_sub);
>
> -extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
> -export_proto(kill);
> +extern GFC_INTEGER_4 PREFIX (kill) (GFC_INTEGER_4, GFC_INTEGER_4);
> +export_proto_np(PREFIX (kill));
>
>  GFC_INTEGER_4
> -kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal)
> +PREFIX (kill) (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal)
>  {
>    int val;
>    val = (int)kill (pid, signal);

Hi,

FYI,  both your patches fix the compilation issue.

Thanks,
bin
>
>
>         Jakub

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-15 17:35                             ` Jakub Jelinek
@ 2018-03-15 15:57                               ` Bin.Cheng
  0 siblings, 0 replies; 20+ messages in thread
From: Bin.Cheng @ 2018-03-15 15:57 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Steve Kargl, Richard Biener, Janne Blomqvist, Fortran List, GCC Patches

On Thu, Mar 15, 2018 at 3:54 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Mar 15, 2018 at 03:45:47PM +0000, Bin.Cheng wrote:
>> FYI,  both your patches fix the compilation issue.
>
> It isn't just a compilation problem, it really can't work at all.
> Without the patch, if the function builds, it looks like:
Ah, Thanks.  I haven't checked generated code when it builds.

Thanks,
bin
> 00000000002308b0 <_gfortran_kill>:
>   2308b0:       f3 0f 1e fa             endbr64
>   2308b4:       48 83 ec 08             sub    $0x8,%rsp
>   2308b8:       e8 f3 ff ff ff          callq  2308b0 <_gfortran_kill>
>   2308bd:       85 c0                   test   %eax,%eax
>   2308bf:       74 07                   je     2308c8 <_gfortran_kill+0x18>
>   2308c1:       e8 4a 92 de ff          callq  19b10 <__errno_location@plt>
>   2308c6:       8b 00                   mov    (%rax),%eax
>   2308c8:       48 83 c4 08             add    $0x8,%rsp
>   2308cc:       c3                      retq
>   2308cd:       0f 1f 00                nopl   (%rax)
> i.e. there is endless recursion, it doesn't call the libc kill, but itself.
>
>         Jakub

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-15 16:35                         ` Jakub Jelinek
  2018-03-15 15:45                           ` Bin.Cheng
@ 2018-03-15 16:28                           ` Steve Kargl
  1 sibling, 0 replies; 20+ messages in thread
From: Steve Kargl @ 2018-03-15 16:28 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Richard Biener, Bin.Cheng, Janne Blomqvist, Fortran List, GCC Patches

On Thu, Mar 15, 2018 at 04:08:10PM +0100, Jakub Jelinek wrote:
> On Thu, Mar 15, 2018 at 07:10:26AM -0700, Steve Kargl wrote:
> > On Thu, Mar 15, 2018 at 01:35:23PM +0100, Richard Biener wrote:
> > > >
> > > > extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
> > > > extern __typeof(kill) kill __asm__("" "_gfortran_kill");
> > > 
> > > Why do you need to jump through these hoops anyway?  Just do ...
> > > 
> > 
> > Not sure who the "you" refers to.  The easiest 
> > fix be appending a 4 to kill.  I'll do that 
> > later.
> 
> I think this is even easier, no need to rename anything:
> 
> 2018-03-15  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR libgfortran/84880
> 	* intrinsics/kill.c (kill): Rename to...
> 	(PREFIX (kill)): ... this.  Use export_proto_np instead of export_proto.
> 

If you haven't committed your patch, it's OK with me.
Sorry about the breakage.

-- 
Steve

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-15 14:10                       ` Steve Kargl
@ 2018-03-15 16:35                         ` Jakub Jelinek
  2018-03-15 15:45                           ` Bin.Cheng
  2018-03-15 16:28                           ` Steve Kargl
  0 siblings, 2 replies; 20+ messages in thread
From: Jakub Jelinek @ 2018-03-15 16:35 UTC (permalink / raw)
  To: Steve Kargl
  Cc: Richard Biener, Bin.Cheng, Janne Blomqvist, Fortran List, GCC Patches

On Thu, Mar 15, 2018 at 07:10:26AM -0700, Steve Kargl wrote:
> On Thu, Mar 15, 2018 at 01:35:23PM +0100, Richard Biener wrote:
> > >
> > > extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
> > > extern __typeof(kill) kill __asm__("" "_gfortran_kill");
> > 
> > Why do you need to jump through these hoops anyway?  Just do ...
> > 
> 
> Not sure who the "you" refers to.  The easiest 
> fix be appending a 4 to kill.  I'll do that 
> later.

I think this is even easier, no need to rename anything:

2018-03-15  Jakub Jelinek  <jakub@redhat.com>

	PR libgfortran/84880
	* intrinsics/kill.c (kill): Rename to...
	(PREFIX (kill)): ... this.  Use export_proto_np instead of export_proto.

--- libgfortran/intrinsics/kill.c.jj	2018-03-14 09:44:57.988975360 +0100
+++ libgfortran/intrinsics/kill.c	2018-03-15 16:01:02.725668658 +0100
@@ -51,11 +51,11 @@ kill_sub (GFC_INTEGER_4 pid, GFC_INTEGER
 }
 iexport(kill_sub);
 
-extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4);
-export_proto(kill);
+extern GFC_INTEGER_4 PREFIX (kill) (GFC_INTEGER_4, GFC_INTEGER_4);
+export_proto_np(PREFIX (kill));
 
 GFC_INTEGER_4
-kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal)
+PREFIX (kill) (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal)
 {
   int val;
   val = (int)kill (pid, signal);


	Jakub

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Fortran -- clean up KILL
  2018-03-15 15:45                           ` Bin.Cheng
@ 2018-03-15 17:35                             ` Jakub Jelinek
  2018-03-15 15:57                               ` Bin.Cheng
  0 siblings, 1 reply; 20+ messages in thread
From: Jakub Jelinek @ 2018-03-15 17:35 UTC (permalink / raw)
  To: Bin.Cheng
  Cc: Steve Kargl, Richard Biener, Janne Blomqvist, Fortran List, GCC Patches

On Thu, Mar 15, 2018 at 03:45:47PM +0000, Bin.Cheng wrote:
> FYI,  both your patches fix the compilation issue.

It isn't just a compilation problem, it really can't work at all.
Without the patch, if the function builds, it looks like:
00000000002308b0 <_gfortran_kill>:
  2308b0:       f3 0f 1e fa             endbr64 
  2308b4:       48 83 ec 08             sub    $0x8,%rsp
  2308b8:       e8 f3 ff ff ff          callq  2308b0 <_gfortran_kill>
  2308bd:       85 c0                   test   %eax,%eax
  2308bf:       74 07                   je     2308c8 <_gfortran_kill+0x18>
  2308c1:       e8 4a 92 de ff          callq  19b10 <__errno_location@plt>
  2308c6:       8b 00                   mov    (%rax),%eax
  2308c8:       48 83 c4 08             add    $0x8,%rsp
  2308cc:       c3                      retq   
  2308cd:       0f 1f 00                nopl   (%rax)
i.e. there is endless recursion, it doesn't call the libc kill, but itself.

	Jakub

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2018-03-15 17:35 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-11 16:52 [PATCH] Fortran -- clean up KILL Steve Kargl
2018-03-11 20:16 ` Janne Blomqvist
2018-03-11 20:42   ` Steve Kargl
2018-03-12 16:56     ` Janne Blomqvist
2018-03-12 17:37       ` Steve Kargl
2018-03-12 19:05         ` Janne Blomqvist
2018-03-13  4:08           ` Steve Kargl
2018-03-13 19:49             ` Janne Blomqvist
2018-03-14  0:57               ` Steve Kargl
2018-03-15 10:18                 ` Bin.Cheng
2018-03-15 12:11                   ` Bin.Cheng
2018-03-15 12:20                     ` Bin.Cheng
2018-03-15 15:07                       ` Steve Kargl
2018-03-15 12:35                     ` Richard Biener
2018-03-15 14:10                       ` Steve Kargl
2018-03-15 16:35                         ` Jakub Jelinek
2018-03-15 15:45                           ` Bin.Cheng
2018-03-15 17:35                             ` Jakub Jelinek
2018-03-15 15:57                               ` Bin.Cheng
2018-03-15 16:28                           ` Steve Kargl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).