diff mbox series

Add --disable-tm-clone-registry libgcc configure option.

Message ID 8f106efb5581e875ad121e7e9fe2280a@optimitech.com
State New
Headers show
Series Add --disable-tm-clone-registry libgcc configure option. | expand

Commit Message

Ilia Diachkov June 13, 2019, 12:56 a.m. UTC
Hello,

This patch adds libgcc configuration option to disable TM clone 
registry. This option helps to reduce code size for embedded targets 
which do not need transactional memory support.

Tested on x86_64 and riscv64-unknown-elf-gcc, the last is with the 
option enabled.

If the change is Ok with you, please commit it since I have no write 
access to gcc repository.

Best regards,
Ilia.

libgcc/ChangeLog:

         * Makefile.in: Add @use_tm_clone_registry@.
         * configure: Regenerate.
         * configure.ac: Add --disable-tm-clone-registry option.

gcc/ChangeLog:

         * doc/install.texi: Document --disable-tm-clone-registry.

Comments

Jim Wilson June 20, 2019, 7:50 p.m. UTC | #1
On Wed, Jun 12, 2019 at 5:57 PM <ilia.diachkov@optimitech.com> wrote:
> This patch adds libgcc configuration option to disable TM clone
> registry. This option helps to reduce code size for embedded targets
> which do not need transactional memory support.

This looks OK to me.  It is worth pointing out that ARM already ships
compilers built this way, but they didn't bother adding a configure
option.  They just override Makefile variables in their build scripts.
I think this is much cleaner as a documented configure option.

One issue here is that I don't know the transactional memory support
well enough to know what harm comes when we drop the tm clone registry
support.  Perhaps a change should be made to the compiler to disable
the transactional memory support, or some subset of it.  Maybe Aldy
can comment on that?  The intent here is to only use this for size
constrained embedded targets (e.g. newlib-nano targets), and such
targets are very unlikely to ever want transactional memory support.
But someone might accidentally use this configure option with an
x86_64-linux toolchain, and then get confusing transactional memory
failures, and we might want to try to prevent that before it happens.

Jim
Jim Wilson June 27, 2019, 11:43 p.m. UTC | #2
On Thu, Jun 20, 2019 at 12:50 PM Jim Wilson <jimw@sifive.com> wrote:
> This looks OK to me.  It is worth pointing out that ARM already ships
> compilers built this way, but they didn't bother adding a configure
> option.  They just override Makefile variables in their build scripts.
> I think this is much cleaner as a documented configure option.

It has been a week, and no one else commented on this, so I went ahead
and committed it, with the regenerated configure.

Jim
diff mbox series

Patch

diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 29d0470..1a0e8c7 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -1284,6 +1284,11 @@  assumptions made by the configure test are incorrect.
 Specify that the target does not support TLS.
 This is an alias for @option{--enable-tls=no}.
 
+@item --disable-tm-clone-registry
+Disable TM clone registry in libgcc. It is enabled in libgcc by default.
+This option helps to reduce code size for embedded targets which do
+not use transactional memory.
+
 @item --with-cpu=@var{cpu}
 @itemx --with-cpu-32=@var{cpu}
 @itemx --with-cpu-64=@var{cpu}
diff --git a/libgcc/Makefile.in b/libgcc/Makefile.in
index fb77881..189f9ff 100644
--- a/libgcc/Makefile.in
+++ b/libgcc/Makefile.in
@@ -259,6 +259,8 @@  PICFLAG = @PICFLAG@
 
 CET_FLAGS = @CET_FLAGS@
 
+USE_TM_CLONE_REGISTRY = @use_tm_clone_registry@
+
 # Defined in libgcc2.c, included only in the static library.
 LIB2FUNCS_ST = _eprintf __gcc_bcmp
 
@@ -299,7 +301,7 @@  CRTSTUFF_CFLAGS = -O2 $(GCC_CFLAGS) $(INCLUDES) $(MULTILIB_CFLAGS) -g0 \
   $(NO_PIE_CFLAGS) -finhibit-size-directive -fno-inline -fno-exceptions \
   -fno-zero-initialized-in-bss -fno-toplevel-reorder -fno-tree-vectorize \
   -fbuilding-libgcc -fno-stack-protector $(FORCE_EXPLICIT_EH_REGISTRY) \
-  $(INHIBIT_LIBC_CFLAGS)
+  $(INHIBIT_LIBC_CFLAGS) $(USE_TM_CLONE_REGISTRY)
 
 # Extra flags to use when compiling crt{begin,end}.o.
 CRTSTUFF_T_CFLAGS =
diff --git a/libgcc/configure.ac b/libgcc/configure.ac
index 5f11455..b1b90d2 100644
--- a/libgcc/configure.ac
+++ b/libgcc/configure.ac
@@ -261,6 +261,16 @@  fi
 ])
 AC_SUBST([force_explicit_eh_registry])
 
+AC_ARG_ENABLE([tm-clone-registry],
+[  --disable-tm-clone-registry    disable TM clone registry],
+[
+use_tm_clone_registry=
+if test "$enable_tm_clone_registry" = no; then
+  use_tm_clone_registry=-DUSE_TM_CLONE_REGISTRY=0
+fi
+])
+AC_SUBST([use_tm_clone_registry])
+
 AC_LIB_PROG_LD_GNU
 
 AC_MSG_CHECKING([for thread model used by GCC])