Skip to contentSkip to conversation

Import the glibc implementation of x86 memset into u-boot

PATCH Superseded Gabe Black 6 participants 28 messages Nov 8, 2011 – Nov 30, 2011 x86

lore
Share & tools
lore

Gabe Black

author
#0 lore raw

PATCH 1/1 Import the glibc implementation of x86 memset into u-boot

The new implementation is about twice as fast as the old.

Signed-off-by Gabe Black <[email protected]>
---
 arch/x86/include/asm/string.h |    2 +-
 arch/x86/lib/Makefile         |    1 +
 arch/x86/lib/string.c         |   87 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/lib/string.c
arch/x86/include/asm/string.h +1 -1
index 3643a79..3aa6c11 100644
@@ -23,7 +23,7 @@ extern void * memmove(void *, const void *, __kernel_size_t);
 #undef __HAVE_ARCH_MEMCHR
 extern void * memchr(const void *, int, __kernel_size_t);
 
-#undef __HAVE_ARCH_MEMSET
+#define __HAVE_ARCH_MEMSET
 extern void * memset(void *, int, __kernel_size_t);
 
 #undef __HAVE_ARCH_MEMZERO
arch/x86/lib/Makefile +1 -0
index 71e94f7..d9cabdd 100644
@@ -38,6 +38,7 @@ COBJS-$(CONFIG_SYS_GENERIC_TIMER) += pcat_timer.o
 COBJS-$(CONFIG_PCI) += pci.o
 COBJS-$(CONFIG_PCI) += pci_type1.o
 COBJS-y	+= realmode.o
+COBJS-y	+= string.o
 COBJS-y	+= timer.o
 COBJS-y	+= video_bios.o
 COBJS-y	+= video.o
arch/x86/lib/string.c new file +87 -0

87 lines not on this page · raw message

Signature · 1 line
1.7.3.1
#1 lore raw

PATCH 1/1 Import the glibc implementation of x86 memset into u-boot

i'd tweak the summary to be something like:
[x86] import the glibc memset implementation

otherwise,

Acked-by Mike Frysinger <[email protected]>

-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/7ea0b3cb/attachment.pgp

author
#2 lore raw

PATCH v2 1/1 x86: Import the glibc memset implementation

In reply to Gabe Black

The new implementation is about twice as fast as the old.

Signed-off-by Gabe Black <[email protected]>

---
Changes in v2:
Update the commit summary as suggested by Mike Frysinger.

 arch/x86/include/asm/string.h |    2 +-
 arch/x86/lib/Makefile         |    1 +
 arch/x86/lib/string.c         |   87 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/lib/string.c
arch/x86/include/asm/string.h +1 -1
index 3643a79..3aa6c11 100644
@@ -23,7 +23,7 @@ extern void * memmove(void *, const void *, __kernel_size_t);
 #undef __HAVE_ARCH_MEMCHR
 extern void * memchr(const void *, int, __kernel_size_t);
 
-#undef __HAVE_ARCH_MEMSET
+#define __HAVE_ARCH_MEMSET
 extern void * memset(void *, int, __kernel_size_t);
 
 #undef __HAVE_ARCH_MEMZERO
arch/x86/lib/Makefile +1 -0
index 71e94f7..d9cabdd 100644
@@ -38,6 +38,7 @@ COBJS-$(CONFIG_SYS_GENERIC_TIMER) += pcat_timer.o
 COBJS-$(CONFIG_PCI) += pci.o
 COBJS-$(CONFIG_PCI) += pci_type1.o
 COBJS-y	+= realmode.o
+COBJS-y	+= string.o
 COBJS-y	+= timer.o
 COBJS-y	+= video_bios.o
 COBJS-y	+= video.o
arch/x86/lib/string.c new file +87 -0

87 lines not on this page · raw message

Signature · 1 line
1.7.3.1
#3 lore raw

PATCH v2 1/1 x86: Import the glibc memset implementation

Hi Gabe,

Quoted 2 lines · Gabe Black

The new implementation is about twice as fast as the old.

I don't get it - If this is the glibc implementation, we aren't we just using the implementation in the glibc library which we link to?

Regards,

Graeme

author
#4 lore raw

PATCH v2 1/1 x86: Import the glibc memset implementation

I'm pretty sure u-boot doesn't link with glibc. I'd expect that to cause all sorts of problems on top of being really big. There are default, generic implementations which it can use, or you can specialize them to take advantage of architecture specific features like I'm doing here.

Gabe

Quoted 14 lines · Graeme Russ

Hi Gabe,

Quoted 2 lines · Gabe Black

The new implementation is about twice as fast as the old.

I don't get it - If this is the glibc implementation, we aren't we just using the implementation in the glibc library which we link to?

Regards,

Graeme

#5 lore raw

PATCH v2 1/1 x86: Import the glibc memset implementation

Hi Gabe,

first up - Please stop top-posting

Quoted 4 lines · Gabe Black

I'm pretty sure u-boot doesn't link with glibc. I'd expect that to cause all sorts of problems on top of being really big. There are default, generic implementations which it can use, or you can specialize them to take advantage of architecture specific features like I'm doing here.

Hmm,

# Add GCC lib
ifdef USE_PRIVATE_LIBGCC
ifeq ("$(USE_PRIVATE_LIBGCC)", "yes") PLATFORM_LIBGCC = $(OBJTREE)/arch/$(ARCH)/lib/libgcc.o else
PLATFORM_LIBGCC = -L $(USE_PRIVATE_LIBGCC) -lgcc endif
else
PLATFORM_LIBGCC := -L $(shell dirname $(CC) $(CFLAGS) -print-libgcc-file-name) -lgcc
endif
PLATFORM_LIBS += $(PLATFORM_LIBGCC)
export PLATFORM_LIBS

So we link with libgcc - In another email I raise the question of why regparm is not conflicting here

And why do we have an option to not link to libgcc, but on the other hand we never link to libc - I'm confused

Regards,

Graeme

Quoted 17 lines · Graeme Russ
Quoted 14 lines

Hi Gabe,

Quoted 2 lines · Gabe Black

The new implementation is about twice as fast as the old.

I don't get it - If this is the glibc implementation, we aren't we just using the implementation in the glibc library which we link to?

Regards,

Graeme

#6 lore raw

PATCH v2 1/1 x86: Import the glibc memset implementation

Quoted 31 lines · Graeme Russ

Hi Gabe,

first up - Please stop top-posting

Quoted 1 line · Gabe Black

I'm pretty sure u-boot doesn't link with glibc. I'd expect that to cause

all

Quoted 3 lines

sorts of problems on top of being really big. There are default, generic implementations which it can use, or you can specialize them to take advantage of architecture specific features like I'm doing here.

Hmm,

# Add GCC lib
ifdef USE_PRIVATE_LIBGCC
ifeq ("$(USE_PRIVATE_LIBGCC)", "yes") PLATFORM_LIBGCC = $(OBJTREE)/arch/$(ARCH)/lib/libgcc.o else
PLATFORM_LIBGCC = -L $(USE_PRIVATE_LIBGCC) -lgcc endif
else
PLATFORM_LIBGCC := -L $(shell dirname $(CC) $(CFLAGS) -print-libgcc-file-name) -lgcc
endif
PLATFORM_LIBS += $(PLATFORM_LIBGCC)
export PLATFORM_LIBS

So we link with libgcc - In another email I raise the question of why regparm is not conflicting here

Quoted 8 lines

And why do we have an option to not link to libgcc, but on the other hand we never link to libc - I'm confused

Regards,

Graeme

I don't understand the question. regparm did conflict, which is the point of my other patch. Why would having an option to link libgcc have anything to do with glibc? U-Boot has its own partial libc implementation which has generic implementations of certain functions which can be overridden. By not linking to the generic implementation and providing a specialized one, the performance improves two fold. Those are the two choices.

Gabe

#7 lore raw

PATCH v2 1/1 x86: Import the glibc memset implementation

Quoted 38 lines · Gabe Black
Quoted 35 lines · Graeme Russ

Hi Gabe,

first up - Please stop top-posting

Quoted 5 lines · Gabe Black

I'm pretty sure u-boot doesn't link with glibc. I'd expect that to cause all
sorts of problems on top of being really big. There are default, generic implementations which it can use, or you can specialize them to take advantage of architecture specific features like I'm doing here.

Hmm,

# Add GCC lib
ifdef USE_PRIVATE_LIBGCC
ifeq ("$(USE_PRIVATE_LIBGCC)", "yes") PLATFORM_LIBGCC = $(OBJTREE)/arch/$(ARCH)/lib/libgcc.o else
PLATFORM_LIBGCC = -L $(USE_PRIVATE_LIBGCC) -lgcc endif
else
PLATFORM_LIBGCC := -L $(shell dirname $(CC) $(CFLAGS) -print-libgcc-file-name) -lgcc
endif
PLATFORM_LIBS += $(PLATFORM_LIBGCC)
export PLATFORM_LIBS

So we link with libgcc - In another email I raise the question of why regparm is not conflicting here

And why do we have an option to not link to libgcc, but on the other hand we never link to libc - I'm confused

Quoted 6 lines
Quoted 4 lines

Regards,

Graeme

I don't understand the question. regparm did conflict, which is the point of

Only on 64-bit operations - surely there are other libgcc functions not covered by your patch. And why had I not seen these before - was is a case of never hitting that call path?

Quoted 3 lines

my other patch. Why would having an option to link libgcc have anything to do with glibc? U-Boot has its own partial libc implementation which has generic implementations of certain functions which can be overridden. By not

My point is, why does U-Boot implement parts of libgcc so as to not link to libgcc, but uses (by default) glibc - Why not always implement both, or always (by default) link to both?

Quoted 2 lines

linking to the generic implementation and providing a specialized one, the performance improves two fold. Those are the two choices.

Regards,

Graeme

author
#8 lore raw

PATCH v2 1/1 x86: Import the glibc memset implementation

Quoted 47 lines · Graeme Russ
Quoted 3 lines · Gabe Black

On Tue, Nov 8, 2011 at 5:10 PM, Graeme Russ <[email protected]>

Quoted 8 lines · wrote:
Quoted 8 lines

Hi Gabe,

first up - Please stop top-posting

Quoted 1 line · Gabe Black

I'm pretty sure u-boot doesn't link with glibc. I'd expect that to

cause

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

all
sorts of problems on top of being really big. There are default,

generic

Quoted 30 lines
Quoted 29 lines
Quoted 2 lines

implementations which it can use, or you can specialize them to take advantage of architecture specific features like I'm doing here.

Hmm,

# Add GCC lib
ifdef USE_PRIVATE_LIBGCC
ifeq ("$(USE_PRIVATE_LIBGCC)", "yes") PLATFORM_LIBGCC = $(OBJTREE)/arch/$(ARCH)/lib/libgcc.o else
PLATFORM_LIBGCC = -L $(USE_PRIVATE_LIBGCC) -lgcc endif
else
PLATFORM_LIBGCC := -L $(shell dirname $(CC) $(CFLAGS) -print-libgcc-file-name) -lgcc
endif
PLATFORM_LIBS += $(PLATFORM_LIBGCC)
export PLATFORM_LIBS

So we link with libgcc - In another email I raise the question of why regparm is not conflicting here

And why do we have an option to not link to libgcc, but on the other hand we never link to libc - I'm confused

Regards,

Graeme

Quoted 7 lines
Quoted 1 line

I don't understand the question. regparm did conflict, which is the

point of

Only on 64-bit operations - surely there are other libgcc functions not covered by your patch. And why had I not seen these before - was is a case of never hitting that call path?

We didn't see problems for a while either. I guess 64 bit math isn't very common in u-boot. I think it also tends to "work" in the sense that nothing is wrong enough to fail and no divide by zero exception or other fatal problem crops up. There are possibly other libgcc functions which are not covered, but this is actually a superset of the functions that are currently being used, or at least circa the date this patch was put together. I don't think there's all that much to libgcc, but I don't know that for a fact.

Quoted 11 lines
Quoted 2 lines

my other patch. Why would having an option to link libgcc have anything

to

Quoted 2 lines

do with glibc? U-Boot has its own partial libc implementation which has generic implementations of certain functions which can be overridden. By

not

My point is, why does U-Boot implement parts of libgcc so as to not link to libgcc, but uses (by default) glibc - Why not always implement both, or always (by default) link to both?

I don't think it necessarily implements parts of libgcc, I think that's a per arch setting. Turning that (USE_PRIVATE_LIBGCC) on didn't work on x86 when I accidentally did that a while ago.

I would be very surprised if glibc was used by default or otherwise. Speaking perhaps too generally, glibc is a big library intended for user space applications running under an OS and wouldn't be appropriate to use in U-Boot. I have to assume that's why U-Boot implements the libc functionality it needs by itself without relying on an external library, even though there are libc implementations which are more appropriate for embedded/firmware use. In this case, we're grabbing a small bit of code out of glibc and transplanting it by itself into u-boot. There's no special linking involved, just porting over a single function.

Quoted 9 lines
Quoted 2 lines

linking to the generic implementation and providing a specialized one,

the

Quoted 1 line

performance improves two fold. Those are the two choices.

Regards,

Graeme

#9 lore raw

PATCH v2 1/1 x86: Import the glibc memset implementation

Quoted 59 lines · Gabe Black
Quoted 48 lines · Graeme Russ
Quoted 46 lines · Gabe Black
Quoted 41 lines · Graeme Russ

Hi Gabe,

first up - Please stop top-posting

Quoted 7 lines · Gabe Black

I'm pretty sure u-boot doesn't link with glibc. I'd expect that to cause
all
sorts of problems on top of being really big. There are default, generic
implementations which it can use, or you can specialize them to take advantage of architecture specific features like I'm doing here.

Hmm,

# Add GCC lib
ifdef USE_PRIVATE_LIBGCC
ifeq ("$(USE_PRIVATE_LIBGCC)", "yes") PLATFORM_LIBGCC = $(OBJTREE)/arch/$(ARCH)/lib/libgcc.o else
PLATFORM_LIBGCC = -L $(USE_PRIVATE_LIBGCC) -lgcc endif
else
PLATFORM_LIBGCC := -L $(shell dirname $(CC) $(CFLAGS) -print-libgcc-file-name) -lgcc
endif
PLATFORM_LIBS += $(PLATFORM_LIBGCC)
export PLATFORM_LIBS

So we link with libgcc - In another email I raise the question of why regparm is not conflicting here

And why do we have an option to not link to libgcc, but on the other hand we never link to libc - I'm confused

Regards,

Graeme

Quoted 6 lines
Quoted 2 lines

I don't understand the question. regparm did conflict, which is the point of

Only on 64-bit operations - surely there are other libgcc functions not covered by your patch. And why had I not seen these before - was is a case of never hitting that call path?

Quoted 18 lines

We didn't see problems for a while either. I guess 64 bit math isn't very common in u-boot. I think it also tends to "work" in the sense that nothing is wrong enough to fail and no divide by zero exception or other fatal problem crops up. There are possibly other libgcc functions which are not covered, but this is actually a superset of the functions that are currently being used, or at least circa the date this patch was put together. I don't think there's all that much to libgcc, but I don't know that for a fact.

Quoted 10 lines
Quoted 6 lines

my other patch. Why would having an option to link libgcc have anything to
do with glibc? U-Boot has its own partial libc implementation which has generic implementations of certain functions which can be overridden. By not

My point is, why does U-Boot implement parts of libgcc so as to not link to libgcc, but uses (by default) glibc - Why not always implement both, or always (by default) link to both?

Oops, I meant U-Boot implements a subset of (g)libc. I think I understand why - U-Boot needs specific implementations of stdio (printf() and friends) Because we need to implement _some_ of glibc, we need to implement all (or at least all the functions we use) otherwise we will get all sorts of symbol conflicts in the linker

Quoted 3 lines

I don't think it necessarily implements parts of libgcc, I think that's a per arch setting. Turning that (USE_PRIVATE_LIBGCC) on didn't work on x86 when I accidentally did that a while ago.

If you define USE_PRIVATE_LIBGCC then you need to implement the subset of libgcc required by U-Boot. See arch/arm/lib - Specifically the following are compiled if USE_PRIVATE_LIBGCC is defined:

	_ashldi3.o
	_ashrdi3.o
	_divsi3.o
	_lshrdi3.o
	_modsi3.o
	_udivsi3.o
	_umodsi3.o
	div0.o
Quoted 9 lines

I would be very surprised if glibc was used by default or otherwise. Speaking perhaps too generally, glibc is a big library intended for user space applications running under an OS and wouldn't be appropriate to use in U-Boot. I have to assume that's why U-Boot implements the libc functionality it needs by itself without relying on an external library, even though there are libc implementations which are more appropriate for embedded/firmware use. In this case, we're grabbing a small bit of code out of glibc and transplanting it by itself into u-boot. There's no special linking involved, just porting over a single function.

Now we are getting into your 'Wrap small helper functions from libgcc to avoid an ABI mismatch' patch...

#10 lore raw

PATCH v2 1/1 x86: Import the glibc memset implementation

Quoted 5 lines · On Tuesday 08 November 2011 21:53:04 Graeme Russ wrote:

Oops, I meant U-Boot implements a subset of (g)libc. I think I understand why - U-Boot needs specific implementations of stdio (printf() and friends) Because we need to implement _some_ of glibc, we need to implement all (or at least all the functions we use) otherwise we will get all sorts of symbol conflicts in the linker

glibc assumes an OS. there's no way around that. u-boot doesn't have an OS, so it has never linked against glibc (ignoring the "sandbox" arch). libgcc is a different issue: it rarely (if ever) relies on an OS. it's largely pure math functions, so u-boot has historically used it since things "just worked".

the reason for arm getting the private libgcc is because (1) newer versions of libgcc started to rely on the OS (see the whole libgcc-likes-to-call-abort()) and (2) the ABI hell that ARM is currently in -- the hard/soft/vfp/etc..., and little vs big endian. it ends up being pretty hard to get a single sane arm toolchain to build all the u-boot arm targets.

for most arches though, these libgcc issues don't come up, so they don't support the private libgcc option. i think wrapping the few bad funcs like coreboot does (i.e. Gabe's patch here) makes sense.
-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/ddbb269d/attachment.pgp

author
#11 lore raw

PATCH v2 1/1 x86: Import the glibc memset implementation

Quoted 27 lines · Mike Frysinger
Quoted 2 lines · On Tuesday 08 November 2011 21:53:04 Graeme Russ wrote:

Oops, I meant U-Boot implements a subset of (g)libc. I think I understand why - U-Boot needs specific implementations of stdio (printf() and

friends)

Quoted 1 line

Because we need to implement _some_ of glibc, we need to implement all

(or

Quoted 2 lines

at least all the functions we use) otherwise we will get all sorts of symbol conflicts in the linker

glibc assumes an OS. there's no way around that. u-boot doesn't have an OS,
so it has never linked against glibc (ignoring the "sandbox" arch).
libgcc is
a different issue: it rarely (if ever) relies on an OS. it's largely pure math
functions, so u-boot has historically used it since things "just worked".

the reason for arm getting the private libgcc is because (1) newer versions of
libgcc started to rely on the OS (see the whole libgcc-likes-to-call-abort())
and (2) the ABI hell that ARM is currently in -- the hard/soft/vfp/etc..., and
little vs big endian. it ends up being pretty hard to get a single sane arm
toolchain to build all the u-boot arm targets.

Quoted 5 lines

for most arches though, these libgcc issues don't come up, so they don't support the private libgcc option. i think wrapping the few bad funcs like coreboot does (i.e. Gabe's patch here) makes sense.
-mike

I do understand what you (Graeme) are worried about and was initially worried about the same thing myself, but people who have experience with these sorts of things were of the opinion that it isn't a big problem.

I expect the regparm option is there for performance reasons. The i386 ABI says that arguments should go on the stack, but performance is better if they go in registers. X86 processors put a lot of effort into making the stack fast since the ISA is register poor and has a lot of stack-centric features, but apparently it's still not as fast as using registers.

Gabe

#12 lore raw

PATCH v2 1/1 x86: Import the glibc memset implementation

In reply to Mike Frysinger
Quoted 11 lines · Mike Frysinger
Quoted 5 lines · On Tuesday 08 November 2011 21:53:04 Graeme Russ wrote:

Oops, I meant U-Boot implements a subset of (g)libc. I think I understand why - U-Boot needs specific implementations of stdio (printf() and friends) Because we need to implement _some_ of glibc, we need to implement all (or at least all the functions we use) otherwise we will get all sorts of symbol conflicts in the linker

glibc assumes an OS. there's no way around that. u-boot doesn't have an OS, so it has never linked against glibc (ignoring the "sandbox" arch). libgcc is a different issue: it rarely (if ever) relies on an OS. it's largely pure math functions, so u-boot has historically used it since things "just worked".

Plus, libgcc is closely bound to compiler internals.

Quoted 3 lines

the reason for arm getting the private libgcc is because (1) newer versions of libgcc started to rely on the OS (see the whole libgcc-likes-to-call-abort()) and

IMHO it's reasonable for the compiler to want a way to report fatal errors from its runtime -- define it in the linker script as NULL, or point it at some existing exception/error code, if you don't want to spend the few bytes on a stub implementation (though we typically spend a bunch more bytes on exception vectors to handle similar errors...).

The environmental requirements of the compiler (which could come from either libgcc or directly generated code) are documented in section 2.1 of the GCC manual:

Quoted 5 lines

Most of the compiler support routines used by GCC are present in
libgcc', but there are a few exceptions. GCC requires the freestanding environment provide memcpy', memmove', memset' and memcmp'. Finally, if __builtin_trap' is used, and the target does not implement the trap' pattern, then GCC will emit a call to abort'.

The "if __builtin_trap is used" clause should be removed if libgcc uses __builtin_trap() (or otherwise calls abort()), though.

Quoted 3 lines

(2) the ABI hell that ARM is currently in -- the hard/soft/vfp/etc..., and little vs big endian. it ends up being pretty hard to get a single sane arm toolchain to build all the u-boot arm targets.

Last I tried, I couldn't get it to work even with USE_PRIVATE_LIBGCC, because the compiler was generating calls to things that U-Boot didn't implement.

-Scott

#13 lore raw

PATCH v2 1/1 x86: Import the glibc memset implementation

Quoted 3 lines · On Wednesday 09 November 2011 14:12:15 Scott Wood wrote:

Last I tried, I couldn't get it to work even with USE_PRIVATE_LIBGCC, because the compiler was generating calls to things that U-Boot didn't implement.

last i tried, i saw failures only for boards using yaffs as apparently that code triggered some extended math operations that the local libgcc omits. i didn't look further to see if it was a matter of the yaffs code being wrong (i.e. trying to do a 64bit divide rather than using do_div), or libgcc should include that logic. i don't use yaffs, so meh :).
-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/6b66956a/attachment.pgp

author
#14 lore raw

PATCH v2 1/1 x86: Import the glibc memset implementation

Quoted 14 lines · Mike Frysinger
Quoted 3 lines · On Wednesday 09 November 2011 14:12:15 Scott Wood wrote:

Last I tried, I couldn't get it to work even with USE_PRIVATE_LIBGCC, because the compiler was generating calls to things that U-Boot didn't implement.

last i tried, i saw failures only for boards using yaffs as apparently that code triggered some extended math operations that the local libgcc omits.
i
didn't look further to see if it was a matter of the yaffs code being wrong (i.e. trying to do a 64bit divide rather than using do_div), or libgcc should
include that logic. i don't use yaffs, so meh :).
-mike

Quoted 6 lines

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

I'm sure folks realize this, but I thought I would point out that we're actually primarily talking about a different patch. To me it sounded like this one, importing glibc's memset, was good to go, and I wouldn't want it to be held up by the controversy around the other patch, how to deal with libgcc. If there's anything more to say about memset lets say it here, and if there's more to say about libgcc (I have a feeling there is) lets please move that over to the other thread.

Gabe

#15 lore raw

PATCH v2 1/1 x86: Import the glibc memset implementation

Hi Gabe,

Quoted 23 lines · Gabe Black

On Wed, Nov 9, 2011 at 12:47 PM, Mike Frysinger <[email protected] <mailto:[email protected]>> wrote:

On Wednesday 09 November 2011 14:12:15 Scott Wood wrote:
> Last I tried, I couldn't get it to work even with USE_PRIVATE_LIBGCC, > because the compiler was generating calls to things that U-Boot didn't > implement.

last i tried, i saw failures only for boards using yaffs as apparently that code triggered some extended math operations that the local libgcc omits. i
didn't look further to see if it was a matter of the yaffs code being wrong (i.e. trying to do a 64bit divide rather than using do_div), or libgcc should
include that logic. i don't use yaffs, so meh :).
-mike

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de <mailto:[email protected]>
http://lists.denx.de/mailman/listinfo/u-boot

Quoted 7 lines

I'm sure folks realize this, but I thought I would point out that we're actually primarily talking about a different patch. To me it sounded like this one, importing glibc's memset, was good to go, and I wouldn't want it to be held up by the controversy around the other patch, how to deal with libgcc. If there's anything more to say about memset lets say it here, and if there's more to say about libgcc (I have a feeling there is) lets please move that over to the other thread.

I have no issue with this patch - It will look to apply shortly

Regards,

Graeme

#16 lore raw

PATCH v2 1/1 x86: Import the glibc memset implementation

In reply to Graeme Russ

Hi Gabe,

[snip]

Quoted 6 lines

From what I understand, USE_PRIVATE_LIBGCC was introduced because some compilers/linkers have ABI compatibility issue (someone more familiar with the history may be able to correct me) - This sounds _exactly_ what we have here with x86 - x86 has an ABI incompatibility by using reparm(3). I think reparm was used to simplify some asm/C interfaces - maybe these can be re-coded and we can drop regparm and we can drop the wrappers.

My commit 5c161653db3aa585f3e47a650ae177ba9ffb7232 (7 Oct 2010) introduced reparam to reduce code size and increase speed

Quoted 5 lines

Alternatively, I think I would prefer to use USE_PRIVATE_LIBGCC instead of the wrappers as this highlights that U-Boot is _not_ ABI compliant and therefore cannot link to libgcc. I can just see someone in the future using a libgcc function and not implementing a wrapper and doing a lot of head scratching when things wrong...

I think we should ditch reparam and partially revert what the above commit did

Regards,

Graeme

#17 lore raw

PATCH v2 1/1 x86: Import the glibc memset implementation

In reply to Graeme Russ

Dear Graeme Russ,

Quoted 4 lines · In message <CALButCJq9GdNb5ZtaUb7nepQMV7ULPcR01fon2SxcLdRoPzg+g@mail.gmail.com>…

My point is, why does U-Boot implement parts of libgcc so as to not link to libgcc, but uses (by default) glibc - Why not always implement both, or always (by default) link to both?

We do NOT link glibc. IF there should beany such case anywhere, it would be a bug that needs urgent fixing.

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The flow chart is a most thoroughly oversold piece of  program  docu-
mentation.              -- Frederick Brooks, "The Mythical Man Month"
#18 lore raw

PATCH v2 1/1 x86: Import the glibc memset implementation

Hi Wolfgang,

Quoted 10 lines · Wolfgang Denk

Dear Graeme Russ,

Quoted 4 lines · In message <CALButCJq9GdNb5ZtaUb7nepQMV7ULPcR01fon2SxcLdRoPzg+g@mail.gmail.com>…

My point is, why does U-Boot implement parts of libgcc so as to not link to libgcc, but uses (by default) glibc - Why not always implement both, or always (by default) link to both?

We do NOT link glibc. ?IF there should beany such case anywhere, it would be a bug that needs urgent fixing.

I've already corrected my mis-statement on this - We link to libgcc

My issue is that this link can be tenuous and very subtle (the x86 case has been wrong forever but slipped through because the failure scenario has only just been picked up)

I understand the argument for linking to libgcc, but I have several against:
- U-Boot x68 uses regparm which changes the ABI and is, therefore,
inconsistent with libgcc which result in either coding out regparm or implementing wrappers
- Implementing wrappers for in-use functions adds overhead (I'm not
really worried by this at all in reality)
- Implementing wrappers may cover all the 'right now' cases, but the
solution is non-obvious to anyone that, for whatever reason, uses a libgcc function which is not wrapped. There is no linker error, and the failure may be non-obvious
- On an x64 build machine, default 64-bit installs of gcc/binutils
will provide a compiler and linker with command line options to compile 32-bit code (-m32) but this is not sufficient - You also need to install 32-bit libgcc (and x86 needs patches to handle specifying the location of said 32-bit libgcc)

Of all the issues, it is the third that worries me the most. If U-Boot uses an ABI not compatible with a library, then, IMHO, we should not link to that library (wrappers or not) - We are just asking for trouble...

We already have one 'hack' to get around ABI issues - I'm not a fan of adding another.

Regards,

Graeme

#19 lore raw

PATCH 1/1 Import the glibc implementation of x86 memset into u-boot

In reply to Gabe Black

Hi Gabe,

Quoted 10 lines · Gabe Black

The new implementation is about twice as fast as the old.

Signed-off-by Gabe Black <[email protected]>
---
 arch/x86/include/asm/string.h |    2 +-
 arch/x86/lib/Makefile         |    1 +
 arch/x86/lib/string.c         |   87 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/lib/string.c

Can you please rebase against u-boot-x86/master and re-submit

Thanks,

Graeme

author
#20 lore raw

PATCH v3 1/1 x86: Import the glibc memset implementation

The new implementation is about twice as fast as the old.

Signed-off-by Gabe Black <[email protected]>

---
Changes in v2:
Update the commit summary as suggested by Mike Frysinger.

Changes in v3:
Rebase onto the x86 repository.

 arch/x86/include/asm/string.h |    2 +-
 arch/x86/lib/Makefile         |    1 +
 arch/x86/lib/string.c         |   87 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/lib/string.c
arch/x86/include/asm/string.h +1 -1
index 3643a79..3aa6c11 100644
@@ -23,7 +23,7 @@ extern void * memmove(void *, const void *, __kernel_size_t);
 #undef __HAVE_ARCH_MEMCHR
 extern void * memchr(const void *, int, __kernel_size_t);
 
-#undef __HAVE_ARCH_MEMSET
+#define __HAVE_ARCH_MEMSET
 extern void * memset(void *, int, __kernel_size_t);
 
 #undef __HAVE_ARCH_MEMZERO
arch/x86/lib/Makefile +1 -0
index eb5fa10..90f3283 100644
@@ -38,6 +38,7 @@ COBJS-$(CONFIG_SYS_GENERIC_TIMER) += pcat_timer.o
 COBJS-$(CONFIG_PCI) += pci.o
 COBJS-$(CONFIG_PCI) += pci_type1.o
 COBJS-$(CONFIG_SYS_X86_REALMODE)	+= realmode.o
+COBJS-y	+= string.o
 COBJS-$(CONFIG_SYS_X86_ISR_TIMER)	+= timer.o
 COBJS-$(CONFIG_VIDEO)	+= video_bios.o
 COBJS-$(CONFIG_VIDEO)	+= video.o
arch/x86/lib/string.c new file +87 -0

87 lines not on this page · raw message

Signature · 1 line
1.7.3.1
#21 lore raw

PATCH v3 1/1 x86: Import the glibc memset implementation

Hi Gabe,

Sorry about the lateness, but I just noticed a couple of issues..

Can you put the glibc version in the patch summary? I know it is in string.c, but including it in the summary makes it more obvious.

Quoted 4 lines · Gabe Black

The new implementation is about twice as fast as the old.

Signed-off-by Gabe Black <[email protected]>

---

[snip]

arch/x86/lib/string.c line 2

Quoted 8 lines
arch/x86/lib/string.c new file +2 -0
new file mode 100644
index 0000000..1346173
@@ -0,0 +1,87 @@
+/*
+ * Copyright (C) 1991,1992,1993,1997,1998,2003, 2005 Free Software Foundation, Inc.

Line over 80 characters - I can live with this as it is a verbatim import

arch/x86/lib/string.c line 4

Quoted 2 lines
arch/x86/lib/string.c new file +2 -0
+ * This file is part of the GNU C Library.
+ * Copyright (c) 2011 The Chromium OS Authors. All rights reserved.

This, however, causes a problem - There is some conjecture as to whether "All rights reserved" can co-exist with GPL. I believe there is a mandate from Wolfgang to not accept patches with this clause - Do I understand correctly Wolfgang?

Regards,

Graeme

#22 lore raw

PATCH v3 1/1 x86: Import the glibc memset implementation

Quoted 29 lines · Graeme Russ

Hi Gabe,

Sorry about the lateness, but I just noticed a couple of issues..

Can you put the glibc version in the patch summary? I know it is in string.c, but including it in the summary makes it more obvious.

Quoted 4 lines · Gabe Black

The new implementation is about twice as fast as the old.

Signed-off-by Gabe Black <[email protected]>

---

[snip]

arch/x86/lib/string.c line 2

Quoted 8 lines
arch/x86/lib/string.c new file +2 -0
new file mode 100644
index 0000000..1346173
@@ -0,0 +1,87 @@
+/*
+ * Copyright (C) 1991,1992,1993,1997,1998,2003, 2005 Free Software

Foundation, Inc.

Line over 80 characters - I can live with this as it is a verbatim import

arch/x86/lib/string.c line 4

Quoted 2 lines
arch/x86/lib/string.c new file +2 -0
+ * This file is part of the GNU C Library.
+ * Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
Quoted 9 lines

This, however, causes a problem - There is some conjecture as to whether "All rights reserved" can co-exist with GPL. I believe there is a mandate from Wolfgang to not accept patches with this clause - Do I understand correctly Wolfgang?

Regards,

Graeme

For reference, I ran this command

grep -R 'All rights reserved' * | wc -l

in a several months old U-Boot repository and got 238 hits, and ran it in the Linux 2.6.39.3 source and got 3420 hits. There seems to be a lot of precedence for this language in what I assume is mostly GPL-ed code. That said, I'm not a lawyer and I'm personally not very worried about exactly how this is worded beyond my obligations to my employer.

Gabe

#23 lore raw

PATCH v3 1/1 x86: Import the glibc memset implementation

Hi Gabe,

Quoted 43 lines · Gabe Black
Quoted 30 lines · Graeme Russ

Hi Gabe,

Sorry about the lateness, but I just noticed a couple of issues..

Can you put the glibc version in the patch summary? I know it is in string.c, but including it in the summary makes it more obvious.

Quoted 4 lines · Gabe Black

The new implementation is about twice as fast as the old.

Signed-off-by Gabe Black <[email protected]>

---

[snip]

Quoted 9 lines
arch/x86/lib/string.c new file +2 -0
new file mode 100644
index 0000000..1346173
@@ -0,0 +1,87 @@
+/*
+ * Copyright (C) 1991,1992,1993,1997,1998,2003, 2005 Free Software

Foundation, Inc.

Line over 80 characters - I can live with this as it is a verbatim import

Quoted 2 lines

+ * This file is part of the GNU C Library.
+ * Copyright (c) 2011 The Chromium OS Authors. All rights reserved.

Quoted 8 lines

This, however, causes a problem - There is some conjecture as to whether "All rights reserved" can co-exist with GPL. I believe there is a mandate from Wolfgang to not accept patches with this clause - Do I understand correctly Wolfgang?

Regards,

Graeme

Quoted 7 lines

For reference, I ran this command
grep -R 'All rights reserved' * | wc -l
in a several months old U-Boot repository and got 238 hits, and ran it in the Linux 2.6.39.3 source and got 3420 hits. There seems to be a lot of precedence for this language in what I assume is mostly GPL-ed code. That said, I'm not a lawyer and I'm personally not very worried about exactly how this is worded beyond my obligations to my employer.

A number of those hits are files using the BSD licence. Of course there are a lot in files using the GPL as well but as has been said before, bad practices of the past should not be used as an example for the future...

I'm not too concerned by it either - I'm waiting for Wolfgang's response

Regards,

Graeme

#24 lore raw

PATCH v3 1/1 x86: Import the glibc memset implementation

Quoted 62 lines · Graeme Russ

Hi Gabe,

Quoted 3 lines · Gabe Black

On Sun, Nov 13, 2011 at 3:38 AM, Graeme Russ <[email protected]>

Quoted 15 lines · wrote:
Quoted 15 lines

Hi Gabe,

Sorry about the lateness, but I just noticed a couple of issues..

Can you put the glibc version in the patch summary? I know it is in string.c, but including it in the summary makes it more obvious.

Quoted 4 lines · Gabe Black

The new implementation is about twice as fast as the old.

Signed-off-by Gabe Black <[email protected]>

---

[snip]

Quoted 11 lines
Quoted 11 lines
Quoted 9 lines
arch/x86/lib/string.c new file +2 -0
new file mode 100644
index 0000000..1346173
@@ -0,0 +1,87 @@
+/*
+ * Copyright (C) 1991,1992,1993,1997,1998,2003, 2005 Free Software

Foundation, Inc.

Line over 80 characters - I can live with this as it is a verbatim

import

Quoted 6 lines
Quoted 6 lines
Quoted 3 lines

+ * This file is part of the GNU C Library.
+ * Copyright (c) 2011 The Chromium OS Authors. All rights reserved.

This, however, causes a problem - There is some conjecture as to whether "All rights reserved" can co-exist with GPL. I believe there is a

mandate

Quoted 8 lines
Quoted 6 lines

from Wolfgang to not accept patches with this clause - Do I understand correctly Wolfgang?

Regards,

Graeme

Quoted 6 lines

For reference, I ran this command
grep -R 'All rights reserved' * | wc -l
in a several months old U-Boot repository and got 238 hits, and ran it in the Linux 2.6.39.3 source and got 3420 hits. There seems to be a lot of precedence for this language in what I assume is mostly GPL-ed code. That said, I'm not a lawyer and I'm personally not very worried about exactly

how

Quoted 1 line

this is worded beyond my obligations to my employer.

A number of those hits are files using the BSD licence. Of course there are a lot in files using the GPL as well but as has been said before, bad practices of the past should not be used as an example for the future...

Quoted 6 lines

I'm not too concerned by it either - I'm waiting for Wolfgang's response

Regards,

Graeme

I asked, and I've been told it's fine to remove that. I'll send a new version soon.

Gabe

author
#25 lore raw

PATCH v4 1/1 x86: Import the glibc memset implementation

In reply to Graeme Russ

The new implementation is about twice as fast as the old. This is from glibc-2.14, sysdeps/i386/memset.c.

Signed-off-by Gabe Black <[email protected]>

---
Changes in v2:
Update the commit summary as suggested by Mike Frysinger.

Changes in v3:
Rebase onto the x86 repository.

Changes in v4:
Remove "All rights reserved."
Add the glibc version number to the commit message.

 arch/x86/include/asm/string.h |    2 +-
 arch/x86/lib/Makefile         |    1 +
 arch/x86/lib/string.c         |   87 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/lib/string.c
arch/x86/include/asm/string.h +1 -1
index 3643a79..3aa6c11 100644
@@ -23,7 +23,7 @@ extern void * memmove(void *, const void *, __kernel_size_t);
 #undef __HAVE_ARCH_MEMCHR
 extern void * memchr(const void *, int, __kernel_size_t);
 
-#undef __HAVE_ARCH_MEMSET
+#define __HAVE_ARCH_MEMSET
 extern void * memset(void *, int, __kernel_size_t);
 
 #undef __HAVE_ARCH_MEMZERO
arch/x86/lib/Makefile +1 -0
index eb5fa10..90f3283 100644
@@ -38,6 +38,7 @@ COBJS-$(CONFIG_SYS_GENERIC_TIMER) += pcat_timer.o
 COBJS-$(CONFIG_PCI) += pci.o
 COBJS-$(CONFIG_PCI) += pci_type1.o
 COBJS-$(CONFIG_SYS_X86_REALMODE)	+= realmode.o
+COBJS-y	+= string.o
 COBJS-$(CONFIG_SYS_X86_ISR_TIMER)	+= timer.o
 COBJS-$(CONFIG_VIDEO)	+= video_bios.o
 COBJS-$(CONFIG_VIDEO)	+= video.o
arch/x86/lib/string.c new file +87 -0

87 lines not on this page · raw message

Signature · 1 line
1.7.3.1
#26 lore raw

PATCH v4 1/1 x86: Import the glibc memset implementation

Hi Gabe,

Quoted 15 lines · Gabe Black

The new implementation is about twice as fast as the old. This is from glibc-2.14, sysdeps/i386/memset.c.

Signed-off-by Gabe Black <[email protected]>

---
Changes in v2:
Update the commit summary as suggested by Mike Frysinger.

Changes in v3:
Rebase onto the x86 repository.

Changes in v4:
Remove "All rights reserved."
Add the glibc version number to the commit message.

Quoted 5 lines
 arch/x86/include/asm/string.h |    2 +-
 arch/x86/lib/Makefile         |    1 +
 arch/x86/lib/string.c         |   87 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/lib/string.c

Applied to u-boot-x86/master

Thanks,

Graeme

#27 lore raw

PATCH v3 1/1 x86: Import the glibc memset implementation

In reply to Graeme Russ
Quoted 11 lines · On Sunday 13 November 2011 06:38:17 Graeme Russ wrote:
Quoted 7 lines · Gabe Black

The new implementation is about twice as fast as the old.

arch/x86/lib/string.c new file +2 -0
@@ -0,0 +1,87 @@
+/*
+ * Copyright (C) 1991,1992,1993,1997,1998,2003, 2005 Free Software

Foundation, Inc.

Line over 80 characters - I can live with this as it is a verbatim import

Quoted 7 lines
Quoted 2 lines

+ * This file is part of the GNU C Library.
+ * Copyright (c) 2011 The Chromium OS Authors. All rights reserved.

This, however, causes a problem - There is some conjecture as to whether "All rights reserved" can co-exist with GPL. I believe there is a mandate from Wolfgang to not accept patches with this clause - Do I understand correctly Wolfgang?

in reality, "All rights reserved" is meaningless. i don't think there are any GPL compatibility issues.

but, since it is meaningless, it should get culled.

http://en.wikipedia.org/wiki/All_Rights_Reserved
-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/20111114/f6e9e064/attachment.pgp