All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] [x86] Wrap small helper functions from libgcc to avoid an ABI mismatch
@ 2011-11-08  9:27 Gabe Black
  2011-11-08 13:33 ` Mike Frysinger
  2011-11-08 22:34 ` [U-Boot] [PATCH v2] x86: " Gabe Black
  0 siblings, 2 replies; 36+ messages in thread
From: Gabe Black @ 2011-11-08  9:27 UTC (permalink / raw)
  To: u-boot

When gcc compiles some 64 bit operations on a 32 bit machine, it generates
calls to small functions instead of instructions which do the job directly.
Those functions are defined in libgcc and transparently provide whatever
functionality was necessary. Unfortunately, u-boot can be built with a
non-standard ABI when libgcc isn't. When the two are linked together, very
confusing bugs can crop up, for instance seemingly normal integer division
or modulus getting the wrong answer or even raising a spurious divide by
zero exception.

This change barrows (steals) a technique and some code from coreboot which
solves this problem by creating wrappers which translate the calling
convention when calling the functions in libgcc. Unfortunately that means that
these instructions which had already been turned into functions have even more
overhead, but more importantly it makes them work properly.

To find all of the functions that needed wrapping, u-boot was compiled without
linking in libgcc. All the symbols the linker complained were undefined were
presumed to be the symbols that are needed from libgcc. These were a subset of
the symbols covered by the coreboot code, so it was used unmodified.

Signed-off-by: Gabe Black <gabeblack@chromium.org>
---
 arch/x86/config.mk    |    3 +++
 arch/x86/lib/Makefile |    1 +
 arch/x86/lib/gcc.c    |   38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 0 deletions(-)
 create mode 100644 arch/x86/lib/gcc.c

diff --git a/arch/x86/config.mk b/arch/x86/config.mk
index fe9083f..c38ac70 100644
--- a/arch/x86/config.mk
+++ b/arch/x86/config.mk
@@ -41,3 +41,6 @@ PLATFORM_RELFLAGS += -ffunction-sections -fvisibility=hidden
 PLATFORM_LDFLAGS += --emit-relocs -Bsymbolic -Bsymbolic-functions
 
 LDFLAGS_FINAL += --gc-sections -pie
+LDFLAGS_FINAL += --wrap=__divdi3 --wrap=__udivdi3
+LDFLAGS_FINAL += --wrap=__moddi3 --wrap=__umoddi3
+LDSCRIPT := $(SRCTREE)/$(CPUDIR)/u-boot.lds
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index 71e94f7..210dbbe 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -32,6 +32,7 @@ SOBJS-y	+= realmode_switch.o
 COBJS-y	+= bios_setup.o
 COBJS-y	+= board.o
 COBJS-y	+= bootm.o
+COBJS-y	+= gcc.o
 COBJS-y	+= interrupts.o
 COBJS-$(CONFIG_SYS_PCAT_INTERRUPTS) += pcat_interrupts.o
 COBJS-$(CONFIG_SYS_GENERIC_TIMER) += pcat_timer.o
diff --git a/arch/x86/lib/gcc.c b/arch/x86/lib/gcc.c
new file mode 100644
index 0000000..b11ea5f
--- /dev/null
+++ b/arch/x86/lib/gcc.c
@@ -0,0 +1,38 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2009 coresystems GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 or later of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA, 02110-1301 USA
+ */
+
+#ifdef __GNUC__
+
+/*
+ * GCC's libgcc handling is quite broken. While the libgcc functions
+ * are always regparm(0) the code that calls them uses whatever the
+ * compiler call specifies. Therefore we need a wrapper around those
+ * functions. See gcc bug PR41055 for more information.
+ */
+#define WRAP_LIBGCC_CALL(type, name) \
+	type __real_##name(type a, type b) __attribute__((regparm(0))); \
+	type __wrap_##name(type a, type b); \
+	type __wrap_##name(type a, type b) { return __real_##name(a, b); }
+
+WRAP_LIBGCC_CALL(long long, __divdi3)
+WRAP_LIBGCC_CALL(unsigned long long, __udivdi3)
+WRAP_LIBGCC_CALL(long long, __moddi3)
+WRAP_LIBGCC_CALL(unsigned long long, __umoddi3)
+
+#endif
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [U-Boot] [PATCH] [x86] Wrap small helper functions from libgcc to avoid an ABI mismatch
  2011-11-08  9:27 [U-Boot] [PATCH] [x86] Wrap small helper functions from libgcc to avoid an ABI mismatch Gabe Black
@ 2011-11-08 13:33 ` Mike Frysinger
  2011-11-08 22:27   ` Gabe Black
  2011-11-08 22:34 ` [U-Boot] [PATCH v2] x86: " Gabe Black
  1 sibling, 1 reply; 36+ messages in thread
From: Mike Frysinger @ 2011-11-08 13:33 UTC (permalink / raw)
  To: u-boot

On Tuesday 08 November 2011 04:27:44 Gabe Black wrote:
> When gcc compiles some 64 bit operations on a 32 bit machine, it generates
> calls to small functions instead of instructions which do the job directly.
> Those functions are defined in libgcc and transparently provide whatever
> functionality was necessary. Unfortunately, u-boot can be built with a
> non-standard ABI when libgcc isn't. When the two are linked together, very
> confusing bugs can crop up, for instance seemingly normal integer division
> or modulus getting the wrong answer or even raising a spurious divide by
> zero exception.

might be good to explicitly mention that this is due to u-boot using -mregparm

> --- a/arch/x86/config.mk
> +++ b/arch/x86/config.mk
>
> +LDSCRIPT := $(SRCTREE)/$(CPUDIR)/u-boot.lds

looks like some old code sneaked in.  bad rebase ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111108/ce4175e8/attachment.pgp 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [PATCH] [x86] Wrap small helper functions from libgcc to avoid an ABI mismatch
  2011-11-08 13:33 ` Mike Frysinger
@ 2011-11-08 22:27   ` Gabe Black
  0 siblings, 0 replies; 36+ messages in thread
From: Gabe Black @ 2011-11-08 22:27 UTC (permalink / raw)
  To: u-boot

On Tue, Nov 8, 2011 at 5:33 AM, Mike Frysinger <vapier@gentoo.org> wrote:

> On Tuesday 08 November 2011 04:27:44 Gabe Black wrote:
> > When gcc compiles some 64 bit operations on a 32 bit machine, it
> generates
> > calls to small functions instead of instructions which do the job
> directly.
> > Those functions are defined in libgcc and transparently provide whatever
> > functionality was necessary. Unfortunately, u-boot can be built with a
> > non-standard ABI when libgcc isn't. When the two are linked together,
> very
> > confusing bugs can crop up, for instance seemingly normal integer
> division
> > or modulus getting the wrong answer or even raising a spurious divide by
> > zero exception.
>
> might be good to explicitly mention that this is due to u-boot using
> -mregparm
>
> > --- a/arch/x86/config.mk
> > +++ b/arch/x86/config.mk
> >
> > +LDSCRIPT := $(SRCTREE)/$(CPUDIR)/u-boot.lds
>
> looks like some old code sneaked in.  bad rebase ?
>

Yep, that's right. The original change doesn't have that line so it must
have slipped in during a rebase.

Gabe

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [PATCH v2] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch
  2011-11-08  9:27 [U-Boot] [PATCH] [x86] Wrap small helper functions from libgcc to avoid an ABI mismatch Gabe Black
  2011-11-08 13:33 ` Mike Frysinger
@ 2011-11-08 22:34 ` Gabe Black
  2011-11-08 23:14   ` Graeme Russ
                     ` (2 more replies)
  1 sibling, 3 replies; 36+ messages in thread
From: Gabe Black @ 2011-11-08 22:34 UTC (permalink / raw)
  To: u-boot

When gcc compiles some 64 bit operations on a 32 bit machine, it generates
calls to small functions instead of instructions which do the job directly.
Those functions are defined in libgcc and transparently provide whatever
functionality was necessary. Unfortunately, u-boot can be built with a
non-standard ABI when libgcc isn't. More specifically, u-boot uses
-mregparm. When the u-boot and libgcc are linked together, very confusing
bugs can crop up, for instance seemingly normal integer division or modulus
getting the wrong answer or even raising a spurious divide by zero
exception.

This change barrows (steals) a technique and some code from coreboot which
solves this problem by creating wrappers which translate the calling
convention when calling the functions in libgcc. Unfortunately that means that
these instructions which had already been turned into functions have even more
overhead, but more importantly it makes them work properly.

To find all of the functions that needed wrapping, u-boot was compiled without
linking in libgcc. All the symbols the linker complained were undefined were
presumed to be the symbols that are needed from libgcc. These were a subset of
the symbols covered by the coreboot code, so it was used unmodified.

Signed-off-by: Gabe Black <gabeblack@chromium.org>
---
Changes in v2:
- Change the [x86] tag to x86:
- Mention -mregparm in the commit message.
- Get rid of a stray line which snuck in during a rebase.

 arch/x86/config.mk    |    2 ++
 arch/x86/lib/Makefile |    1 +
 arch/x86/lib/gcc.c    |   38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 0 deletions(-)
 create mode 100644 arch/x86/lib/gcc.c

diff --git a/arch/x86/config.mk b/arch/x86/config.mk
index fe9083f..995fa1e 100644
--- a/arch/x86/config.mk
+++ b/arch/x86/config.mk
@@ -41,3 +41,5 @@ PLATFORM_RELFLAGS += -ffunction-sections -fvisibility=hidden
 PLATFORM_LDFLAGS += --emit-relocs -Bsymbolic -Bsymbolic-functions
 
 LDFLAGS_FINAL += --gc-sections -pie
+LDFLAGS_FINAL += --wrap=__divdi3 --wrap=__udivdi3
+LDFLAGS_FINAL += --wrap=__moddi3 --wrap=__umoddi3
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index 71e94f7..210dbbe 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -32,6 +32,7 @@ SOBJS-y	+= realmode_switch.o
 COBJS-y	+= bios_setup.o
 COBJS-y	+= board.o
 COBJS-y	+= bootm.o
+COBJS-y	+= gcc.o
 COBJS-y	+= interrupts.o
 COBJS-$(CONFIG_SYS_PCAT_INTERRUPTS) += pcat_interrupts.o
 COBJS-$(CONFIG_SYS_GENERIC_TIMER) += pcat_timer.o
diff --git a/arch/x86/lib/gcc.c b/arch/x86/lib/gcc.c
new file mode 100644
index 0000000..b11ea5f
--- /dev/null
+++ b/arch/x86/lib/gcc.c
@@ -0,0 +1,38 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2009 coresystems GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 or later of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA, 02110-1301 USA
+ */
+
+#ifdef __GNUC__
+
+/*
+ * GCC's libgcc handling is quite broken. While the libgcc functions
+ * are always regparm(0) the code that calls them uses whatever the
+ * compiler call specifies. Therefore we need a wrapper around those
+ * functions. See gcc bug PR41055 for more information.
+ */
+#define WRAP_LIBGCC_CALL(type, name) \
+	type __real_##name(type a, type b) __attribute__((regparm(0))); \
+	type __wrap_##name(type a, type b); \
+	type __wrap_##name(type a, type b) { return __real_##name(a, b); }
+
+WRAP_LIBGCC_CALL(long long, __divdi3)
+WRAP_LIBGCC_CALL(unsigned long long, __udivdi3)
+WRAP_LIBGCC_CALL(long long, __moddi3)
+WRAP_LIBGCC_CALL(unsigned long long, __umoddi3)
+
+#endif
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [U-Boot] [PATCH v2] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch
  2011-11-08 22:34 ` [U-Boot] [PATCH v2] x86: " Gabe Black
@ 2011-11-08 23:14   ` Graeme Russ
  2011-11-09  4:49     ` Mike Frysinger
  2011-11-09  3:05   ` [U-Boot] [PATCH v2] " Graeme Russ
  2011-11-09 10:32   ` [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage Graeme Russ
  2 siblings, 1 reply; 36+ messages in thread
From: Graeme Russ @ 2011-11-08 23:14 UTC (permalink / raw)
  To: u-boot

Hi Gabe,

On Wed, Nov 9, 2011 at 9:34 AM, Gabe Black <gabeblack@chromium.org> wrote:
> When gcc compiles some 64 bit operations on a 32 bit machine, it generates
> calls to small functions instead of instructions which do the job directly.
> Those functions are defined in libgcc and transparently provide whatever
> functionality was necessary. Unfortunately, u-boot can be built with a
> non-standard ABI when libgcc isn't. More specifically, u-boot uses
> -mregparm. When the u-boot and libgcc are linked together, very confusing
> bugs can crop up, for instance seemingly normal integer division or modulus
> getting the wrong answer or even raising a spurious divide by zero
> exception.

Hmmm, very interesting. I would think this would apply to _all_ libgcc
functions, but somehow the issue is being avoided. I haven't had a good
look at what libgcc functions are called by U-Boot - have you?

> This change barrows (steals) a technique and some code from coreboot which

s/barrows/borrows/

> solves this problem by creating wrappers which translate the calling
> convention when calling the functions in libgcc. Unfortunately that means that
> these instructions which had already been turned into functions have even more
> overhead, but more importantly it makes them work properly.
>
> To find all of the functions that needed wrapping, u-boot was compiled without
> linking in libgcc. All the symbols the linker complained were undefined were
> presumed to be the symbols that are needed from libgcc. These were a subset of
> the symbols covered by the coreboot code, so it was used unmodified.

Ah, yes you have - Can you give me a list? I think to be pragmatic we
should either wrap the all or go down the private libgcc path like ARM has
done. Private lib functions would elliminate the overhead, but is this
really such a problem anyway?

Regards,

Graeme

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [PATCH v2] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch
  2011-11-08 22:34 ` [U-Boot] [PATCH v2] x86: " Gabe Black
  2011-11-08 23:14   ` Graeme Russ
@ 2011-11-09  3:05   ` Graeme Russ
  2011-11-09 10:32   ` [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage Graeme Russ
  2 siblings, 0 replies; 36+ messages in thread
From: Graeme Russ @ 2011-11-09  3:05 UTC (permalink / raw)
  To: u-boot

Hi Gabe,

On Wed, Nov 9, 2011 at 9:34 AM, Gabe Black <gabeblack@chromium.org> wrote:
> When gcc compiles some 64 bit operations on a 32 bit machine, it generates
> calls to small functions instead of instructions which do the job directly.
> Those functions are defined in libgcc and transparently provide whatever
> functionality was necessary. Unfortunately, u-boot can be built with a
> non-standard ABI when libgcc isn't. More specifically, u-boot uses
> -mregparm. When the u-boot and libgcc are linked together, very confusing
> bugs can crop up, for instance seemingly normal integer division or modulus
> getting the wrong answer or even raising a spurious divide by zero
> exception.
>
> This change barrows (steals) a technique and some code from coreboot which
> solves this problem by creating wrappers which translate the calling
> convention when calling the functions in libgcc. Unfortunately that means that
> these instructions which had already been turned into functions have even more
> overhead, but more importantly it makes them work properly.
>
> To find all of the functions that needed wrapping, u-boot was compiled without
> linking in libgcc. All the symbols the linker complained were undefined were
> presumed to be the symbols that are needed from libgcc. These were a subset of
> the symbols covered by the coreboot code, so it was used unmodified.
>
> Signed-off-by: Gabe Black <gabeblack@chromium.org>
> ---
> Changes in v2:
> - Change the [x86] tag to x86:
> - Mention -mregparm in the commit message.
> - Get rid of a stray line which snuck in during a rebase.

As mentioned in a reply to 'Import the glibc memset implementa?tion', I
think I would prefer to either:

 - Investigate if regparm usage can be dropped so U-Boot is ABI compliant
 - If not, use USE_PRIVATE_LIBGCC and implement all required libgcc
   functions in U-Boot

Either way, I don't want to have the possibility that someone uses another
libgcc function, forgets to put a wrapper around it, and encounter 'weird'
bugs

Regards,

Graeme

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [PATCH v2] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch
  2011-11-09  4:49     ` Mike Frysinger
@ 2011-11-09  3:57       ` Graeme Russ
  2011-11-09  5:34         ` Mike Frysinger
  0 siblings, 1 reply; 36+ messages in thread
From: Graeme Russ @ 2011-11-09  3:57 UTC (permalink / raw)
  To: u-boot

On Wed, Nov 9, 2011 at 3:49 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tuesday 08 November 2011 18:14:46 Graeme Russ wrote:
>> Ah, yes you have - Can you give me a list? I think to be pragmatic we
>> should either wrap the all or go down the private libgcc path like ARM has
>> done. Private lib functions would elliminate the overhead, but is this
>> really such a problem anyway?
>
> it then becomes a sync issue ... updates to gcc's libgcc aren't reflected in u-
> boot automatically ...

Are those updates needed? We already have a fair chunk of libc which
is not automatically sync'd and going by what is in arch/arm/lib,
there are very few libgcc functions (far less than libc) and each of
those are relatively trivial and unlikely to require updating.

Also, I already know of issues compiling U-Boot on an x64 OS because
of the 32/64 bit incompatibility of libgcc. I never encountered this
because I only have a 32-bit build machine

So the handful of libgcc functions are starting to become a
disproportionate headache

Regards,

Graeme

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [PATCH v2] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch
  2011-11-08 23:14   ` Graeme Russ
@ 2011-11-09  4:49     ` Mike Frysinger
  2011-11-09  3:57       ` Graeme Russ
  0 siblings, 1 reply; 36+ messages in thread
From: Mike Frysinger @ 2011-11-09  4:49 UTC (permalink / raw)
  To: u-boot

On Tuesday 08 November 2011 18:14:46 Graeme Russ wrote:
> Ah, yes you have - Can you give me a list? I think to be pragmatic we
> should either wrap the all or go down the private libgcc path like ARM has
> done. Private lib functions would elliminate the overhead, but is this
> really such a problem anyway?

it then becomes a sync issue ... updates to gcc's libgcc aren't reflected in u-
boot automatically ...
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111108/8392dba8/attachment.pgp 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [PATCH v2] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch
  2011-11-09  3:57       ` Graeme Russ
@ 2011-11-09  5:34         ` Mike Frysinger
  2011-11-17  9:01           ` [U-Boot] [PATCH v3] " Gabe Black
  0 siblings, 1 reply; 36+ messages in thread
From: Mike Frysinger @ 2011-11-09  5:34 UTC (permalink / raw)
  To: u-boot

On Tuesday 08 November 2011 22:57:31 Graeme Russ wrote:
> On Wed, Nov 9, 2011 at 3:49 PM, Mike Frysinger wrote:
> > On Tuesday 08 November 2011 18:14:46 Graeme Russ wrote:
> >> Ah, yes you have - Can you give me a list? I think to be pragmatic we
> >> should either wrap the all or go down the private libgcc path like ARM
> >> has done. Private lib functions would elliminate the overhead, but is
> >> this really such a problem anyway?
> > 
> > it then becomes a sync issue ... updates to gcc's libgcc aren't reflected
> > in u- boot automatically ...
> 
> Are those updates needed?

you mean bug/math fixes and optimizations ?  i would think so.

> We already have a fair chunk of libc which is not automatically sync'd

we have very very little of glibc ... really only a few optimized 
string/memory funcs, and those aren't glibc specific.  the only other C lib 
type stuff is taken from Linux, or zlib, or dlmalloc, or other locations that 
aren't possible to link against.  libgcc is a bit unique in this regard.

> and going by what is in arch/arm/lib,
> there are very few libgcc functions (far less than libc) and each of
> those are relatively trivial and unlikely to require updating.

it depends on the architecture.  if the core ISA doesn't change, then the math 
funcs won't change much.  i'm not terribly familiar with the gcc x86 internals 
to say how much we actually rely on libgcc.a considering most of the fun stuff 
is real hardware insns.  unlike the normal embedded risc arches which need to 
implement more complicated funcs with many insns.

> Also, I already know of issues compiling U-Boot on an x64 OS because
> of the 32/64 bit incompatibility of libgcc. I never encountered this
> because I only have a 32-bit build machine

yes, this would be an issue, although most people on x86-64 have multilib 
toolchains, so it'd work anyways.  maybe the x86 config.mk should be 
automatically adding -m32 when available.

> So the handful of libgcc functions are starting to become a
> disproportionate headache

seems fairly low to me, but i'm not the x86 maintainer.  i'm not seeing these 
funcs in Linux' arch/x86/ tree though ... maybe there's a better solution ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111109/21da441d/attachment.pgp 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-08 22:34 ` [U-Boot] [PATCH v2] x86: " Gabe Black
  2011-11-08 23:14   ` Graeme Russ
  2011-11-09  3:05   ` [U-Boot] [PATCH v2] " Graeme Russ
@ 2011-11-09 10:32   ` Graeme Russ
  2011-11-09 17:12     ` Mike Frysinger
  2011-11-16 23:00     ` Graeme Russ
  2 siblings, 2 replies; 36+ messages in thread
From: Graeme Russ @ 2011-11-09 10:32 UTC (permalink / raw)
  To: u-boot

Hi Gabe,

Can you please try this patch - If it solves your libgcc problem, I will
add it to the misc cleanup patch

Thanks,

Graeme
---
 arch/x86/config.mk        |    3 ---
 arch/x86/cpu/interrupts.c |    2 +-
 arch/x86/cpu/start.S      |    5 ++---
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/x86/config.mk b/arch/x86/config.mk
index fe9083f..ec5f707 100644
--- a/arch/x86/config.mk
+++ b/arch/x86/config.mk
@@ -23,10 +23,7 @@

 CONFIG_STANDALONE_LOAD_ADDR ?= 0x40000

-PLATFORM_CPPFLAGS += -fno-strict-aliasing
 PLATFORM_CPPFLAGS += -Wstrict-prototypes
-PLATFORM_CPPFLAGS += -mregparm=3
-PLATFORM_CPPFLAGS += -fomit-frame-pointer
 PF_CPPFLAGS_X86   := $(call cc-option, -ffreestanding) \
 		     $(call cc-option, -fno-toplevel-reorder, \
 		       $(call cc-option, -fno-unit-at-a-time)) \
diff --git a/arch/x86/cpu/interrupts.c b/arch/x86/cpu/interrupts.c
index e0958eb..a15d70a 100644
--- a/arch/x86/cpu/interrupts.c
+++ b/arch/x86/cpu/interrupts.c
@@ -249,7 +249,7 @@ int disable_interrupts(void)
 }

 /* IRQ Low-Level Service Routine */
-void irq_llsr(struct irq_regs *regs)
+void __attribute__ ((regparm(1))) irq_llsr(struct irq_regs *regs)
 {
 	/*
 	 * For detailed description of each exception, refer to:
diff --git a/arch/x86/cpu/start.S b/arch/x86/cpu/start.S
index f87633b..119ca2d 100644
--- a/arch/x86/cpu/start.S
+++ b/arch/x86/cpu/start.S
@@ -84,9 +84,8 @@ car_init_ret:
 	 */
 	movl	$CONFIG_SYS_INIT_SP_ADDR, %esp

-	/* Set parameter to board_init_f() to boot flags */
-	xorl	%eax, %eax
-	movw	%bx, %ax
+	/* Set parameter to board_init_f() - Unused dummy value */
+	pushl	$0

 	/* Enter, U-boot! */
 	call	board_init_f
--
1.7.5.2.317.g391b14

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-09 10:32   ` [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage Graeme Russ
@ 2011-11-09 17:12     ` Mike Frysinger
  2011-11-09 21:42       ` Graeme Russ
  2011-11-16 23:00     ` Graeme Russ
  1 sibling, 1 reply; 36+ messages in thread
From: Mike Frysinger @ 2011-11-09 17:12 UTC (permalink / raw)
  To: u-boot

On Wednesday 09 November 2011 05:32:59 Graeme Russ wrote:
> --- a/arch/x86/config.mk
> +++ b/arch/x86/config.mk
> 
> -PLATFORM_CPPFLAGS += -mregparm=3
> -PLATFORM_CPPFLAGS += -fomit-frame-pointer

this sounds like you're throwing the baby out with the bath water.  doesn't 
this severely affect the generated code ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111109/e693844b/attachment.pgp 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-09 17:12     ` Mike Frysinger
@ 2011-11-09 21:42       ` Graeme Russ
  2011-11-10  4:13         ` Mike Frysinger
  0 siblings, 1 reply; 36+ messages in thread
From: Graeme Russ @ 2011-11-09 21:42 UTC (permalink / raw)
  To: u-boot

Hi Mike,

On Thu, Nov 10, 2011 at 4:12 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Wednesday 09 November 2011 05:32:59 Graeme Russ wrote:
>> --- a/arch/x86/config.mk
>> +++ b/arch/x86/config.mk
>>
>> -PLATFORM_CPPFLAGS += -mregparm=3
>> -PLATFORM_CPPFLAGS += -fomit-frame-pointer
>
> this sounds like you're throwing the baby out with the bath water. ?doesn't
> this severely affect the generated code ?

No - omit-frame-pointer is enabled by -O2 and also:

http://gcc.gnu.org/gcc-4.6/changes.html

"The default setting (when not optimizing for size) for 32-bit
GNU/Linux and Darwin x86 targets has been changed to
-fomit-frame-pointer. The default can be reverted to
-fno-omit-frame-pointer by configuring GCC with the
--enable-frame-pointer configure option."

So the flag is a do-nothing

Regards,

Graeme

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-09 21:42       ` Graeme Russ
@ 2011-11-10  4:13         ` Mike Frysinger
  2011-11-10  4:22           ` Graeme Russ
  0 siblings, 1 reply; 36+ messages in thread
From: Mike Frysinger @ 2011-11-10  4:13 UTC (permalink / raw)
  To: u-boot

On Wednesday 09 November 2011 16:42:51 Graeme Russ wrote:
> On Thu, Nov 10, 2011 at 4:12 AM, Mike Frysinger wrote:
> > On Wednesday 09 November 2011 05:32:59 Graeme Russ wrote:
> >> --- a/arch/x86/config.mk
> >> +++ b/arch/x86/config.mk
> >> 
> >> -PLATFORM_CPPFLAGS += -mregparm=3
> >> -PLATFORM_CPPFLAGS += -fomit-frame-pointer
> > 
> > this sounds like you're throwing the baby out with the bath water.
> >  doesn't this severely affect the generated code ?
> 
> No - omit-frame-pointer is enabled by -O2 and also:
> 
> http://gcc.gnu.org/gcc-4.6/changes.html
> 
> "The default setting (when not optimizing for size) for 32-bit
> GNU/Linux and Darwin x86 targets has been changed to
> -fomit-frame-pointer. The default can be reverted to
> -fno-omit-frame-pointer by configuring GCC with the
> --enable-frame-pointer configure option."
> 
> So the flag is a do-nothing

ok, except:
 - we build u-boot with -Os and not -O2
 - i'd say most people aren't using gcc-4.6

i was referring also to throwing away -mregparm=3 ...
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111109/2aae86ce/attachment.pgp 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-10  4:13         ` Mike Frysinger
@ 2011-11-10  4:22           ` Graeme Russ
  2011-11-10  5:10             ` Graeme Russ
  2011-11-10 17:15             ` Mike Frysinger
  0 siblings, 2 replies; 36+ messages in thread
From: Graeme Russ @ 2011-11-10  4:22 UTC (permalink / raw)
  To: u-boot

Hi Mike,

On Thu, Nov 10, 2011 at 3:13 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Wednesday 09 November 2011 16:42:51 Graeme Russ wrote:
>> On Thu, Nov 10, 2011 at 4:12 AM, Mike Frysinger wrote:
>> > On Wednesday 09 November 2011 05:32:59 Graeme Russ wrote:
>> >> --- a/arch/x86/config.mk
>> >> +++ b/arch/x86/config.mk
>> >>
>> >> -PLATFORM_CPPFLAGS += -mregparm=3
>> >> -PLATFORM_CPPFLAGS += -fomit-frame-pointer
>> >
>> > this sounds like you're throwing the baby out with the bath water.
>> > ?doesn't this severely affect the generated code ?
>>
>> No - omit-frame-pointer is enabled by -O2 and also:
>>
>> http://gcc.gnu.org/gcc-4.6/changes.html
>>
>> "The default setting (when not optimizing for size) for 32-bit
>> GNU/Linux and Darwin x86 targets has been changed to
>> -fomit-frame-pointer. The default can be reverted to
>> -fno-omit-frame-pointer by configuring GCC with the
>> --enable-frame-pointer configure option."
>>
>> So the flag is a do-nothing
>
> ok, except:
> ?- we build u-boot with -Os and not -O2
> ?- i'd say most people aren't using gcc-4.6

Ah, OK then I'll leave it in - It was only a cruft reduction plan anyway :)

> i was referring also to throwing away -mregparm=3 ...

Yes, it does effect the code - It makes it ABI compliant like everyone
else (except ARM) :) I expect a code size increase (have not measured
it yet)

As I've stated, I really do not want arbitrary wrapper functions where
it is not obvious that they need to be updated if new code uses
previously unused (and unwrapped) libgcc functions (in particular if
there are new libgcc functions in the future which we can't wrap
todday anyway)

Option a) is to remove regparm=3
Option b) is to use private libgcc
Option c) is to use wrappers

If this patch works, I'll look at the code impact and we can discuss
which option we take :)

Regards,

Graeme

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-10  4:22           ` Graeme Russ
@ 2011-11-10  5:10             ` Graeme Russ
  2011-11-10 17:15             ` Mike Frysinger
  1 sibling, 0 replies; 36+ messages in thread
From: Graeme Russ @ 2011-11-10  5:10 UTC (permalink / raw)
  To: u-boot

Hi Mike,

On Thu, Nov 10, 2011 at 3:22 PM, Graeme Russ <graeme.russ@gmail.com> wrote:
> Hi Mike,
>
> On Thu, Nov 10, 2011 at 3:13 PM, Mike Frysinger <vapier@gentoo.org> wrote:
>> On Wednesday 09 November 2011 16:42:51 Graeme Russ wrote:
>>> On Thu, Nov 10, 2011 at 4:12 AM, Mike Frysinger wrote:
>>> > On Wednesday 09 November 2011 05:32:59 Graeme Russ wrote:
>>> >> --- a/arch/x86/config.mk
>>> >> +++ b/arch/x86/config.mk
>>> >>
>>> >> -PLATFORM_CPPFLAGS += -mregparm=3
>>> >> -PLATFORM_CPPFLAGS += -fomit-frame-pointer
>>> >
>>> > this sounds like you're throwing the baby out with the bath water.
>>> > ?doesn't this severely affect the generated code ?
>>>
>>> No - omit-frame-pointer is enabled by -O2 and also:
>>>
>>> http://gcc.gnu.org/gcc-4.6/changes.html
>>>
>>> "The default setting (when not optimizing for size) for 32-bit
>>> GNU/Linux and Darwin x86 targets has been changed to
>>> -fomit-frame-pointer. The default can be reverted to
>>> -fno-omit-frame-pointer by configuring GCC with the
>>> --enable-frame-pointer configure option."
>>>
>>> So the flag is a do-nothing
>>
>> ok, except:
>> ?- we build u-boot with -Os and not -O2
>> ?- i'd say most people aren't using gcc-4.6

http://gcc.gnu.org/onlinedocs/gcc-4.6.2/gcc/Optimize-Options.html#Optimize-Options

-fomit-frame-pointer
...
Enabled at levels -O, -O2, -O3, -Os

Thats goes back to at least gcc 4.3.x

But I'll keep it in anyway, just in case someone wants to remove -Os
(for testing/debugging etc)

Regards,

Graeme

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-10  4:22           ` Graeme Russ
  2011-11-10  5:10             ` Graeme Russ
@ 2011-11-10 17:15             ` Mike Frysinger
  2011-11-10 22:53               ` Graeme Russ
  1 sibling, 1 reply; 36+ messages in thread
From: Mike Frysinger @ 2011-11-10 17:15 UTC (permalink / raw)
  To: u-boot

On Wednesday 09 November 2011 23:22:34 Graeme Russ wrote:
> On Thu, Nov 10, 2011 at 3:13 PM, Mike Frysinger wrote:
> > i was referring also to throwing away -mregparm=3 ...
> 
> Yes, it does effect the code - It makes it ABI compliant like everyone
> else (except ARM) :) I expect a code size increase (have not measured
> it yet)

ABI compliance only matters at the boundaries.  since u-boot is largely self-
contained, we shouldn't be afraid to break internal ABI.

> As I've stated, I really do not want arbitrary wrapper functions where
> it is not obvious that they need to be updated if new code uses
> previously unused (and unwrapped) libgcc functions (in particular if
> there are new libgcc functions in the future which we can't wrap
> todday anyway)
> 
> Option a) is to remove regparm=3
> Option b) is to use private libgcc
> Option c) is to use wrappers
> 
> If this patch works, I'll look at the code impact and we can discuss
> which option we take :)

for the record, i'm not against a private libgcc.  it just seems to me that 
the wrapper approach proposed by Gabe has the best pro/con ratio.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111110/9683c94d/attachment.pgp 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-10 17:15             ` Mike Frysinger
@ 2011-11-10 22:53               ` Graeme Russ
  2011-11-11  0:23                 ` Mike Frysinger
  0 siblings, 1 reply; 36+ messages in thread
From: Graeme Russ @ 2011-11-10 22:53 UTC (permalink / raw)
  To: u-boot

Hi Mike,

On Fri, Nov 11, 2011 at 4:15 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Wednesday 09 November 2011 23:22:34 Graeme Russ wrote:
>> On Thu, Nov 10, 2011 at 3:13 PM, Mike Frysinger wrote:
>> > i was referring also to throwing away -mregparm=3 ...
>>
>> Yes, it does effect the code - It makes it ABI compliant like everyone
>> else (except ARM) :) I expect a code size increase (have not measured
>> it yet)
>
> ABI compliance only matters at the boundaries.  since u-boot is largely self-
> contained, we shouldn't be afraid to break internal ABI.

And I'm not afraid to do so

>> As I've stated, I really do not want arbitrary wrapper functions where
>> it is not obvious that they need to be updated if new code uses
>> previously unused (and unwrapped) libgcc functions (in particular if
>> there are new libgcc functions in the future which we can't wrap
>> todday anyway)
>>
>> Option a) is to remove regparm=3
>> Option b) is to use private libgcc
>> Option c) is to use wrappers
>>
>> If this patch works, I'll look at the code impact and we can discuss
>> which option we take :)
>
> for the record, i'm not against a private libgcc.  it just seems to me that
> the wrapper approach proposed by Gabe has the best pro/con ratio.

I'm sorry, but I must respectfully disagree...

The biggest con with wrappers is that the proposed patch only wraps four
functions. arch/arm/lib/ has private libgcc implementations for eight
libgcc functions - I can only assume they are used somewhere. The kicker
is that if anyone uses a libgcc function which is not one of the four
already wrapped, and they do not realise this and fail to wrap them
themselves, no warning will be given by the compiler or linker. Now that
unwrapped function may be in a rarely executed code path (as evidenced by
the fact that this bug has been dormant for a year now). So you could have
in-the-wild version of U-Boot which start exhibiting strange behaviour and
nobody knows why

Another big(ish) con, for me, is that we already have a mechanism in place
to resolve this (USE_PRIVATE_LIBGCC) - I don't see any benefit to add a
second to the mix

The final (trivially small) con is the overhead added to these calls

Now if we use USE_PRIVATE_LIBGCC, unimplemented libgcc functions will
result in link errors, so using an unimplemented libgcc will be obvious at
build time - It may lead to a posting on the mailing list, but at least we
won't have latent libgcc related bugs in-the-wild.

Also, We use an existing mechanism and it is in keeping with --no-builtin.
libgcc is really just a library of functions that are too large to
implement as inline functions internally by gcc anyway:

"GCC provides a low-level runtime library, libgcc.a or libgcc_s.so.1 on
 some platforms. GCC generates calls to routines in this library
 automatically, whenever it needs to perform some operation that is too
 complicated to emit inline code for."

Now the downside has been raised regarding keeping the private libgcc
functions in-sync with mainline libgcc - We are only talking about a
handful of functions. There are no floating point or 'special' functions,
so the list is wholly restricted to:

http://gcc.gnu.org/onlinedocs/gccint/Integer-library-routines.html

I haven't looked at them, but I doubt they are very big (going by the ARM
implementations) and I doubt they change very often (probably not in years)

The more I think about this, the more I feel to just use USE_PRIVATE_LIBGCC

Regards,

Graeme

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-10 22:53               ` Graeme Russ
@ 2011-11-11  0:23                 ` Mike Frysinger
  2011-11-11  1:23                   ` Graeme Russ
  0 siblings, 1 reply; 36+ messages in thread
From: Mike Frysinger @ 2011-11-11  0:23 UTC (permalink / raw)
  To: u-boot

On Thursday 10 November 2011 17:53:06 Graeme Russ wrote:
> The biggest con with wrappers is that the proposed patch only wraps four
> functions. arch/arm/lib/ has private libgcc implementations for eight
> libgcc functions - I can only assume they are used somewhere.

i don't think you can look across arches like that.  arm provides a lot more 
libgcc funcs because it, like most RISC/embedded cpus, do not provide 
dedicated math insns in the ISA.  or the number of insns is so large, that 
creating a dedicated library func and emitting a function call makes more 
sense than emitting them inline.  x86 is a fairly "fat" ISA in that most math 
operations can be accomplished in single or a few insns, and is certainly 
better than emitting func calls to an external library.

in fact, building the current eNET board (the only x86 board) shows that it 
doesn't use *any* calls from libgcc:
	make PLATFORM_LIBGCC= eNET -j4

> The kicker
> is that if anyone uses a libgcc function which is not one of the four
> already wrapped, and they do not realise this and fail to wrap them
> themselves, no warning will be given by the compiler or linker. Now that
> unwrapped function may be in a rarely executed code path (as evidenced by
> the fact that this bug has been dormant for a year now). So you could have
> in-the-wild version of U-Boot which start exhibiting strange behaviour and
> nobody knows why

yes, but the better question is whether those funcs should be called in the 
first place

> The final (trivially small) con is the overhead added to these calls

this con is insignificant when weighed against the alternatives: not using 
regparm anywhere.  further, these funcs are rarely used, so you're talking 
about adding a minor amount of overhead to one or two call sites.

> Now if we use USE_PRIVATE_LIBGCC, unimplemented libgcc functions will
> result in link errors, so using an unimplemented libgcc will be obvious at
> build time - It may lead to a posting on the mailing list, but at least we
> won't have latent libgcc related bugs in-the-wild.

perhaps x86 should be setting PLATFORM_LIBGCC to nothing all the time.  the 
funcs Gabe wants to wrap are 64bit operations.  u-boot should not be doing 64-
bit operations.  that's why we have do_div().
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111110/1ed27e65/attachment.pgp 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-11  0:23                 ` Mike Frysinger
@ 2011-11-11  1:23                   ` Graeme Russ
  2011-11-11  1:40                     ` Mike Frysinger
  0 siblings, 1 reply; 36+ messages in thread
From: Graeme Russ @ 2011-11-11  1:23 UTC (permalink / raw)
  To: u-boot

Hi Mike,

On Fri, Nov 11, 2011 at 11:23 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Thursday 10 November 2011 17:53:06 Graeme Russ wrote:
>> The biggest con with wrappers is that the proposed patch only wraps four
>> functions. arch/arm/lib/ has private libgcc implementations for eight
>> libgcc functions - I can only assume they are used somewhere.
>
> i don't think you can look across arches like that.  arm provides a lot more
> libgcc funcs because it, like most RISC/embedded cpus, do not provide
> dedicated math insns in the ISA.  or the number of insns is so large, that
> creating a dedicated library func and emitting a function call makes more
> sense than emitting them inline.  x86 is a fairly "fat" ISA in that most math
> operations can be accomplished in single or a few insns, and is certainly
> better than emitting func calls to an external library.
>
> in fact, building the current eNET board (the only x86 board) shows that it
> doesn't use *any* calls from libgcc:
>        make PLATFORM_LIBGCC= eNET -j4

Which supports my point - The issue was latent because there were no call
sites

>
>> The kicker
>> is that if anyone uses a libgcc function which is not one of the four
>> already wrapped, and they do not realise this and fail to wrap them
>> themselves, no warning will be given by the compiler or linker. Now that
>> unwrapped function may be in a rarely executed code path (as evidenced by
>> the fact that this bug has been dormant for a year now). So you could have
>> in-the-wild version of U-Boot which start exhibiting strange behaviour and
>> nobody knows why
>
> yes, but the better question is whether those funcs should be called in the
> first place

But we can't control that - especially if the code is not submitted to
mainline. Then get people hitting the ML asking for help with non mainline
code because they used a function they did not know they should not have
used and they are seeing some random weird crash

>> The final (trivially small) con is the overhead added to these calls
>
> this con is insignificant when weighed against the alternatives: not using
> regparm anywhere.  further, these funcs are rarely used, so you're talking
> about adding a minor amount of overhead to one or two call sites.

100% agree

>> Now if we use USE_PRIVATE_LIBGCC, unimplemented libgcc functions will
>> result in link errors, so using an unimplemented libgcc will be obvious at
>> build time - It may lead to a posting on the mailing list, but at least we
>> won't have latent libgcc related bugs in-the-wild.
>
> perhaps x86 should be setting PLATFORM_LIBGCC to nothing all the time.  the
> funcs Gabe wants to wrap are 64bit operations.  u-boot should not be doing 64-
> bit operations.  that's why we have do_div().

Remember that there was a lot of discussion regarding the timer API that
pointed towards using 64-bit counters for all arches. We cannot take it
as gospel that 64-bit operations will never be used in U-Boot. Some people
may have a need for this as part of hardware init.

Regards,

Graeme

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-11  1:23                   ` Graeme Russ
@ 2011-11-11  1:40                     ` Mike Frysinger
  2011-11-11  1:51                       ` Graeme Russ
  0 siblings, 1 reply; 36+ messages in thread
From: Mike Frysinger @ 2011-11-11  1:40 UTC (permalink / raw)
  To: u-boot

On Thursday 10 November 2011 20:23:49 Graeme Russ wrote:
> On Fri, Nov 11, 2011 at 11:23 AM, Mike Frysinger wrote:
> > On Thursday 10 November 2011 17:53:06 Graeme Russ wrote:
> >> Now if we use USE_PRIVATE_LIBGCC, unimplemented libgcc functions will
> >> result in link errors, so using an unimplemented libgcc will be obvious
> >> at build time - It may lead to a posting on the mailing list, but at
> >> least we won't have latent libgcc related bugs in-the-wild.
> > 
> > perhaps x86 should be setting PLATFORM_LIBGCC to nothing all the time. 
> > the funcs Gabe wants to wrap are 64bit operations.  u-boot should not be
> > doing 64- bit operations.  that's why we have do_div().
> 
> Remember that there was a lot of discussion regarding the timer API that
> pointed towards using 64-bit counters for all arches. We cannot take it
> as gospel that 64-bit operations will never be used in U-Boot. Some people
> may have a need for this as part of hardware init.

Linux has no problem doing 64bit timers without 64bit mul/div.  i don't see 
how u-boot could possibly be more special than Linux.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111110/4ac00f1d/attachment.pgp 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-11  1:40                     ` Mike Frysinger
@ 2011-11-11  1:51                       ` Graeme Russ
  2011-11-11  1:55                         ` Mike Frysinger
  0 siblings, 1 reply; 36+ messages in thread
From: Graeme Russ @ 2011-11-11  1:51 UTC (permalink / raw)
  To: u-boot

Hi Mike,

On Fri, Nov 11, 2011 at 12:40 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Thursday 10 November 2011 20:23:49 Graeme Russ wrote:
>> On Fri, Nov 11, 2011 at 11:23 AM, Mike Frysinger wrote:
>> > On Thursday 10 November 2011 17:53:06 Graeme Russ wrote:
>> >> Now if we use USE_PRIVATE_LIBGCC, unimplemented libgcc functions will
>> >> result in link errors, so using an unimplemented libgcc will be obvious
>> >> at build time - It may lead to a posting on the mailing list, but at
>> >> least we won't have latent libgcc related bugs in-the-wild.
>> >
>> > perhaps x86 should be setting PLATFORM_LIBGCC to nothing all the time.
>> > the funcs Gabe wants to wrap are 64bit operations.  u-boot should not be
>> > doing 64- bit operations.  that's why we have do_div().
>>
>> Remember that there was a lot of discussion regarding the timer API that
>> pointed towards using 64-bit counters for all arches. We cannot take it
>> as gospel that 64-bit operations will never be used in U-Boot. Some people
>> may have a need for this as part of hardware init.
>
> Linux has no problem doing 64bit timers without 64bit mul/div.  i don't see
> how u-boot could possibly be more special than Linux.

A few questions (I am unfamiliar with the Linux build environment):

 a) Does Linux link to libgcc
 b) Does Linux use regparm
 c) If a & b are both yes, does Linux use wrappers for libgcc functions

Regards,

Graeme

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-11  1:51                       ` Graeme Russ
@ 2011-11-11  1:55                         ` Mike Frysinger
  2011-11-11  1:59                           ` Graeme Russ
  2011-11-11 19:59                           ` Scott Wood
  0 siblings, 2 replies; 36+ messages in thread
From: Mike Frysinger @ 2011-11-11  1:55 UTC (permalink / raw)
  To: u-boot

On Thursday 10 November 2011 20:51:47 Graeme Russ wrote:
> A few questions (I am unfamiliar with the Linux build environment):
> 
>  a) Does Linux link to libgcc

no Linux port uses libgcc.  they've always done the equivalent of 
PRIVATE_LIBGCC.  but in the case of x86, i can't see them providing any libgcc 
funcs.  so i don't think u-boot should either.

>  b) Does Linux use regparm

yes, it uses -mregparm=3
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111110/e373047f/attachment.pgp 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-11  1:55                         ` Mike Frysinger
@ 2011-11-11  1:59                           ` Graeme Russ
  2011-11-11  2:10                             ` Gabe Black
  2011-11-11  2:44                             ` Mike Frysinger
  2011-11-11 19:59                           ` Scott Wood
  1 sibling, 2 replies; 36+ messages in thread
From: Graeme Russ @ 2011-11-11  1:59 UTC (permalink / raw)
  To: u-boot

Hi Mike,

On Fri, Nov 11, 2011 at 12:55 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Thursday 10 November 2011 20:51:47 Graeme Russ wrote:
>> A few questions (I am unfamiliar with the Linux build environment):
>>
>>  a) Does Linux link to libgcc
>
> no Linux port uses libgcc.  they've always done the equivalent of
> PRIVATE_LIBGCC.  but in the case of x86, i can't see them providing any libgcc
> funcs.  so i don't think u-boot should either.
>
>>  b) Does Linux use regparm
>
> yes, it uses -mregparm=3

Well I think we have an answer - use PRIVATE_LIBGCC but do not implement
any libgcc functions and treat link errors as coding errors. If for some
bizarre reason we need to really, truly, honestly use a 64-bit libgcc
function, we'll port it over then

Regards,

Graeme

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-11  1:59                           ` Graeme Russ
@ 2011-11-11  2:10                             ` Gabe Black
  2011-11-11  2:22                               ` Graeme Russ
  2011-11-11  2:44                             ` Mike Frysinger
  1 sibling, 1 reply; 36+ messages in thread
From: Gabe Black @ 2011-11-11  2:10 UTC (permalink / raw)
  To: u-boot

On Thu, Nov 10, 2011 at 5:59 PM, Graeme Russ <graeme.russ@gmail.com> wrote:

> Hi Mike,
>
> On Fri, Nov 11, 2011 at 12:55 PM, Mike Frysinger <vapier@gentoo.org>
> wrote:
> > On Thursday 10 November 2011 20:51:47 Graeme Russ wrote:
> >> A few questions (I am unfamiliar with the Linux build environment):
> >>
> >>  a) Does Linux link to libgcc
> >
> > no Linux port uses libgcc.  they've always done the equivalent of
> > PRIVATE_LIBGCC.  but in the case of x86, i can't see them providing any
> libgcc
> > funcs.  so i don't think u-boot should either.
> >
> >>  b) Does Linux use regparm
> >
> > yes, it uses -mregparm=3
>
> Well I think we have an answer - use PRIVATE_LIBGCC but do not implement
> any libgcc functions and treat link errors as coding errors. If for some
> bizarre reason we need to really, truly, honestly use a 64-bit libgcc
> function, we'll port it over then
>
> Regards,
>
> Graeme
>

I haven't checked in on this thread in a little while, but I wanted to
point out some things. First, I was originally planning to measure the
performance difference with regparm turned off. I realized that would be
quite annoying to actually do, though, since we have a number of extra
libraries linked into u-boot and they would all have to be recompiled with
different options. Then the things they link with would have to be
recompiled, etc. Even if upstream U-Boot drops regparm, we may need to keep
it just for that reason.

Second, I think I have a solution that preserves regparm, keeps libgcc and
gcc in sync, and also stops unwrapped functions slipping into u-boot. We
can use this command:

objcopy /path/to/your/libgcc/libgcc.a
/somewhere/in/the/u-boot/build/libgcc.a --prefix-symbols=__real

to create a libgcc that has all of its symbols prefixed with the string
__real. Then *all* symbols are prepped for wrapping, regardless of if we
use them now or even know about them. Only the symbols we've explicitly
wrapped will be available. Note that I haven't actually implemented this
yet, but it seems to me like it captures the positive aspects of all the
alternatives.

Gabe

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-11  2:10                             ` Gabe Black
@ 2011-11-11  2:22                               ` Graeme Russ
  2011-11-11  2:41                                 ` Gabe Black
  0 siblings, 1 reply; 36+ messages in thread
From: Graeme Russ @ 2011-11-11  2:22 UTC (permalink / raw)
  To: u-boot

Hi Gabe,

On Fri, Nov 11, 2011 at 1:10 PM, Gabe Black <gabeblack@google.com> wrote:
> On Thu, Nov 10, 2011 at 5:59 PM, Graeme Russ <graeme.russ@gmail.com> wrote:
>>
>> Hi Mike,
>>
>> On Fri, Nov 11, 2011 at 12:55 PM, Mike Frysinger <vapier@gentoo.org>
>> wrote:
>> > On Thursday 10 November 2011 20:51:47 Graeme Russ wrote:
>> >> A few questions (I am unfamiliar with the Linux build environment):
>> >>
>> >>  a) Does Linux link to libgcc
>> >
>> > no Linux port uses libgcc.  they've always done the equivalent of
>> > PRIVATE_LIBGCC.  but in the case of x86, i can't see them providing any
>> > libgcc
>> > funcs.  so i don't think u-boot should either.
>> >
>> >>  b) Does Linux use regparm
>> >
>> > yes, it uses -mregparm=3
>>
>> Well I think we have an answer - use PRIVATE_LIBGCC but do not implement
>> any libgcc functions and treat link errors as coding errors. If for some
>> bizarre reason we need to really, truly, honestly use a 64-bit libgcc
>> function, we'll port it over then
>>
>> Regards,
>>
>> Graeme
>
> I haven't checked in on this thread in a little while, but I wanted to point
> out some things. First, I was originally planning to measure the performance
> difference with regparm turned off. I realized that would be quite annoying
> to actually do, though, since we have a number of extra libraries linked
> into u-boot and they would all have to be recompiled with different options.

I assume they are all GPL :)

> Then the things they link with would have to be recompiled, etc. Even if
> upstream U-Boot drops regparm, we may need to keep it just for that reason.

Hmm, that raises an interresing point - by using a non-standard ABI we can
run afoul of external libraries. Which raises the question - what external
libraries do you need to link to?

> Second, I think I have a solution that preserves regparm, keeps libgcc and
> gcc in sync, and also stops unwrapped functions slipping into u-boot. We can
> use this command:
> objcopy /path/to/your/libgcc/libgcc.a
> /somewhere/in/the/u-boot/build/libgcc.a --prefix-symbols=__real
> to create a libgcc that has all of its symbols prefixed with the string
> __real. Then *all* symbols are prepped for wrapping, regardless of if we use
> them now or even know about them. Only the symbols we've explicitly wrapped
> will be available. Note that I haven't actually implemented this yet, but it
> seems to me like it captures the positive aspects of all the alternatives.

Sorry, I find that rather un-aesthetic - We are only looking at a trivial
handful of functions, and to copy and rename ALL the symbols in libgcc.a
is rather overkill

The other thing I like about using PRIVATE_LIBGCC is that on 64-bit build
platforms, we don't need to worry about installing 32-bit libraries - All
we need to do is add the -m32 option to tell gcc and ld to generate 32-bit
code

Regards,

Graeme

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-11  2:22                               ` Graeme Russ
@ 2011-11-11  2:41                                 ` Gabe Black
  2011-11-11  4:49                                   ` Graeme Russ
  0 siblings, 1 reply; 36+ messages in thread
From: Gabe Black @ 2011-11-11  2:41 UTC (permalink / raw)
  To: u-boot

On Thu, Nov 10, 2011 at 6:22 PM, Graeme Russ <graeme.russ@gmail.com> wrote:

> Hi Gabe,
>
> On Fri, Nov 11, 2011 at 1:10 PM, Gabe Black <gabeblack@google.com> wrote:
> > On Thu, Nov 10, 2011 at 5:59 PM, Graeme Russ <graeme.russ@gmail.com>
> wrote:
> >>
> >> Hi Mike,
> >>
> >> On Fri, Nov 11, 2011 at 12:55 PM, Mike Frysinger <vapier@gentoo.org>
> >> wrote:
> >> > On Thursday 10 November 2011 20:51:47 Graeme Russ wrote:
> >> >> A few questions (I am unfamiliar with the Linux build environment):
> >> >>
> >> >>  a) Does Linux link to libgcc
> >> >
> >> > no Linux port uses libgcc.  they've always done the equivalent of
> >> > PRIVATE_LIBGCC.  but in the case of x86, i can't see them providing
> any
> >> > libgcc
> >> > funcs.  so i don't think u-boot should either.
> >> >
> >> >>  b) Does Linux use regparm
> >> >
> >> > yes, it uses -mregparm=3
> >>
> >> Well I think we have an answer - use PRIVATE_LIBGCC but do not implement
> >> any libgcc functions and treat link errors as coding errors. If for some
> >> bizarre reason we need to really, truly, honestly use a 64-bit libgcc
> >> function, we'll port it over then
> >>
> >> Regards,
> >>
> >> Graeme
> >
> > I haven't checked in on this thread in a little while, but I wanted to
> point
> > out some things. First, I was originally planning to measure the
> performance
> > difference with regparm turned off. I realized that would be quite
> annoying
> > to actually do, though, since we have a number of extra libraries linked
> > into u-boot and they would all have to be recompiled with different
> options.
>
> I assume they are all GPL :)
>
> > Then the things they link with would have to be recompiled, etc. Even if
> > upstream U-Boot drops regparm, we may need to keep it just for that
> reason.
>
> Hmm, that raises an interresing point - by using a non-standard ABI we can
> run afoul of external libraries. Which raises the question - what external
> libraries do you need to link to?
>


You have it reversed. By using a standard ABI we run afoul of external
libraries. We implement our verified boot infrastructure as a library most
notably, and it links in a few other libraries.



>
> > Second, I think I have a solution that preserves regparm, keeps libgcc
> and
> > gcc in sync, and also stops unwrapped functions slipping into u-boot. We
> can
> > use this command:
> > objcopy /path/to/your/libgcc/libgcc.a
> > /somewhere/in/the/u-boot/build/libgcc.a --prefix-symbols=__real
> > to create a libgcc that has all of its symbols prefixed with the string
> > __real. Then *all* symbols are prepped for wrapping, regardless of if we
> use
> > them now or even know about them. Only the symbols we've explicitly
> wrapped
> > will be available. Note that I haven't actually implemented this yet,
> but it
> > seems to me like it captures the positive aspects of all the
> alternatives.
>
> Sorry, I find that rather un-aesthetic - We are only looking at a trivial
> handful of functions, and to copy and rename ALL the symbols in libgcc.a
> is rather overkill
>
> The other thing I like about using PRIVATE_LIBGCC is that on 64-bit build
> platforms, we don't need to worry about installing 32-bit libraries - All
> we need to do is add the -m32 option to tell gcc and ld to generate 32-bit
> code



This argument really doesn't make any sense to me. The number of symbols
involved doesn't make any difference at all. You could imagine we are only
copying a few if you want and there would be no visible, hence aesthetic,
difference. Reimplementing libgcc, along with the inevitable bugs that go
along with new code, breaking builds when new functions are added silently
by new versions of gcc, etc., seems much less appealing in basically every
respect.

Gabe

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-11  1:59                           ` Graeme Russ
  2011-11-11  2:10                             ` Gabe Black
@ 2011-11-11  2:44                             ` Mike Frysinger
  1 sibling, 0 replies; 36+ messages in thread
From: Mike Frysinger @ 2011-11-11  2:44 UTC (permalink / raw)
  To: u-boot

On Thursday 10 November 2011 20:59:46 Graeme Russ wrote:
> Well I think we have an answer - use PRIVATE_LIBGCC but do not implement
> any libgcc functions and treat link errors as coding errors. If for some
> bizarre reason we need to really, truly, honestly use a 64-bit libgcc
> function, we'll port it over then

that's not how USE_PRIVATE_LIBGCC works in u-boot though.  i'd suggest:
	echo "PLATFORM_LIBGCC :=" >> arch/x86/config.mk
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111110/249155d7/attachment.pgp 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-11  2:41                                 ` Gabe Black
@ 2011-11-11  4:49                                   ` Graeme Russ
  2011-11-11  5:04                                     ` Mike Frysinger
  0 siblings, 1 reply; 36+ messages in thread
From: Graeme Russ @ 2011-11-11  4:49 UTC (permalink / raw)
  To: u-boot

Hi Gabe,

On Fri, Nov 11, 2011 at 1:41 PM, Gabe Black <gabeblack@google.com> wrote:
>
>
> On Thu, Nov 10, 2011 at 6:22 PM, Graeme Russ <graeme.russ@gmail.com> wrote:
>>
>> Hi Gabe,
>>
>> On Fri, Nov 11, 2011 at 1:10 PM, Gabe Black <gabeblack@google.com> wrote:
>> > On Thu, Nov 10, 2011 at 5:59 PM, Graeme Russ <graeme.russ@gmail.com>
>> > wrote:
>> >>
>> >> Hi Mike,
>> >>
>> >> On Fri, Nov 11, 2011 at 12:55 PM, Mike Frysinger <vapier@gentoo.org>
>> >> wrote:
>> >> > On Thursday 10 November 2011 20:51:47 Graeme Russ wrote:

[snip]

>> Hmm, that raises an interresing point - by using a non-standard ABI we can
>> run afoul of external libraries. Which raises the question - what external
>> libraries do you need to link to?
>
>
> You have it reversed. By using a standard ABI we run afoul of external
> libraries. We implement our verified boot infrastructure as a library most
> notably, and it links in a few other libraries.

Ah, so your libraries are regparm(3)

Still, life is going to get interesting for anyone linking in libraries
that are not the saem ABI as U-Boot (as we have seen with libgcc)

>> > Second, I think I have a solution that preserves regparm, keeps libgcc
>> > and
>> > gcc in sync, and also stops unwrapped functions slipping into u-boot. We
>> > can
>> > use this command:
>> > objcopy /path/to/your/libgcc/libgcc.a
>> > /somewhere/in/the/u-boot/build/libgcc.a --prefix-symbols=__real
>> > to create a libgcc that has all of its symbols prefixed with the string
>> > __real. Then *all* symbols are prepped for wrapping, regardless of if we
>> > use
>> > them now or even know about them. Only the symbols we've explicitly
>> > wrapped
>> > will be available. Note that I haven't actually implemented this yet,
>> > but it
>> > seems to me like it captures the positive aspects of all the
>> > alternatives.
>>
>> Sorry, I find that rather un-aesthetic - We are only looking at a trivial
>> handful of functions, and to copy and rename ALL the symbols in libgcc.a
>> is rather overkill
>>
>> The other thing I like about using PRIVATE_LIBGCC is that on 64-bit build
>> platforms, we don't need to worry about installing 32-bit libraries - All
>> we need to do is add the -m32 option to tell gcc and ld to generate 32-bit
>> code
>
> This argument really doesn't make any sense to me. The number of symbols
> involved doesn't make any difference at all. You could imagine we are only
> copying a few if you want and there would be no visible, hence aesthetic,
> difference. Reimplementing libgcc, along with the inevitable bugs that go
> along with new code, breaking builds when new functions are added silently
> by new versions of gcc, etc., seems much less appealing in basically every
> respect.

Remember, U-Boot uses --no-builtin, so apart from the libgcc functions,
there are no gcc functions included. And we are not 'reimplementing
libgcc', we are either 'reimplementing a select few functions' or 'not
using any libgcc functions in U-Boot at all'. So a build will only break
when code is added that uses a function never before used - There should
not be any build breakage due to 'functions added silently by newer
versions of gcc'

I'm assuming that it is the 'verified boot infrastructure' libraries that
are introducing the calls to libgcc? If that is the case, then it is
starting to get very messy - Why should U-Boot have to deal with this?

Regards,

Graeme

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-11  4:49                                   ` Graeme Russ
@ 2011-11-11  5:04                                     ` Mike Frysinger
  2011-11-11  5:16                                       ` Graeme Russ
  0 siblings, 1 reply; 36+ messages in thread
From: Mike Frysinger @ 2011-11-11  5:04 UTC (permalink / raw)
  To: u-boot

On Thursday 10 November 2011 23:49:07 Graeme Russ wrote:
> Remember, U-Boot uses --no-builtin, so apart from the libgcc functions,
> there are no gcc functions included.

i don't think that's generally how gcc builtin's work.  for the vast majority, 
they're of the "optimize away with simple insns when possible" variety.  so if 
you do something like:
	char c[4];
	memset(c, 0, sizeof(c));
gcc will optimize that into a single 32bit load rather than calling memcpy().  
but because we use -fno-builtins, gcc will make sure to call memcpy().

i can't think of any calls off the top of my head which would result in 
invoking a func in libgcc.a.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111111/12662c0a/attachment.pgp 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-11  5:04                                     ` Mike Frysinger
@ 2011-11-11  5:16                                       ` Graeme Russ
  2011-11-11 16:24                                         ` Mike Frysinger
  0 siblings, 1 reply; 36+ messages in thread
From: Graeme Russ @ 2011-11-11  5:16 UTC (permalink / raw)
  To: u-boot

Hi Mike,

On Fri, Nov 11, 2011 at 4:04 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Thursday 10 November 2011 23:49:07 Graeme Russ wrote:
>> Remember, U-Boot uses --no-builtin, so apart from the libgcc functions,
>> there are no gcc functions included.
>
> i don't think that's generally how gcc builtin's work.  for the vast majority,
> they're of the "optimize away with simple insns when possible" variety.  so if
> you do something like:
>        char c[4];
>        memset(c, 0, sizeof(c));
> gcc will optimize that into a single 32bit load rather than calling memcpy().
> but because we use -fno-builtins, gcc will make sure to call memcpy().

List of builtin functions not in libgcc:

http://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html

> i can't think of any calls off the top of my head which would result in
> invoking a func in libgcc.a.

Any function listed here:

http://gcc.gnu.org/onlinedocs/gccint/Libgcc.html

But we can discount any float/double routines, exception handling and
split stack which leaves just:

http://gcc.gnu.org/onlinedocs/gccint/Integer-library-routines.html

Regards,

Graeme

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-11  5:16                                       ` Graeme Russ
@ 2011-11-11 16:24                                         ` Mike Frysinger
  0 siblings, 0 replies; 36+ messages in thread
From: Mike Frysinger @ 2011-11-11 16:24 UTC (permalink / raw)
  To: u-boot

On Friday 11 November 2011 00:16:47 Graeme Russ wrote:
> On Fri, Nov 11, 2011 at 4:04 PM, Mike Frysinger wrote:
> > i can't think of any calls off the top of my head which would result in
> > invoking a func in libgcc.a.
> 
> Any function listed here:
> 
> http://gcc.gnu.org/onlinedocs/gccint/Libgcc.html
> 
> But we can discount any float/double routines, exception handling and
> split stack which leaves just:
> 
> http://gcc.gnu.org/onlinedocs/gccint/Integer-library-routines.html

yes, and pretty much all of those are emitted implicitly due to math 
operations (64bit divs/mults/etc...), or explicitly when the user does 
__builtin_xxx() (and use of __builtin_xxx is not affected by -fno-builtins).  
none of those that i can see would come via a C library call that gcc would 
implicitly rewrite.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111111/594c08d2/attachment.pgp 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-11  1:55                         ` Mike Frysinger
  2011-11-11  1:59                           ` Graeme Russ
@ 2011-11-11 19:59                           ` Scott Wood
  1 sibling, 0 replies; 36+ messages in thread
From: Scott Wood @ 2011-11-11 19:59 UTC (permalink / raw)
  To: u-boot

On 11/10/2011 07:55 PM, Mike Frysinger wrote:
> On Thursday 10 November 2011 20:51:47 Graeme Russ wrote:
>> A few questions (I am unfamiliar with the Linux build environment):
>>
>>  a) Does Linux link to libgcc
> 
> no Linux port uses libgcc.  they've always done the equivalent of 
> PRIVATE_LIBGCC.  but in the case of x86, i can't see them providing any libgcc 
> funcs.  so i don't think u-boot should either.

Some of the less common architectures (openrisc, h8300, cris, m32r,
tile, xtensa) appear to use libgcc.

unicore32 appears to pull selected objects out of both libgcc and libc.

-Scott

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage
  2011-11-09 10:32   ` [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage Graeme Russ
  2011-11-09 17:12     ` Mike Frysinger
@ 2011-11-16 23:00     ` Graeme Russ
  1 sibling, 0 replies; 36+ messages in thread
From: Graeme Russ @ 2011-11-16 23:00 UTC (permalink / raw)
  To: u-boot

Hi Gabe,

On Wed, Nov 9, 2011 at 9:32 PM, Graeme Russ <graeme.russ@gmail.com> wrote:
> Hi Gabe,
>
> Can you please try this patch - If it solves your libgcc problem, I will
> add it to the misc cleanup patch
>
> Thanks,
>
> Graeme
> ---
> ?arch/x86/config.mk ? ? ? ?| ? ?3 ---
> ?arch/x86/cpu/interrupts.c | ? ?2 +-
> ?arch/x86/cpu/start.S ? ? ?| ? ?5 ++---
> ?3 files changed, 3 insertions(+), 7 deletions(-)

I think we all agree this is not the way to go

Regards,

Graeme

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [PATCH v3] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch
  2011-11-09  5:34         ` Mike Frysinger
@ 2011-11-17  9:01           ` Gabe Black
  2011-11-17  9:13             ` Gabe Black
  2011-11-30 11:03             ` Graeme Russ
  0 siblings, 2 replies; 36+ messages in thread
From: Gabe Black @ 2011-11-17  9:01 UTC (permalink / raw)
  To: u-boot

When gcc compiles some 64 bit operations on a 32 bit machine, it generates
calls to small functions instead of instructions which do the job directly.
Those functions are defined in libgcc and transparently provide whatever
functionality was necessary. Unfortunately, u-boot can be built with a
non-standard ABI when libgcc isn't. More specifically, u-boot uses
-mregparm. When the u-boot and libgcc are linked together, very confusing
bugs can crop up, for instance seemingly normal integer division or modulus
getting the wrong answer or even raising a spurious divide by zero
exception.

This change borrows (steals) a technique and some code from coreboot which
solves this problem by creating wrappers which translate the calling
convention when calling the functions in libgcc. Unfortunately that means
that these instructions which had already been turned into functions have
even more overhead, but more importantly it makes them work properly.

To find all of the functions that needed wrapping, u-boot was compiled
without linking in libgcc. All the symbols the linker complained were
undefined were presumed to be the symbols that are needed from libgcc.
These were a subset of the symbols covered by the coreboot code, so it was
used unmodified.

To prevent symbols which are provided by libgcc but not currently wrapped
(or even known about) from being silently linked against by code generated
by libgcc, a new copy of libgcc is created where all the symbols are
prefixed with __normal_. Without being purposefully wrapped, these symbols
will cause linker errors instead of silently introducing very subtle,
confusing bugs.

Another approach would be to whitelist symbols from libgcc and strip out
all the others. The problem with this approach is that it requires the
white listed symbols to be specified three times, once for objcopy, once so
the linker inserts the wrapped, and once to generate the wrapper itself,
while this implementation needs it to be listed only twice. There isn't
much tangible difference in what each approach produces, so this one was
preferred.

Signed-off-by: Gabe Black <gabeblack@chromium.org>
---
Changes in v2:
- Change the [x86] tag to x86:
- Mention -mregparm in the commit message.
- Get rid of a stray line which snuck in during a rebase.

Changes in v3:
- Prevent symbols from libgcc which aren't wrapped from getting silently
picked up by the linker.

 arch/x86/config.mk    |    7 +++++++
 arch/x86/lib/Makefile |    6 ++++++
 arch/x86/lib/gcc.c    |   38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 0 deletions(-)
 create mode 100644 arch/x86/lib/gcc.c

diff --git a/arch/x86/config.mk b/arch/x86/config.mk
index fe9083f..23cacff 100644
--- a/arch/x86/config.mk
+++ b/arch/x86/config.mk
@@ -41,3 +41,10 @@ PLATFORM_RELFLAGS += -ffunction-sections -fvisibility=hidden
 PLATFORM_LDFLAGS += --emit-relocs -Bsymbolic -Bsymbolic-functions
 
 LDFLAGS_FINAL += --gc-sections -pie
+LDFLAGS_FINAL += --wrap=__divdi3 --wrap=__udivdi3
+LDFLAGS_FINAL += --wrap=__moddi3 --wrap=__umoddi3
+
+NORMAL_LIBGCC = $(shell $(CC) $(CFLAGS) -print-libgcc-file-name)
+PREFIXED_LIBGCC = $(OBJTREE)/arch/$(ARCH)/lib/$(shell basename $(NORMAL_LIBGCC))
+
+export USE_PRIVATE_LIBGCC=$(shell dirname $(PREFIXED_LIBGCC))
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index eb5fa10..16db73f 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -32,6 +32,7 @@ SOBJS-$(CONFIG_SYS_X86_REALMODE)	+= realmode_switch.o
 COBJS-$(CONFIG_SYS_PC_BIOS)	+= bios_setup.o
 COBJS-y	+= board.o
 COBJS-y	+= bootm.o
+COBJS-y	+= gcc.o
 COBJS-y	+= interrupts.o
 COBJS-$(CONFIG_SYS_PCAT_INTERRUPTS) += pcat_interrupts.o
 COBJS-$(CONFIG_SYS_GENERIC_TIMER) += pcat_timer.o
@@ -49,6 +50,11 @@ OBJS	:= $(addprefix $(obj),$(SOBJS-y) $(COBJS-y))
 $(LIB):	$(obj).depend $(OBJS)
 	$(call cmd_link_o_target, $(OBJS))
 
+$(PREFIXED_LIBGCC): $(NORMAL_LIBGCC)
+	$(OBJCOPY) $< $@ --prefix-symbols=__normal_
+
+$(LIB): $(PREFIXED_LIBGCC)
+
 #########################################################################
 
 # defines $(obj).depend target
diff --git a/arch/x86/lib/gcc.c b/arch/x86/lib/gcc.c
new file mode 100644
index 0000000..4043431
--- /dev/null
+++ b/arch/x86/lib/gcc.c
@@ -0,0 +1,38 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2009 coresystems GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 or later of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA, 02110-1301 USA
+ */
+
+#ifdef __GNUC__
+
+/*
+ * GCC's libgcc handling is quite broken. While the libgcc functions
+ * are always regparm(0) the code that calls them uses whatever the
+ * compiler call specifies. Therefore we need a wrapper around those
+ * functions. See gcc bug PR41055 for more information.
+ */
+#define WRAP_LIBGCC_CALL(type, name) \
+	type __normal_##name(type a, type b) __attribute__((regparm(0))); \
+	type __wrap_##name(type a, type b); \
+	type __wrap_##name(type a, type b) { return __normal_##name(a, b); }
+
+WRAP_LIBGCC_CALL(long long, __divdi3)
+WRAP_LIBGCC_CALL(unsigned long long, __udivdi3)
+WRAP_LIBGCC_CALL(long long, __moddi3)
+WRAP_LIBGCC_CALL(unsigned long long, __umoddi3)
+
+#endif
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [U-Boot] [PATCH v3] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch
  2011-11-17  9:01           ` [U-Boot] [PATCH v3] " Gabe Black
@ 2011-11-17  9:13             ` Gabe Black
  2011-11-30 11:03             ` Graeme Russ
  1 sibling, 0 replies; 36+ messages in thread
From: Gabe Black @ 2011-11-17  9:13 UTC (permalink / raw)
  To: u-boot

On Thu, Nov 17, 2011 at 1:01 AM, Gabe Black <gabeblack@chromium.org> wrote:

> When gcc compiles some 64 bit operations on a 32 bit machine, it generates
> calls to small functions instead of instructions which do the job directly.
> Those functions are defined in libgcc and transparently provide whatever
> functionality was necessary. Unfortunately, u-boot can be built with a
> non-standard ABI when libgcc isn't. More specifically, u-boot uses
> -mregparm. When the u-boot and libgcc are linked together, very confusing
> bugs can crop up, for instance seemingly normal integer division or modulus
> getting the wrong answer or even raising a spurious divide by zero
> exception.
>
> This change borrows (steals) a technique and some code from coreboot which
> solves this problem by creating wrappers which translate the calling
> convention when calling the functions in libgcc. Unfortunately that means
> that these instructions which had already been turned into functions have
> even more overhead, but more importantly it makes them work properly.
>
> To find all of the functions that needed wrapping, u-boot was compiled
> without linking in libgcc. All the symbols the linker complained were
> undefined were presumed to be the symbols that are needed from libgcc.
> These were a subset of the symbols covered by the coreboot code, so it was
> used unmodified.
>
> To prevent symbols which are provided by libgcc but not currently wrapped
> (or even known about) from being silently linked against by code generated
> by libgcc, a new copy of libgcc is created where all the symbols are
> prefixed with __normal_. Without being purposefully wrapped, these symbols
> will cause linker errors instead of silently introducing very subtle,
> confusing bugs.
>
> Another approach would be to whitelist symbols from libgcc and strip out
> all the others. The problem with this approach is that it requires the
> white listed symbols to be specified three times, once for objcopy, once so
> the linker inserts the wrapped, and once to generate the wrapper itself,
> while this implementation needs it to be listed only twice. There isn't
> much tangible difference in what each approach produces, so this one was
> preferred.
>
> Signed-off-by: Gabe Black <gabeblack@chromium.org>
> ---
> Changes in v2:
> - Change the [x86] tag to x86:
> - Mention -mregparm in the commit message.
> - Get rid of a stray line which snuck in during a rebase.
>
> Changes in v3:
> - Prevent symbols from libgcc which aren't wrapped from getting silently
> picked up by the linker.
>
>  arch/x86/config.mk    |    7 +++++++
>  arch/x86/lib/Makefile |    6 ++++++
>  arch/x86/lib/gcc.c    |   38 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 51 insertions(+), 0 deletions(-)
>  create mode 100644 arch/x86/lib/gcc.c
>
> diff --git a/arch/x86/config.mk b/arch/x86/config.mk
> index fe9083f..23cacff 100644
> --- a/arch/x86/config.mk
> +++ b/arch/x86/config.mk
> @@ -41,3 +41,10 @@ PLATFORM_RELFLAGS += -ffunction-sections
> -fvisibility=hidden
>  PLATFORM_LDFLAGS += --emit-relocs -Bsymbolic -Bsymbolic-functions
>
>  LDFLAGS_FINAL += --gc-sections -pie
> +LDFLAGS_FINAL += --wrap=__divdi3 --wrap=__udivdi3
> +LDFLAGS_FINAL += --wrap=__moddi3 --wrap=__umoddi3
> +
> +NORMAL_LIBGCC = $(shell $(CC) $(CFLAGS) -print-libgcc-file-name)
> +PREFIXED_LIBGCC = $(OBJTREE)/arch/$(ARCH)/lib/$(shell basename
> $(NORMAL_LIBGCC))
> +
> +export USE_PRIVATE_LIBGCC=$(shell dirname $(PREFIXED_LIBGCC))
> diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
> index eb5fa10..16db73f 100644
> --- a/arch/x86/lib/Makefile
> +++ b/arch/x86/lib/Makefile
> @@ -32,6 +32,7 @@ SOBJS-$(CONFIG_SYS_X86_REALMODE)      +=
> realmode_switch.o
>  COBJS-$(CONFIG_SYS_PC_BIOS)    += bios_setup.o
>  COBJS-y        += board.o
>  COBJS-y        += bootm.o
> +COBJS-y        += gcc.o
>  COBJS-y        += interrupts.o
>  COBJS-$(CONFIG_SYS_PCAT_INTERRUPTS) += pcat_interrupts.o
>  COBJS-$(CONFIG_SYS_GENERIC_TIMER) += pcat_timer.o
> @@ -49,6 +50,11 @@ OBJS := $(addprefix $(obj),$(SOBJS-y) $(COBJS-y))
>  $(LIB):        $(obj).depend $(OBJS)
>        $(call cmd_link_o_target, $(OBJS))
>
> +$(PREFIXED_LIBGCC): $(NORMAL_LIBGCC)
> +       $(OBJCOPY) $< $@ --prefix-symbols=__normal_
> +
> +$(LIB): $(PREFIXED_LIBGCC)
> +
>  #########################################################################
>
>  # defines $(obj).depend target
> diff --git a/arch/x86/lib/gcc.c b/arch/x86/lib/gcc.c
> new file mode 100644
> index 0000000..4043431
> --- /dev/null
> +++ b/arch/x86/lib/gcc.c
> @@ -0,0 +1,38 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2009 coresystems GmbH
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 or later of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA, 02110-1301
> USA
> + */
> +
> +#ifdef __GNUC__
> +
> +/*
> + * GCC's libgcc handling is quite broken. While the libgcc functions
> + * are always regparm(0) the code that calls them uses whatever the
> + * compiler call specifies. Therefore we need a wrapper around those
> + * functions. See gcc bug PR41055 for more information.
> + */
> +#define WRAP_LIBGCC_CALL(type, name) \
> +       type __normal_##name(type a, type b) __attribute__((regparm(0))); \
> +       type __wrap_##name(type a, type b); \
> +       type __wrap_##name(type a, type b) { return __normal_##name(a, b);
> }
> +
> +WRAP_LIBGCC_CALL(long long, __divdi3)
> +WRAP_LIBGCC_CALL(unsigned long long, __udivdi3)
> +WRAP_LIBGCC_CALL(long long, __moddi3)
> +WRAP_LIBGCC_CALL(unsigned long long, __umoddi3)
> +
> +#endif
> --
> 1.7.3.1
>
>

The patch I just sent out is not 100% checkpatch clean (1 error, 1
warning), but the error is because it misidentifies the macro in gcc.c as a
collection of statements instead of function prototypes/definitions and
insists they're wrapped in a do {} while loop. That wouldn't be possible,
so I'm sending this out as is. There may also be small bits of review
feedback I haven't implemented (the name of gcc.c?) because I wasn't able
to find the email the feedback was in. Please send them again and I'll spin
another version as necessary.

This has been tested in our tree for our build target and upstream for the
eNET target, both in our environment. It wouldn't be a bad idea to test it
again in a different environment to be sure I'm not just getting lucky
somehow.

Gabe

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [U-Boot] [PATCH v3] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch
  2011-11-17  9:01           ` [U-Boot] [PATCH v3] " Gabe Black
  2011-11-17  9:13             ` Gabe Black
@ 2011-11-30 11:03             ` Graeme Russ
  1 sibling, 0 replies; 36+ messages in thread
From: Graeme Russ @ 2011-11-30 11:03 UTC (permalink / raw)
  To: u-boot

Hi Gabe,

On 17/11/11 20:01, Gabe Black wrote:
> When gcc compiles some 64 bit operations on a 32 bit machine, it generates
> calls to small functions instead of instructions which do the job directly.
> Those functions are defined in libgcc and transparently provide whatever
> functionality was necessary. Unfortunately, u-boot can be built with a
> non-standard ABI when libgcc isn't. More specifically, u-boot uses
> -mregparm. When the u-boot and libgcc are linked together, very confusing
> bugs can crop up, for instance seemingly normal integer division or modulus
> getting the wrong answer or even raising a spurious divide by zero
> exception.
> 
> This change borrows (steals) a technique and some code from coreboot which
> solves this problem by creating wrappers which translate the calling
> convention when calling the functions in libgcc. Unfortunately that means
> that these instructions which had already been turned into functions have
> even more overhead, but more importantly it makes them work properly.
> 
> To find all of the functions that needed wrapping, u-boot was compiled
> without linking in libgcc. All the symbols the linker complained were
> undefined were presumed to be the symbols that are needed from libgcc.
> These were a subset of the symbols covered by the coreboot code, so it was
> used unmodified.
> 
> To prevent symbols which are provided by libgcc but not currently wrapped
> (or even known about) from being silently linked against by code generated
> by libgcc, a new copy of libgcc is created where all the symbols are
> prefixed with __normal_. Without being purposefully wrapped, these symbols
> will cause linker errors instead of silently introducing very subtle,
> confusing bugs.
> 
> Another approach would be to whitelist symbols from libgcc and strip out
> all the others. The problem with this approach is that it requires the
> white listed symbols to be specified three times, once for objcopy, once so
> the linker inserts the wrapped, and once to generate the wrapper itself,
> while this implementation needs it to be listed only twice. There isn't
> much tangible difference in what each approach produces, so this one was
> preferred.
> 
> Signed-off-by: Gabe Black <gabeblack@chromium.org>
> ---
> Changes in v2:
> - Change the [x86] tag to x86:
> - Mention -mregparm in the commit message.
> - Get rid of a stray line which snuck in during a rebase.
> 
> Changes in v3:
> - Prevent symbols from libgcc which aren't wrapped from getting silently
> picked up by the linker.
> 
>  arch/x86/config.mk    |    7 +++++++
>  arch/x86/lib/Makefile |    6 ++++++
>  arch/x86/lib/gcc.c    |   38 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 51 insertions(+), 0 deletions(-)
>  create mode 100644 arch/x86/lib/gcc.c

Applied to u-boot-x86/master

Thanks,

Graeme

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2011-11-30 11:03 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-08  9:27 [U-Boot] [PATCH] [x86] Wrap small helper functions from libgcc to avoid an ABI mismatch Gabe Black
2011-11-08 13:33 ` Mike Frysinger
2011-11-08 22:27   ` Gabe Black
2011-11-08 22:34 ` [U-Boot] [PATCH v2] x86: " Gabe Black
2011-11-08 23:14   ` Graeme Russ
2011-11-09  4:49     ` Mike Frysinger
2011-11-09  3:57       ` Graeme Russ
2011-11-09  5:34         ` Mike Frysinger
2011-11-17  9:01           ` [U-Boot] [PATCH v3] " Gabe Black
2011-11-17  9:13             ` Gabe Black
2011-11-30 11:03             ` Graeme Russ
2011-11-09  3:05   ` [U-Boot] [PATCH v2] " Graeme Russ
2011-11-09 10:32   ` [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage Graeme Russ
2011-11-09 17:12     ` Mike Frysinger
2011-11-09 21:42       ` Graeme Russ
2011-11-10  4:13         ` Mike Frysinger
2011-11-10  4:22           ` Graeme Russ
2011-11-10  5:10             ` Graeme Russ
2011-11-10 17:15             ` Mike Frysinger
2011-11-10 22:53               ` Graeme Russ
2011-11-11  0:23                 ` Mike Frysinger
2011-11-11  1:23                   ` Graeme Russ
2011-11-11  1:40                     ` Mike Frysinger
2011-11-11  1:51                       ` Graeme Russ
2011-11-11  1:55                         ` Mike Frysinger
2011-11-11  1:59                           ` Graeme Russ
2011-11-11  2:10                             ` Gabe Black
2011-11-11  2:22                               ` Graeme Russ
2011-11-11  2:41                                 ` Gabe Black
2011-11-11  4:49                                   ` Graeme Russ
2011-11-11  5:04                                     ` Mike Frysinger
2011-11-11  5:16                                       ` Graeme Russ
2011-11-11 16:24                                         ` Mike Frysinger
2011-11-11  2:44                             ` Mike Frysinger
2011-11-11 19:59                           ` Scott Wood
2011-11-16 23:00     ` Graeme Russ

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.