Patchwork [ARM] Don't pull in unwinder for 64-bit division routines

login
register
mail settings
Submitter Julian Brown
Date July 24, 2012, 12:27 p.m.
Message ID <20120724132708.49bc2afc@octopus>
Download mbox | patch
Permalink /patch/172865/
State New
Headers show

Comments

Julian Brown - July 24, 2012, 12:27 p.m.
On Fri, 20 Jul 2012 11:15:27 +0100
Julian Brown <julian@codesourcery.com> wrote:

> Anyway: this revised version of the patch removes the strange libgcc
> Makefile-fragment changes, the equivalent of which have since been
> incorporated into mainline GCC now anyway, so the patch is somewhat
> more straightforward than it was previously.

Joey Ye contacted me offlist and suggested that the t-divmod-ef
fragment might be better integrated into t-bpabi instead. Doing that
makes the patch somewhat smaller/cleaner.

Minimally re-tested, looks OK.

Julian

ChangeLog

    libgcc/
    * Makefile.in (LIB2_DIVMOD_EXCEPTION_FLAGS): Default to
    -fexceptions -fnon-call-exceptions if not defined.
    ($(lib2-divmod-o), $(lib2-divmod-s-o)): Use above.
    * config/arm/t-bpabi (LIB2_DIVMOD_EXCEPTION_FLAGS): Define.

    gcc/testsuite/
    * gcc.target/arm/div64-unwinding.c: New test.
Sebastian Huber - July 30, 2012, 6:59 a.m.
Hello,

with this move to t-bpabi other targets like RTEMS profit also from this 
change.  This is very good since the unwinder pull-in for 64-bit divisions was 
pretty bad for small Cortex-M3 systems with internal flash only.
Ramana Radhakrishnan - Aug. 16, 2012, 6:56 p.m.
On 07/24/12 13:27, Julian Brown wrote:
> On Fri, 20 Jul 2012 11:15:27 +0100
> Julian Brown <julian@codesourcery.com> wrote:
>
>> Anyway: this revised version of the patch removes the strange libgcc
>> Makefile-fragment changes, the equivalent of which have since been
>> incorporated into mainline GCC now anyway, so the patch is somewhat
>> more straightforward than it was previously.
>
> Joey Ye contacted me offlist and suggested that the t-divmod-ef
> fragment might be better integrated into t-bpabi instead. Doing that
> makes the patch somewhat smaller/cleaner.
>
> Minimally re-tested, looks OK.

The original submission makes no mention of testing ? The ARM specific 
portions look OK to me modulo no regressions.

Ian, can also take a quick look.

regards,
Ramana
Julian Brown - Aug. 16, 2012, 7:29 p.m.
On Thu, 16 Aug 2012 19:56:52 +0100
Ramana Radhakrishnan <ramrad01@arm.com> wrote:

> On 07/24/12 13:27, Julian Brown wrote:
> > On Fri, 20 Jul 2012 11:15:27 +0100
> > Julian Brown <julian@codesourcery.com> wrote:
> >
> >> Anyway: this revised version of the patch removes the strange
> >> libgcc Makefile-fragment changes, the equivalent of which have
> >> since been incorporated into mainline GCC now anyway, so the patch
> >> is somewhat more straightforward than it was previously.
> >
> > Joey Ye contacted me offlist and suggested that the t-divmod-ef
> > fragment might be better integrated into t-bpabi instead. Doing that
> > makes the patch somewhat smaller/cleaner.
> >
> > Minimally re-tested, looks OK.
> 
> The original submission makes no mention of testing ? The ARM
> specific portions look OK to me modulo no regressions.

Thanks -- I'm sure I did test the patch, but just omitted to mention
that fact in the mail :-O. We've also been carrying a version of this
patch in our local source base for many years now.

> Ian, can also take a quick look.

Cheers,

Julian
Ian Taylor - Aug. 17, 2012, 1:13 a.m.
On Thu, Aug 16, 2012 at 11:56 AM, Ramana Radhakrishnan <ramrad01@arm.com> wrote:
> On 07/24/12 13:27, Julian Brown wrote:
>>
>> On Fri, 20 Jul 2012 11:15:27 +0100
>> Julian Brown <julian@codesourcery.com> wrote:
>>
>>> Anyway: this revised version of the patch removes the strange libgcc
>>> Makefile-fragment changes, the equivalent of which have since been
>>> incorporated into mainline GCC now anyway, so the patch is somewhat
>>> more straightforward than it was previously.
>>
>>
>> Joey Ye contacted me offlist and suggested that the t-divmod-ef
>> fragment might be better integrated into t-bpabi instead. Doing that
>> makes the patch somewhat smaller/cleaner.
>>
>> Minimally re-tested, looks OK.
>
>
> The original submission makes no mention of testing ? The ARM specific
> portions look OK to me modulo no regressions.
>
> Ian, can also take a quick look.

Looks fine to me.

Ian
Ye Joey - Aug. 21, 2012, 7:47 a.m.
On Fri, Aug 17, 2012 at 9:13 AM, Ian Lance Taylor <iant@google.com> wrote:
>
> Looks fine to me.
>
> Ian
Will backport to arm/embedded-4_7-branch. No sure if appropriate for
4.7 branch since it is not a stability problem.

- Joey
Michael Hope - Aug. 21, 2012, 10:26 p.m.
On 17 August 2012 07:29, Julian Brown <julian@codesourcery.com> wrote:
> On Thu, 16 Aug 2012 19:56:52 +0100
> Ramana Radhakrishnan <ramrad01@arm.com> wrote:
>
>> On 07/24/12 13:27, Julian Brown wrote:
>> > On Fri, 20 Jul 2012 11:15:27 +0100
>> > Julian Brown <julian@codesourcery.com> wrote:
>> >
>> >> Anyway: this revised version of the patch removes the strange
>> >> libgcc Makefile-fragment changes, the equivalent of which have
>> >> since been incorporated into mainline GCC now anyway, so the patch
>> >> is somewhat more straightforward than it was previously.
>> >
>> > Joey Ye contacted me offlist and suggested that the t-divmod-ef
>> > fragment might be better integrated into t-bpabi instead. Doing that
>> > makes the patch somewhat smaller/cleaner.
>> >
>> > Minimally re-tested, looks OK.
>>
>> The original submission makes no mention of testing ? The ARM
>> specific portions look OK to me modulo no regressions.
>
> Thanks -- I'm sure I did test the patch, but just omitted to mention
> that fact in the mail :-O. We've also been carrying a version of this
> patch in our local source base for many years now.

Hi Julian.  The test case fails on arm-linux-gnueabi:
 http://gcc.gnu.org/ml/gcc-testresults/2012-08/msg02100.html

FAIL: gcc.target/arm/div64-unwinding.c execution test

The test aborts as &_Unwind_RaiseException is not null.  _divdi3.o
itself looks fine and no longer pulls in the unwinder so I assume
something else in the environment is.  I've put the binaries up at
http://people.linaro.org/~michaelh/incoming/div64-unwinding.exe and
http://people.linaro.org/~michaelh/incoming/_divdi3.o if that helps.

-- Michael

Patch

Index: libgcc/Makefile.in
===================================================================
--- libgcc/Makefile.in	(revision 189807)
+++ libgcc/Makefile.in	(working copy)
@@ -497,18 +497,24 @@  libgcc-s-objects += $(patsubst %,%_s$(ob
 endif
 endif
 
+ifeq ($(LIB2_DIVMOD_EXCEPTION_FLAGS),)
+# Provide default flags for compiling divmod functions, if they haven't been
+# set already by a target-specific Makefile fragment.
+LIB2_DIVMOD_EXCEPTION_FLAGS := -fexceptions -fnon-call-exceptions
+endif
+
 # Build LIB2_DIVMOD_FUNCS.
 lib2-divmod-o = $(patsubst %,%$(objext),$(LIB2_DIVMOD_FUNCS))
 $(lib2-divmod-o): %$(objext): $(srcdir)/libgcc2.c
 	$(gcc_compile) -DL$* -c $< \
-	  -fexceptions -fnon-call-exceptions $(vis_hide)
+	  $(LIB2_DIVMOD_EXCEPTION_FLAGS) $(vis_hide)
 libgcc-objects += $(lib2-divmod-o)
 
 ifeq ($(enable_shared),yes)
 lib2-divmod-s-o = $(patsubst %,%_s$(objext),$(LIB2_DIVMOD_FUNCS))
 $(lib2-divmod-s-o): %_s$(objext): $(srcdir)/libgcc2.c
 	$(gcc_s_compile) -DL$* -c $< \
-	  -fexceptions -fnon-call-exceptions
+	  $(LIB2_DIVMOD_EXCEPTION_FLAGS)
 libgcc-s-objects += $(lib2-divmod-s-o)
 endif
 
Index: libgcc/config/arm/t-bpabi
===================================================================
--- libgcc/config/arm/t-bpabi	(revision 189807)
+++ libgcc/config/arm/t-bpabi	(working copy)
@@ -13,3 +13,8 @@  LIB2ADDEH = $(srcdir)/config/arm/unwind-
 
 # Add the BPABI names.
 SHLIB_MAPFILES += $(srcdir)/config/arm/libgcc-bpabi.ver
+
+# On ARM, specifying -fnon-call-exceptions will needlessly pull in
+# the unwinder in simple programs which use 64-bit division.  Omitting
+# the option is safe.
+LIB2_DIVMOD_EXCEPTION_FLAGS := -fexceptions
Index: gcc/testsuite/gcc.target/arm/div64-unwinding.c
===================================================================
--- gcc/testsuite/gcc.target/arm/div64-unwinding.c	(revision 0)
+++ gcc/testsuite/gcc.target/arm/div64-unwinding.c	(revision 0)
@@ -0,0 +1,24 @@ 
+/* Performing a 64-bit division should not pull in the unwinder.  */
+
+/* { dg-do run } */
+/* { dg-options "-O0" } */
+
+#include <stdlib.h>
+
+long long
+foo (long long c, long long d)
+{
+  return c/d;
+}
+
+long long x = 0;
+long long y = 1;
+
+extern int (*_Unwind_RaiseException) (void *) __attribute__((weak));
+
+int main(void)
+{
+  if (&_Unwind_RaiseException != NULL)
+    abort ();;
+  return foo (x, y);
+}