| Message ID | 000901ceae04$546ced10$fd46c730$@arm.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Sep 10, 2013 at 2:01 AM, Joey Ye <joey.ye@arm.com> wrote: > Updated to http://gcc.gnu.org/ml/gcc-patches/2012-12/msg01097.html > > Build passes on arm-none-eabi and bootstrap passes on x86. > > OK to trunk? > > ChangeLog > * libgcc/Makefile.in: Include JAVA_IS_ENABLED in CFLAGS. > * libgcc/configure.ac (java_is_enabled): New variable. > * libgcc/configure: Regenerated. > * libgcc/crtstuff.c: Check JAVA_IS_ENABLED. The ChangeLog entries should be in libgcc/ChangeLog, and they should not have the libgcc/ prefix on the file names. Compare to the other entries in that file. This patch is OK for libgcc. However, before committing it, I would like it to be approved by a Java maintainer. I've CC'ed the Java maintainers on this message. Thanks. Ian > Index: Makefile.in > =================================================================== > --- Makefile.in (revision 194467) > +++ Makefile.in (working copy) > @@ -281,7 +281,8 @@ > -finhibit-size-directive -fno-inline -fno-exceptions \ > -fno-zero-initialized-in-bss -fno-toplevel-reorder -fno-tree-vectorize \ > -fno-stack-protector \ > - $(INHIBIT_LIBC_CFLAGS) > + $(INHIBIT_LIBC_CFLAGS) \ > + -DJAVA_IS_ENABLED=@java_is_enabled@ > > # Extra flags to use when compiling crt{begin,end}.o. > CRTSTUFF_T_CFLAGS = > Index: configure.ac > =================================================================== > --- configure.ac (revision 194467) > +++ configure.ac (working copy) > @@ -204,6 +204,17 @@ > esac], > [enable_sjlj_exceptions=auto]) > > +# Disable jcr section if we are not building java > +case ,${enable_languages}, in > + *,java,*) > + java_is_enabled=1 > + ;; > + *) > + java_is_enabled=0 > + ;; > +esac > +AC_SUBST(java_is_enabled) > + > AC_CACHE_CHECK([whether to use setjmp/longjmp exceptions], > [libgcc_cv_lib_sjlj_exceptions], > [AC_LANG_CONFTEST( > Index: crtstuff.c > =================================================================== > --- crtstuff.c (revision 194467) > +++ crtstuff.c (working copy) > @@ -145,6 +145,10 @@ > # define USE_TM_CLONE_REGISTRY 1 > #endif > > +#if !JAVA_IS_ENABLED > +#undef JCR_SECTION_NAME > +#endif > + > /* We do not want to add the weak attribute to the declarations of these > routines in unwind-dw2-fde.h because that will cause the definition of > these symbols to be weak as well. > Index: configure > =================================================================== > --- configure (revision 194467) > +++ configure (working copy) > @@ -566,6 +566,7 @@ > set_use_emutls > set_have_cc_tls > vis_hide > +java_is_enabled > fixed_point > enable_decimal_float > decimal_float > @@ -4191,6 +4192,17 @@ > fi > > > +# Disable jcr section if we are not building java > +case ,${enable_languages}, in > + *,java,*) > + java_is_enabled=1 > + ;; > + *) > + java_is_enabled=0 > + ;; > +esac > + > + > { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to use > setjmp/longjmp exceptions" >&5 > $as_echo_n "checking whether to use setjmp/longjmp exceptions... " >&6; } > if test "${libgcc_cv_lib_sjlj_exceptions+set}" = set; then : > > >
Dear Java maintainers, are you OK with this patch? - Joey > -----Original Message----- > From: Ian Lance Taylor [mailto:iant@google.com] > Sent: Thursday, September 12, 2013 3:28 > To: Joey Ye > Cc: gcc-patches; H.J. Lu; per@bothner.com; aph@redhat.com; Tom Tromey > Subject: Re: [PATCH, libgcc] Disable JCR section when java is not enabled > > On Tue, Sep 10, 2013 at 2:01 AM, Joey Ye <joey.ye@arm.com> wrote: > > Updated to http://gcc.gnu.org/ml/gcc-patches/2012-12/msg01097.html > > > > Build passes on arm-none-eabi and bootstrap passes on x86. > > > > OK to trunk? > > > > ChangeLog > > * libgcc/Makefile.in: Include JAVA_IS_ENABLED in CFLAGS. > > * libgcc/configure.ac (java_is_enabled): New variable. > > * libgcc/configure: Regenerated. > > * libgcc/crtstuff.c: Check JAVA_IS_ENABLED. > > > The ChangeLog entries should be in libgcc/ChangeLog, and they should not > have the libgcc/ prefix on the file names. Compare to the other entries in > that file. > > This patch is OK for libgcc. > > However, before committing it, I would like it to be approved by a Java > maintainer. I've CC'ed the Java maintainers on this message. > > Thanks. > > Ian > > > > > > Index: Makefile.in > > > ========================================================== > ========= > > --- Makefile.in (revision 194467) > > +++ Makefile.in (working copy) > > @@ -281,7 +281,8 @@ > > -finhibit-size-directive -fno-inline -fno-exceptions \ > > -fno-zero-initialized-in-bss -fno-toplevel-reorder -fno-tree-vectorize \ > > -fno-stack-protector \ > > - $(INHIBIT_LIBC_CFLAGS) > > + $(INHIBIT_LIBC_CFLAGS) \ > > + -DJAVA_IS_ENABLED=@java_is_enabled@ > > > > # Extra flags to use when compiling crt{begin,end}.o. > > CRTSTUFF_T_CFLAGS = > > Index: configure.ac > > > ========================================================== > ========= > > --- configure.ac (revision 194467) > > +++ configure.ac (working copy) > > @@ -204,6 +204,17 @@ > > esac], > > [enable_sjlj_exceptions=auto]) > > > > +# Disable jcr section if we are not building java case > > +,${enable_languages}, in > > + *,java,*) > > + java_is_enabled=1 > > + ;; > > + *) > > + java_is_enabled=0 > > + ;; > > +esac > > +AC_SUBST(java_is_enabled) > > + > > AC_CACHE_CHECK([whether to use setjmp/longjmp exceptions], > > [libgcc_cv_lib_sjlj_exceptions], [AC_LANG_CONFTEST( > > Index: crtstuff.c > > > ========================================================== > ========= > > --- crtstuff.c (revision 194467) > > +++ crtstuff.c (working copy) > > @@ -145,6 +145,10 @@ > > # define USE_TM_CLONE_REGISTRY 1 > > #endif > > > > +#if !JAVA_IS_ENABLED > > +#undef JCR_SECTION_NAME > > +#endif > > + > > /* We do not want to add the weak attribute to the declarations of these > > routines in unwind-dw2-fde.h because that will cause the definition of > > these symbols to be weak as well. > > Index: configure > > > ========================================================== > ========= > > --- configure (revision 194467) > > +++ configure (working copy) > > @@ -566,6 +566,7 @@ > > set_use_emutls > > set_have_cc_tls > > vis_hide > > +java_is_enabled > > fixed_point > > enable_decimal_float > > decimal_float > > @@ -4191,6 +4192,17 @@ > > fi > > > > > > +# Disable jcr section if we are not building java case > > +,${enable_languages}, in > > + *,java,*) > > + java_is_enabled=1 > > + ;; > > + *) > > + java_is_enabled=0 > > + ;; > > +esac > > + > > + > > { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to use > > setjmp/longjmp exceptions" >&5 $as_echo_n "checking whether to use > > setjmp/longjmp exceptions... " >&6; } if test > > "${libgcc_cv_lib_sjlj_exceptions+set}" = set; then : > > > > > >
On Thu, Oct 10, 2013 at 04:22:52PM +0800, Joey Ye wrote:
> Dear Java maintainers, are you OK with this patch?
Given the state of gcj that it is now only rarely used and most people
just use OpenJDK instead, wouldn't it be a good idea to just require
that gcj code is linked using gcj driver or, if linked in any other driver,
just using a special non-default option (-flink-jcr or similar), that would
be automatically set by gcj driver, move this JCR stuff out of the normal
crt* files and put it into crtjava*.o instead, and only link in if
-flink-jcr is passed or gcj driver used? Or treat -lgcj as that magic
switch?
Or, alternatively, at least for selected targets, live with the extra 8
bytes in .jcr section for every binary/shared library, but move the
_Jv_RegisterClasses call into libgcj_nonshared.a and libgcj.a and make
libgcj.so be a linker script containing both libgcj_nonshared.a and
libgcj.so.*.
Also, looking at crtstuff.c makes me wonder where are classes deregistered,
there are only calls to _Jv_RegisterClasses, but never to to deregistration,
wonder what happens if you dlclose a shared library with registered classes.
Jakub
Jakub> Given the state of gcj that it is now only rarely used and most Jakub> people just use OpenJDK instead, wouldn't it be a good idea to Jakub> just require that gcj code is linked using gcj driver or, if Jakub> linked in any other driver, just using a special non-default Jakub> option (-flink-jcr or similar), that would be automatically set Jakub> by gcj driver, move this JCR stuff out of the normal crt* files Jakub> and put it into crtjava*.o instead, and only link in if Jakub> -flink-jcr is passed or gcj driver used? Or treat -lgcj as that Jakub> magic switch? The irony of the situation is that this would require significantly more work than has gone into gcj in the past N years. Jakub> Also, looking at crtstuff.c makes me wonder where are classes Jakub> deregistered, there are only calls to _Jv_RegisterClasses, but Jakub> never to to deregistration, wonder what happens if you dlclose a Jakub> shared library with registered classes. I think we never implemented class GC for compiled classes, though it's hard to remember. Tom
> -----Original Message----- > From: Jakub Jelinek [mailto:jakub@redhat.com] > Sent: Thursday, October 10, 2013 16:48 > To: Joey Ye > Cc: per@bothner.com; aph@redhat.com; Tom Tromey; H.J. Lu; gcc-patches; > 'Ian Lance Taylor' > Subject: Re: [PATCH, libgcc] Disable JCR section when java is not enabled > > On Thu, Oct 10, 2013 at 04:22:52PM +0800, Joey Ye wrote: > > Dear Java maintainers, are you OK with this patch? > > Given the state of gcj that it is now only rarely used and most people just use > OpenJDK instead, wouldn't it be a good idea to just require that gcj code is > linked using gcj driver or, if linked in any other driver, just using a special non- > default option (-flink-jcr or similar), that would be automatically set by gcj > driver, move this JCR stuff out of the normal > crt* files and put it into crtjava*.o instead, and only link in if -flink-jcr is > passed or gcj driver used? Or treat -lgcj as that magic switch? > > Or, alternatively, at least for selected targets, live with the extra 8 bytes > in .jcr section for every binary/shared library, but move the > _Jv_RegisterClasses call into libgcj_nonshared.a and libgcj.a and make > libgcj.so be a linker script containing both libgcj_nonshared.a and libgcj.so.*. 8 bytes of RAM is precious for embedded system. As Tom pointed out that a complete solution need significantly more work, I'd prefer to enable this simple fix. OK for trunk? Thanks, Joey
Ping, as wasting 8 bytes of RAM isn't ignorable on embedded system. OK to trunk stage 1? > -----Original Message----- > From: Tom Tromey [mailto:tromey@redhat.com] > Sent: Thursday, October 10, 2013 21:32 > To: Jakub Jelinek > Cc: Joey Ye; per@bothner.com; aph@redhat.com; H.J. Lu; gcc-patches; 'Ian > Lance Taylor' > Subject: Re: [PATCH, libgcc] Disable JCR section when java is not enabled > > Jakub> Given the state of gcj that it is now only rarely used and most > Jakub> people just use OpenJDK instead, wouldn't it be a good idea to > Jakub> just require that gcj code is linked using gcj driver or, if > Jakub> linked in any other driver, just using a special non-default > Jakub> option (-flink-jcr or similar), that would be automatically set > Jakub> by gcj driver, move this JCR stuff out of the normal crt* files > Jakub> and put it into crtjava*.o instead, and only link in if > Jakub> -flink-jcr is passed or gcj driver used? Or treat -lgcj as that > Jakub> magic switch? > > The irony of the situation is that this would require significantly more work > than has gone into gcj in the past N years. > > Jakub> Also, looking at crtstuff.c makes me wonder where are classes > Jakub> deregistered, there are only calls to _Jv_RegisterClasses, but > Jakub> never to to deregistration, wonder what happens if you dlclose a > Jakub> shared library with registered classes. > > I think we never implemented class GC for compiled classes, though it's hard > to remember. > > Tom
Index: Makefile.in =================================================================== --- Makefile.in (revision 194467) +++ Makefile.in (working copy) @@ -281,7 +281,8 @@ -finhibit-size-directive -fno-inline -fno-exceptions \ -fno-zero-initialized-in-bss -fno-toplevel-reorder -fno-tree-vectorize \ -fno-stack-protector \ - $(INHIBIT_LIBC_CFLAGS) + $(INHIBIT_LIBC_CFLAGS) \ + -DJAVA_IS_ENABLED=@java_is_enabled@ # Extra flags to use when compiling crt{begin,end}.o. CRTSTUFF_T_CFLAGS = Index: configure.ac =================================================================== --- configure.ac (revision 194467) +++ configure.ac (working copy) @@ -204,6 +204,17 @@ esac], [enable_sjlj_exceptions=auto]) +# Disable jcr section if we are not building java +case ,${enable_languages}, in + *,java,*) + java_is_enabled=1 + ;; + *) + java_is_enabled=0 + ;; +esac +AC_SUBST(java_is_enabled) + AC_CACHE_CHECK([whether to use setjmp/longjmp exceptions], [libgcc_cv_lib_sjlj_exceptions], [AC_LANG_CONFTEST( Index: crtstuff.c =================================================================== --- crtstuff.c (revision 194467) +++ crtstuff.c (working copy) @@ -145,6 +145,10 @@ # define USE_TM_CLONE_REGISTRY 1 #endif +#if !JAVA_IS_ENABLED +#undef JCR_SECTION_NAME +#endif + /* We do not want to add the weak attribute to the declarations of these routines in unwind-dw2-fde.h because that will cause the definition of these symbols to be weak as well. Index: configure =================================================================== --- configure (revision 194467) +++ configure (working copy) @@ -566,6 +566,7 @@ set_use_emutls set_have_cc_tls vis_hide +java_is_enabled fixed_point enable_decimal_float decimal_float @@ -4191,6 +4192,17 @@ fi +# Disable jcr section if we are not building java +case ,${enable_languages}, in + *,java,*) + java_is_enabled=1 + ;; + *) + java_is_enabled=0 + ;; +esac + + { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to use setjmp/longjmp exceptions" >&5 $as_echo_n "checking whether to use setjmp/longjmp exceptions... " >&6; }
Updated to http://gcc.gnu.org/ml/gcc-patches/2012-12/msg01097.html Build passes on arm-none-eabi and bootstrap passes on x86. OK to trunk? ChangeLog * libgcc/Makefile.in: Include JAVA_IS_ENABLED in CFLAGS. * libgcc/configure.ac (java_is_enabled): New variable. * libgcc/configure: Regenerated. * libgcc/crtstuff.c: Check JAVA_IS_ENABLED. if test "${libgcc_cv_lib_sjlj_exceptions+set}" = set; then :