public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
To: Oleg Endo <oleg.endo@t-online.de>
Cc: "gcc-patches@gcc.gnu.org\"  <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH 2/4] MSP430: Disable exception handling by default for C++
Date: Fri, 08 Nov 2019 13:26:00 -0000	[thread overview]
Message-ID: <20191108132608.7a774581@jozef-kubuntu> (raw)
In-Reply-To: <8b705a60e10ebf94e432476bb6b738b8fec09094.camel@t-online.de>

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

On Fri, 08 Nov 2019 09:07:39 +0900
Oleg Endo <oleg.endo@t-online.de> wrote:

> On Thu, 2019-11-07 at 21:37 +0000, Jozef Lawrynowicz wrote:
> > The code size bloat added by building C++ programs using libraries containing
> > support for exceptions is significant. When using simple constructs such as
> > static variables, sometimes many kB from the libraries are unnecessarily
> > pulled in.
> > 
> > So this patch disable exceptions by default for MSP430 when compiling for C++,
> > by implicitly passing -fno-exceptions unless -fexceptions is passed.  
> 
> It is extremely annoying when GCC's default standard behavior differs
> across different targets.  And as a consequence, you have to add a load
> of workarounds and disable other things, like fiddling with the
> testsuite.  It's the same thing as setting "double = float" to get more
> "speed" by default.
> 
> I would strongly advice against making such non-standard behaviors the
> default in the vanilla compiler.  C++ normally has exceptions enabled. 
> If a user doesn't want them and is willing to deal with it all the
> consequences, then we already have a mechanism to do that:
>  --fno-exceptions
> 
> Perhaps it's generally more useful to add a global configure option for
> GCC to disable exception handling by default.  Then you can provide a
> turn-key toolchain to your customers as well -- just add an option to
> the configure line.
> 
> Cheers,
> Oleg
> 

Fair point, I probably should have realised whilst implementing all the
testsuite workarounds that this wasn't the best choice for upstream GCC and
integrating nicely with the testsuite.

So I've regtested and attached a revised patch to instead build -fno-exceptions
multilibs, so the reduced code size can still be achieved by passing with
-fno-exceptions.

And the --disable-no-exceptions multilib option is added to reduce build time
for developers.

Thanks for providing your input,
Jozef

[-- Attachment #2: 0002-MSP430-Add-fno-exceptions-multilib.patch --]
[-- Type: text/x-patch, Size: 6295 bytes --]

From fe67a5ff71bc48af05b086b2d495fbf77e1a070d Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Fri, 8 Nov 2019 10:47:26 +0000
Subject: [PATCH 2/4] MSP430: Add -fno-exceptions multilib

ChangeLog:

2019-11-08  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* config-ml.in: Support --disable-no-exceptions configure flag.

gcc/ChangeLog:

2019-11-08  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* config/msp430/msp430.h (STARTFILE_SPEC) [fno-exceptions]: Use
	crtbegin_no_eh.o.
	(ENDFILE_SPEC) [fno-exceptions]: Use crtend_no_eh.o.
	* config/msp430/t-msp430: Add -fno-exceptions multilib.
	* doc/install.texi: Document --disable-no-exceptions multilib configure
	option.

gcc/testsuite/ChangeLog:

2019-11-08  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* lib/gcc-dg.exp: Add dg-prune messages for when exception handling is
	disabled.

libgcc/ChangeLog:

2019-11-08  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* config.host: Add crt{begin,end}_no_eh.o to "extra_parts".
	* config/msp430/t-msp430: Add rules to build crt{begin,end}_no_eh.o.

---
 config-ml.in                  | 13 +++++++++++++
 gcc/config/msp430/msp430.h    |  6 ++++--
 gcc/config/msp430/t-msp430    |  9 +++++----
 gcc/doc/install.texi          |  3 +++
 gcc/testsuite/lib/gcc-dg.exp  | 10 ++++++++++
 libgcc/config.host            |  3 ++-
 libgcc/config/msp430/t-msp430 |  6 ++++++
 7 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/config-ml.in b/config-ml.in
index 3e37f875c88..5720d38d23f 100644
--- a/config-ml.in
+++ b/config-ml.in
@@ -383,6 +383,19 @@ mips*-*-*)
 	  done
 	fi
 	;;
+msp430-*-*)
+	if [ x$enable_no_exceptions = xno ]
+	then
+	  old_multidirs="${multidirs}"
+	  multidirs=""
+	  for x in ${old_multidirs}; do
+	    case "$x" in
+	      *no-exceptions* ) : ;;
+	      *) multidirs="${multidirs} ${x}" ;;
+	    esac
+	  done
+	fi
+	;;
 powerpc*-*-* | rs6000*-*-*)
 	if [ x$enable_aix64 = xno ]
 	then
diff --git a/gcc/config/msp430/msp430.h b/gcc/config/msp430/msp430.h
index 73afe2e2d16..4d796f67d1b 100644
--- a/gcc/config/msp430/msp430.h
+++ b/gcc/config/msp430/msp430.h
@@ -46,11 +46,13 @@ extern bool msp430x;
 
 #undef  STARTFILE_SPEC
 #define STARTFILE_SPEC "%{pg:gcrt0.o%s}" \
-  "%{!pg:%{minrt:crt0-minrt.o%s}%{!minrt:crt0.o%s}} %{!minrt:crtbegin.o%s}"
+  "%{!pg:%{minrt:crt0-minrt.o%s}%{!minrt:crt0.o%s}} " \
+  "%{!minrt:%{fno-exceptions:crtbegin_no_eh.o%s; :crtbegin.o%s}}"
 
 /* -lgcc is included because crtend.o needs __mspabi_func_epilog_1.  */
 #undef  ENDFILE_SPEC
-#define ENDFILE_SPEC "%{!minrt:crtend.o%s} " \
+#define ENDFILE_SPEC \
+  "%{!minrt:%{fno-exceptions:crtend_no_eh.o%s; :crtend.o%s}} "  \
   "%{minrt:%:if-exists(crtn-minrt.o%s)}%{!minrt:%:if-exists(crtn.o%s)} -lgcc"
 
 #define ASM_SPEC "-mP " /* Enable polymorphic instructions.  */ \
diff --git a/gcc/config/msp430/t-msp430 b/gcc/config/msp430/t-msp430
index f8ba7751123..e180ce3efdb 100644
--- a/gcc/config/msp430/t-msp430
+++ b/gcc/config/msp430/t-msp430
@@ -28,8 +28,8 @@ msp430-devices.o: $(srcdir)/config/msp430/msp430-devices.c \
 
 # Enable multilibs:
 
-MULTILIB_OPTIONS    = mcpu=msp430 mlarge  mdata-region=none
-MULTILIB_DIRNAMES   = 430	   large  full-memory-range
+MULTILIB_OPTIONS    = mcpu=msp430 mlarge  mdata-region=none fno-exceptions
+MULTILIB_DIRNAMES   = 430	   large  full-memory-range no-exceptions
 
 # Match -mcpu=430
 MULTILIB_MATCHES    = mcpu?msp430=mcpu?430
@@ -41,9 +41,10 @@ MULTILIB_MATCHES   += mdata-region?none=mdata-region?either
 # hard-coded data here, because DRIVER_SELF_SPECS will place the correct
 # -mcpu option for a given mcu onto the command line.
 
-MULTILIB_REQUIRED = mcpu=msp430
-MULTILIB_REQUIRED += mlarge
+MULTILIB_REQUIRED =		    mcpu=msp430		       mlarge
+MULTILIB_REQUIRED += fno-exceptions mcpu=msp430/fno-exceptions mlarge/fno-exceptions
 MULTILIB_REQUIRED += mlarge/mdata-region=none
+MULTILIB_REQUIRED += mlarge/mdata-region=none/fno-exceptions
 
 
 MULTILIB_EXTRA_OPTS =
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 563de705881..5250547f98e 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -1087,6 +1087,9 @@ softfloat, m68881, m68000, m68020.
 @item mips*-*-*
 single-float, biendian, softfloat.
 
+@item msp430-*-*
+no-exceptions
+
 @item powerpc*-*-*, rs6000*-*-*
 aix64, pthread, softfloat, powercpu, powerpccpu, powerpcos, biendian,
 sysv, aix.
diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
index 1df645e283c..1ce449cb935 100644
--- a/gcc/testsuite/lib/gcc-dg.exp
+++ b/gcc/testsuite/lib/gcc-dg.exp
@@ -417,6 +417,16 @@ proc gcc-dg-prune { system text } {
 	return "::unsupported::large return values"
     }
 
+    # If exceptions are disabled, mark tests expecting exceptions to be enabled
+    # as unsupported.
+    if [regexp "(^|\n)\[^\n\]*: error: exception handling disabled" $text] {
+	return "::unsupported::exception handling disabled"
+    }
+
+    if [regexp "(^|\n)\[^\n\]*: error: #error .__cpp_exceptions." $text] {
+	return "::unsupported::exception handling disabled"
+    }
+
     return $text
 }
 
diff --git a/libgcc/config.host b/libgcc/config.host
index 122113fc519..99cd5d3ae7c 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -1029,7 +1029,8 @@ moxie-*-elf | moxie-*-moxiebox* | moxie-*-uclinux* | moxie-*-rtems*)
 	;;
 msp430*-*-elf)
 	tmake_file="$tm_file t-crtstuff t-fdpbit msp430/t-msp430"
-        extra_parts="$extra_parts libmul_none.a libmul_16.a libmul_32.a libmul_f5.a"
+	extra_parts="$extra_parts crtbegin_no_eh.o crtend_no_eh.o"
+	extra_parts="$extra_parts libmul_none.a libmul_16.a libmul_32.a libmul_f5.a"
 	;;
 nds32*-linux*)
 	# Basic makefile fragment and extra_parts for crt stuff.
diff --git a/libgcc/config/msp430/t-msp430 b/libgcc/config/msp430/t-msp430
index 17d85b6cb23..72ae93a8dae 100644
--- a/libgcc/config/msp430/t-msp430
+++ b/libgcc/config/msp430/t-msp430
@@ -42,6 +42,12 @@ LIB2ADD = \
 
 HOST_LIBGCC2_CFLAGS += -Os -ffunction-sections -fdata-sections -mhwmult=none
 
+crtbegin_no_eh.o: $(srcdir)/crtstuff.c
+	$(crt_compile) -U__LIBGCC_EH_FRAME_SECTION_NAME__ -c $< -DCRT_BEGIN
+
+crtend_no_eh.o: $(srcdir)/crtstuff.c
+	$(crt_compile) -U__LIBGCC_EH_FRAME_SECTION_NAME__ -c $< -DCRT_END
+
 mpy.o: $(srcdir)/config/msp430/mpy.c
 	$(gcc_compile) $< -c
 
-- 
2.17.1


  reply	other threads:[~2019-11-08 13:26 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-07 21:31 [PATCH 0/4][MSP430] Tweaks to default configuration to reduce code size Jozef Lawrynowicz
2019-11-07 21:34 ` [PATCH 1/4] MSP430: Disable TM clone registry by default Jozef Lawrynowicz
2019-11-17 19:32   ` Jeff Law
2019-11-24 14:22     ` Jozef Lawrynowicz
2019-11-24 17:24       ` Jeff Law
2019-11-24 17:55         ` Jozef Lawrynowicz
2019-11-07 21:37 ` [PATCH 2/4] MSP430: Disable exception handling by default for C++ Jozef Lawrynowicz
2019-11-08  0:07   ` Oleg Endo
2019-11-08 13:26     ` Jozef Lawrynowicz [this message]
2019-11-12 21:13       ` Richard Sandiford
2019-11-27 13:51         ` Jozef Lawrynowicz
2019-11-07 21:39 ` [PATCH 3/4] MSP430: Disable __cxa_atexit Jozef Lawrynowicz
2019-11-07 21:41 ` [PATCH 4/4] MSP430: Deprecate -minrt option Jozef Lawrynowicz
2019-11-17 21:02   ` Jeff Law
2019-11-24 16:38     ` Jozef Lawrynowicz
2019-11-08 12:14 ` [PATCH 0/4][MSP430] Tweaks to default configuration to reduce code size Oleg Endo
2019-11-08 13:27   ` Jozef Lawrynowicz
2019-11-08 13:59     ` Oleg Endo
2019-11-08 14:32       ` Jozef Lawrynowicz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191108132608.7a774581@jozef-kubuntu \
    --to=jozef.l@mittosystems.com \
    --cc=oleg.endo@t-online.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).