Re: [PATCH] Fortran -- clean up KILL
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 > > 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
Re: [PATCH] Fortran -- clean up KILL
On Thu, Mar 15, 2018 at 3:54 PM, Jakub Jelinek wrote: > On Thu, Mar 15, 2018 at 03:45:47PM +, 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 > 002308b0 <_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 00nopl (%rax) > i.e. there is endless recursion, it doesn't call the libc kill, but itself. > > Jakub
Re: [PATCH] Fortran -- clean up KILL
On Thu, Mar 15, 2018 at 03:45:47PM +, 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: 002308b0 <_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 00nopl (%rax) i.e. there is endless recursion, it doesn't call the libc kill, but itself. Jakub
Re: [PATCH] Fortran -- clean up KILL
On Thu, Mar 15, 2018 at 3:08 PM, 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 > > PR libgfortran/84880 > * intrinsics/kill.c (kill): Rename to... > (PREFIX (kill)): ... this. Use export_proto_np instead of > export_proto. > > --- libgfortran/intrinsics/kill.c.jj2018-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
Re: [PATCH] Fortran -- clean up KILL
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 PR libgfortran/84880 * intrinsics/kill.c (kill): Rename to... (PREFIX (kill)): ... this. Use export_proto_np instead of export_proto. --- libgfortran/intrinsics/kill.c.jj2018-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
Re: [PATCH] Fortran -- clean up KILL
On Thu, Mar 15, 2018 at 12:20:08PM +, 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
Re: [PATCH] Fortran -- clean up KILL
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
Re: [PATCH] Fortran -- clean up KILL
On Thu, Mar 15, 2018 at 1:11 PM, Bin.Cheng wrote: > On Thu, Mar 15, 2018 at 10:18 AM, Bin.Cheng wrote: >> On Wed, Mar 14, 2018 at 12:57 AM, Steve Kargl >> 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
Re: [PATCH] Fortran -- clean up KILL
On Thu, Mar 15, 2018 at 12:11 PM, Bin.Cheng wrote: > On Thu, Mar 15, 2018 at 10:18 AM, Bin.Cheng wrote: >> On Wed, Mar 14, 2018 at 12:57 AM, Steve Kargl >> 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 w
Re: [PATCH] Fortran -- clean up KILL
On Thu, Mar 15, 2018 at 10:18 AM, Bin.Cheng wrote: > On Wed, Mar 14, 2018 at 12:57 AM, Steve Kargl > 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 a
Re: [PATCH] Fortran -- clean up KILL
On Wed, Mar 14, 2018 at 12:57 AM, Steve Kargl 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
Re: [PATCH] Fortran -- clean up KILL
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
Re: [PATCH] Fortran -- clean up KILL
On Tue, Mar 13, 2018 at 6:08 AM, Steve Kargl 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
Re: [PATCH] Fortran -- clean up KILL
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 * 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 * 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 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 S
Re: [PATCH] Fortran -- clean up KILL
On Mon, Mar 12, 2018 at 7:37 PM, Steve Kargl 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
Re: [PATCH] Fortran -- clean up KILL
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
Re: [PATCH] Fortran -- clean up KILL
On Sun, Mar 11, 2018 at 10:42 PM, Steve Kargl 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 >> 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 >> > >> > * 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 > > T _gfortran_kill_i4 > T _gfortran_kill_i4_sub > T _gfortran_kill_i8 > T _gfortran_kill_i8_sub > T _gfortrani_kill_i4_sub > 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
Re: [PATCH] Fortran -- clean up KILL
On Sun, Mar 11, 2018 at 10:16:01PM +0200, Janne Blomqvist wrote: > On Sun, Mar 11, 2018 at 6:52 PM, Steve Kargl > 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 > > > > * 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 T _gfortran_kill_i4 T _gfortran_kill_i4_sub T _gfortran_kill_i8 T _gfortran_kill_i8_sub T _gfortrani_kill_i4_sub 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
Re: [PATCH] Fortran -- clean up KILL
On Sun, Mar 11, 2018 at 6:52 PM, Steve Kargl 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 > > * 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