Re: [PATCH 3/3] libgcc: Implement TARGET_LIBGCC_REMOVE_DSO_HANDLE
On Wed, 2019-12-11 at 11:49 +, Jozef Lawrynowicz wrote:
> On Mon, 9 Dec 2019 13:05:22 +
> Jozef Lawrynowicz wrote:
>
> > On Sat, 07 Dec 2019 11:27:54 -0700
> > Jeff Law wrote:
> >
> > > On Wed, 2019-11-06 at 16:19 +, Jozef Lawrynowicz wrote:
> > > > From 7bc0971d2936ebe71e7b7d3d805cf1bbf9f9f5af Mon Sep 17
> > > > 00:00:00 2001
> > > > From: Jozef Lawrynowicz
> > > > Date: Mon, 4 Nov 2019 17:38:13 +
> > > > Subject: [PATCH 3/3] libgcc: Implement
> > > > TARGET_LIBGCC_REMOVE_DSO_HANDLE
> > > >
> > > > gcc/ChangeLog:
> > > >
> > > > 2019-11-06 Jozef Lawrynowicz
> > > >
> > > > * doc/tm.texi: Regenerate.
> > > > * doc/tm.texi.in: Define
> > > > TARGET_LIBGCC_REMOVE_DSO_HANDLE.
> > > >
> > > > libgcc/ChangeLog:
> > > >
> > > > 2019-11-06 Jozef Lawrynowicz
> > > >
> > > > * crtstuff.c: Don't declare __dso_handle if
> > > > TARGET_LIBGCC_REMOVE_DSO_HANDLE is defined.
> > > Presumably you'll switch this on for your bare elf target
> > > configuration?
> >
> > Yep that's the plan. I originally didn't include the target changes
> > in
> > this patch since other target changes (disabling __cxa_atexit) were
> > required for
> > the removal of __dso_handle to be OK.
> >
> > > Are there other things, particularly related to shared library
> > > support,
> > > that we wouldn't need to use as well? The reason I ask is I'm
> > > trying
> > > to figure out if REMOVE_DSO_HANDLE is the right name or if we
> > > should
> > > generalize it to a name that indicates shared libraries in
> > > general
> > > aren't supported on the target.
> >
> > CRTSTUFFS_O is defined when compiling shared versions of
> > crt{begin,end} and
> > handles an extra case in crtstuff.c where there's some shared
> > library related
> > functionality we don't need on MSP430.
> >
> > But when CRTSTUFFS_O is undefined __dso_handle is still declared -
> > to 0. The
> > comment gives some additional insight:
> >
> > /* Declare the __dso_handle variable. It should have a unique
> > value
> >in every shared-object; in a main program its value is
> > zero. The
> >object should in any case be protected. This means the
> > instance
> >in one DSO or the main program is not used in another
> > object. The
> >dynamic linker takes care of
> > this. */
> >
> > I haven't noticed any further shared library-related bloat coming
> > from libgcc.
> >
> > I think a better way of solving this problem is just to check
> > DEFAULT_USE_CXA_ATEXIT rather than adding this new macro. If
> > __cxa_atexit is
> > not enabled then as far as I understand __dso_handle serves no
> > purpose.
> > DEFAULT_USE_CXA_ATEXIT is defined at configure time for any targets
> > that want
> > __cxa_atexit support.
> >
> > A quick bootstrap and test of dg.exp on x86_64-pc-linux-gnu shows
> > no issues
> > with the following:
> >
> > > diff --git a/libgcc/crtstuff.c b/libgcc/crtstuff.c
> > > index ae6328d317d..349f8191e61 100644
> > > --- a/libgcc/crtstuff.c
> > > +++ b/libgcc/crtstuff.c
> > > @@ -340,8 +340,10 @@ extern void *__dso_handle __attribute__
> > > ((__visibility__ ("hidden")));
> > > #ifdef CRTSTUFFS_O
> > > void *__dso_handle = &__dso_handle;
> > > #else
> > > +#if DEFAULT_USE_CXA_ATEXIT
> > > void *__dso_handle = 0;
> > > #endif
> > > +#endif
> > >
> > > /* The __cxa_finalize function may not be available so we use
> > > only a
> > > weak declaration. */
> >
> > I'll put that patch through some more rigorous testing.
>
> Successfully bootstrapped and regtested the attached patch for
> x86_64-pc-linux-gnu (where DEFAULT_USE_CXA_ATEXIT is set to 1) and
> the proposed
> msp430-elfbare target (where DEFAULT_USE_CXA_ATEXIT is set to 0).
>
> > libgcc/ChangeLog:
> >
> > 2019-12-11 Jozef Lawrynowicz
> >
> > * crtstuff.c: Declare __dso_handle only if
> > DEFAULT_USE_CXA_ATEXIT is
> > true.
OK
jeff
ps. Sorry about the formatting. Had to switch MUAs last week and
still haven't gotten all the kinks worked out.
Re: [PATCH 3/3] libgcc: Implement TARGET_LIBGCC_REMOVE_DSO_HANDLE
On Mon, 9 Dec 2019 13:05:22 +
Jozef Lawrynowicz wrote:
> On Sat, 07 Dec 2019 11:27:54 -0700
> Jeff Law wrote:
>
> > On Wed, 2019-11-06 at 16:19 +, Jozef Lawrynowicz wrote:
> > > From 7bc0971d2936ebe71e7b7d3d805cf1bbf9f9f5af Mon Sep 17 00:00:00 2001
> > > From: Jozef Lawrynowicz
> > > Date: Mon, 4 Nov 2019 17:38:13 +
> > > Subject: [PATCH 3/3] libgcc: Implement TARGET_LIBGCC_REMOVE_DSO_HANDLE
> > >
> > > gcc/ChangeLog:
> > >
> > > 2019-11-06 Jozef Lawrynowicz
> > >
> > > * doc/tm.texi: Regenerate.
> > > * doc/tm.texi.in: Define TARGET_LIBGCC_REMOVE_DSO_HANDLE.
> > >
> > > libgcc/ChangeLog:
> > >
> > > 2019-11-06 Jozef Lawrynowicz
> > >
> > > * crtstuff.c: Don't declare __dso_handle if
> > > TARGET_LIBGCC_REMOVE_DSO_HANDLE is defined.
> > Presumably you'll switch this on for your bare elf target
> > configuration?
>
> Yep that's the plan. I originally didn't include the target changes in
> this patch since other target changes (disabling __cxa_atexit) were required
> for
> the removal of __dso_handle to be OK.
>
> >
> > Are there other things, particularly related to shared library support,
> > that we wouldn't need to use as well? The reason I ask is I'm trying
> > to figure out if REMOVE_DSO_HANDLE is the right name or if we should
> > generalize it to a name that indicates shared libraries in general
> > aren't supported on the target.
>
> CRTSTUFFS_O is defined when compiling shared versions of crt{begin,end} and
> handles an extra case in crtstuff.c where there's some shared library related
> functionality we don't need on MSP430.
>
> But when CRTSTUFFS_O is undefined __dso_handle is still declared - to 0. The
> comment gives some additional insight:
>
> /* Declare the __dso_handle variable. It should have a unique value
>in every shared-object; in a main program its value is zero. The
>object should in any case be protected. This means the instance
>in one DSO or the main program is not used in another object. The
>dynamic linker takes care of this. */
>
> I haven't noticed any further shared library-related bloat coming from libgcc.
>
> I think a better way of solving this problem is just to check
> DEFAULT_USE_CXA_ATEXIT rather than adding this new macro. If __cxa_atexit is
> not enabled then as far as I understand __dso_handle serves no purpose.
> DEFAULT_USE_CXA_ATEXIT is defined at configure time for any targets that want
> __cxa_atexit support.
>
> A quick bootstrap and test of dg.exp on x86_64-pc-linux-gnu shows no issues
> with the following:
>
> > diff --git a/libgcc/crtstuff.c b/libgcc/crtstuff.c
> > index ae6328d317d..349f8191e61 100644
> > --- a/libgcc/crtstuff.c
> > +++ b/libgcc/crtstuff.c
> > @@ -340,8 +340,10 @@ extern void *__dso_handle __attribute__
> > ((__visibility__ ("hidden")));
> > #ifdef CRTSTUFFS_O
> > void *__dso_handle = &__dso_handle;
> > #else
> > +#if DEFAULT_USE_CXA_ATEXIT
> > void *__dso_handle = 0;
> > #endif
> > +#endif
> >
> > /* The __cxa_finalize function may not be available so we use only a
> > weak declaration. */
>
> I'll put that patch through some more rigorous testing.
Successfully bootstrapped and regtested the attached patch for
x86_64-pc-linux-gnu (where DEFAULT_USE_CXA_ATEXIT is set to 1) and the proposed
msp430-elfbare target (where DEFAULT_USE_CXA_ATEXIT is set to 0).
Ok to apply?
>
> Thanks,
> Jozef
> >
> > Jeff
>From fc2564803c33229184926a5ac6db62ae36ea8d77 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz
Date: Mon, 9 Dec 2019 15:20:23 +
Subject: [PATCH] libgcc: Only define __dso_handle if DEFAULT_USE_CXA_ATEXIT is
true
libgcc/ChangeLog:
2019-12-11 Jozef Lawrynowicz
* crtstuff.c: Declare __dso_handle only if DEFAULT_USE_CXA_ATEXIT is
true.
---
libgcc/crtstuff.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/libgcc/crtstuff.c b/libgcc/crtstuff.c
index 9346cc5ca54..e282cb1aabb 100644
--- a/libgcc/crtstuff.c
+++ b/libgcc/crtstuff.c
@@ -325,11 +325,14 @@ register_tm_clones (void)
#ifdef OBJECT_FORMAT_ELF
+#if DEFAULT_USE_CXA_ATEXIT
/* Declare the __dso_handle variable. It should have a unique value
in every shared-object; in a main program its value is zero. The
object should in any case be protected. This means the instance
in one DSO or the main program is not used in another object. The
- dynamic linker takes care of this. */
+ dynamic linker takes care of this.
+ If __cxa_atexit is not being used, __dso_handle will not be used and
+ doesn't need to be defined. */
#ifdef TARGET_LIBGCC_SDATA_SECTION
extern void *__dso_handle __attribute__ ((__section__ (TARGET_LIBGCC_SDATA_SECTION)));
@@ -342,6 +345,7 @@ void *__dso_handle = &__dso_handle;
#else
void *__dso_handle = 0;
#endif
+#endif /* DEFAULT_USE_CXA_ATEXIT */
/* The __cxa_finalize function may not be available so we use only a
weak declaration. */
--
2.17.1
Re: [PATCH 3/3] libgcc: Implement TARGET_LIBGCC_REMOVE_DSO_HANDLE
On Sat, 07 Dec 2019 11:27:54 -0700
Jeff Law wrote:
> On Wed, 2019-11-06 at 16:19 +, Jozef Lawrynowicz wrote:
> > From 7bc0971d2936ebe71e7b7d3d805cf1bbf9f9f5af Mon Sep 17 00:00:00 2001
> > From: Jozef Lawrynowicz
> > Date: Mon, 4 Nov 2019 17:38:13 +0000
> > Subject: [PATCH 3/3] libgcc: Implement TARGET_LIBGCC_REMOVE_DSO_HANDLE
> >
> > gcc/ChangeLog:
> >
> > 2019-11-06 Jozef Lawrynowicz
> >
> > * doc/tm.texi: Regenerate.
> > * doc/tm.texi.in: Define TARGET_LIBGCC_REMOVE_DSO_HANDLE.
> >
> > libgcc/ChangeLog:
> >
> > 2019-11-06 Jozef Lawrynowicz
> >
> > * crtstuff.c: Don't declare __dso_handle if
> > TARGET_LIBGCC_REMOVE_DSO_HANDLE is defined.
> Presumably you'll switch this on for your bare elf target
> configuration?
Yep that's the plan. I originally didn't include the target changes in
this patch since other target changes (disabling __cxa_atexit) were required for
the removal of __dso_handle to be OK.
>
> Are there other things, particularly related to shared library support,
> that we wouldn't need to use as well? The reason I ask is I'm trying
> to figure out if REMOVE_DSO_HANDLE is the right name or if we should
> generalize it to a name that indicates shared libraries in general
> aren't supported on the target.
CRTSTUFFS_O is defined when compiling shared versions of crt{begin,end} and
handles an extra case in crtstuff.c where there's some shared library related
functionality we don't need on MSP430.
But when CRTSTUFFS_O is undefined __dso_handle is still declared - to 0. The
comment gives some additional insight:
/* Declare the __dso_handle variable. It should have a unique value
in every shared-object; in a main program its value is zero. The
object should in any case be protected. This means the instance
in one DSO or the main program is not used in another object. The
dynamic linker takes care of this. */
I haven't noticed any further shared library-related bloat coming from libgcc.
I think a better way of solving this problem is just to check
DEFAULT_USE_CXA_ATEXIT rather than adding this new macro. If __cxa_atexit is
not enabled then as far as I understand __dso_handle serves no purpose.
DEFAULT_USE_CXA_ATEXIT is defined at configure time for any targets that want
__cxa_atexit support.
A quick bootstrap and test of dg.exp on x86_64-pc-linux-gnu shows no issues
with the following:
> diff --git a/libgcc/crtstuff.c b/libgcc/crtstuff.c
> index ae6328d317d..349f8191e61 100644
> --- a/libgcc/crtstuff.c
> +++ b/libgcc/crtstuff.c
> @@ -340,8 +340,10 @@ extern void *__dso_handle __attribute__ ((__visibility__
> ("hidden")));
> #ifdef CRTSTUFFS_O
> void *__dso_handle = &__dso_handle;
> #else
> +#if DEFAULT_USE_CXA_ATEXIT
> void *__dso_handle = 0;
> #endif
> +#endif
>
> /* The __cxa_finalize function may not be available so we use only a
> weak declaration. */
I'll put that patch through some more rigorous testing.
Thanks,
Jozef
>
> Jeff
Re: [PATCH 3/3] libgcc: Implement TARGET_LIBGCC_REMOVE_DSO_HANDLE
On Wed, 2019-11-06 at 16:19 +, Jozef Lawrynowicz wrote: > From 7bc0971d2936ebe71e7b7d3d805cf1bbf9f9f5af Mon Sep 17 00:00:00 2001 > From: Jozef Lawrynowicz > Date: Mon, 4 Nov 2019 17:38:13 + > Subject: [PATCH 3/3] libgcc: Implement TARGET_LIBGCC_REMOVE_DSO_HANDLE > > gcc/ChangeLog: > > 2019-11-06 Jozef Lawrynowicz > > * doc/tm.texi: Regenerate. > * doc/tm.texi.in: Define TARGET_LIBGCC_REMOVE_DSO_HANDLE. > > libgcc/ChangeLog: > > 2019-11-06 Jozef Lawrynowicz > > * crtstuff.c: Don't declare __dso_handle if > TARGET_LIBGCC_REMOVE_DSO_HANDLE is defined. Presumably you'll switch this on for your bare elf target configuration? Are there other things, particularly related to shared library support, that we wouldn't need to use as well? The reason I ask is I'm trying to figure out if REMOVE_DSO_HANDLE is the right name or if we should generalize it to a name that indicates shared libraries in general aren't supported on the target. Jeff
[PATCH 3/3] libgcc: Implement TARGET_LIBGCC_REMOVE_DSO_HANDLE
For some targets __dso_handle will never be used, and its definition in
crtstuff.c can cause a domino effect resulting in the size of the final
executable increasing much more than just the size of this piece of data.
For msp430, CRT functions to initialize global data are only included if there
is global data to initialize and it is more than feasible to write functional
programs which do not use any global data. In these cases, the definition of
__dso_handle will cause code size to increase by around 100 bytes as library
code to initialize data is linked into the executable.
Removing __dso_handle can therefore save on code size.
This does require the target to NOT use __cxa_atexit, so either
TARGET_CXX_USE_ATEXIT_FOR_CXA_ATEXIT must return true or -fno-use-cxa-atexit
must be used as a target flag when building GCC.
This is because __cxa_atexit includes functionality to unload dynamic shared
objects and so cp/decl.c will create a reference to __dso_handle to facilitate
this in programs with static destructors.
>From 7bc0971d2936ebe71e7b7d3d805cf1bbf9f9f5af Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz
Date: Mon, 4 Nov 2019 17:38:13 +
Subject: [PATCH 3/3] libgcc: Implement TARGET_LIBGCC_REMOVE_DSO_HANDLE
gcc/ChangeLog:
2019-11-06 Jozef Lawrynowicz
* doc/tm.texi: Regenerate.
* doc/tm.texi.in: Define TARGET_LIBGCC_REMOVE_DSO_HANDLE.
libgcc/ChangeLog:
2019-11-06 Jozef Lawrynowicz
* crtstuff.c: Don't declare __dso_handle if
TARGET_LIBGCC_REMOVE_DSO_HANDLE is defined.
---
gcc/doc/tm.texi| 11 +++
gcc/doc/tm.texi.in | 11 +++
libgcc/crtstuff.c | 2 ++
3 files changed, 24 insertions(+)
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index cd9aed9874f..89ef0a8e616 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -11425,6 +11425,17 @@ from shared libraries (DLLs).
You need not define this macro if it would always evaluate to zero.
@end defmac
+@defmac TARGET_LIBGCC_REMOVE_DSO_HANDLE
+Define this macro if, for targets where dynamic shared objects cannot be used,
+the declaration of @samp{__dso_handle} in libgcc/crtstuff.c can be removed.
+
+If this macro is defined, __cxa_atexit must be disabled at GCC configure time,
+otherwise references to __dso_handle will be created when the middle-end
+creates destructors for static objects.
+
+This macro is undefined by default.
+@end defmac
+
@deftypefn {Target Hook} {rtx_insn *} TARGET_MD_ASM_ADJUST (vec& @var{outputs}, vec& @var{inputs}, vec& @var{constraints}, vec& @var{clobbers}, HARD_REG_SET& @var{clobbered_regs})
This target hook may add @dfn{clobbers} to @var{clobbers} and
@var{clobbered_regs} for any hard regs the port wishes to automatically
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 2739e9ceec5..3a211a7658a 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -7853,6 +7853,17 @@ from shared libraries (DLLs).
You need not define this macro if it would always evaluate to zero.
@end defmac
+@defmac TARGET_LIBGCC_REMOVE_DSO_HANDLE
+Define this macro if, for targets where dynamic shared objects cannot be used,
+the declaration of @samp{__dso_handle} in libgcc/crtstuff.c can be removed.
+
+If this macro is defined, __cxa_atexit must be disabled at GCC configure time,
+otherwise references to __dso_handle will be created when the middle-end
+creates destructors for static objects.
+
+This macro is undefined by default.
+@end defmac
+
@hook TARGET_MD_ASM_ADJUST
@defmac MATH_LIBRARY
diff --git a/libgcc/crtstuff.c b/libgcc/crtstuff.c
index 0b0a0b865fe..d1d17f959d3 100644
--- a/libgcc/crtstuff.c
+++ b/libgcc/crtstuff.c
@@ -335,6 +335,7 @@ register_tm_clones (void)
in one DSO or the main program is not used in another object. The
dynamic linker takes care of this. */
+#ifndef TARGET_LIBGCC_REMOVE_DSO_HANDLE
#ifdef TARGET_LIBGCC_SDATA_SECTION
extern void *__dso_handle __attribute__ ((__section__ (TARGET_LIBGCC_SDATA_SECTION)));
#endif
@@ -346,6 +347,7 @@ void *__dso_handle = &__dso_handle;
#else
void *__dso_handle = 0;
#endif
+#endif /* TARGET_LIBGCC_REMOVE_DSO_HANDLE */
/* The __cxa_finalize function may not be available so we use only a
weak declaration. */
--
2.17.1
