| Message ID | 1320792058-23566-1-git-send-email-gabeblack@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Graeme Russ |
| Headers | show |
Hi Gabe, On Wed, Nov 9, 2011 at 9:40 AM, Gabe Black <gabeblack@chromium.org> wrote: > 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
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 On Tue, Nov 8, 2011 at 4:45 PM, Graeme Russ <graeme.russ@gmail.com> wrote: > Hi Gabe, > > > On Wed, Nov 9, 2011 at 9:40 AM, Gabe Black <gabeblack@chromium.org> wrote: > > 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 >
Hi Gabe, first up - Please stop top-posting On Wed, Nov 9, 2011 at 11:59 AM, Gabe Black <gabeblack@chromium.org> wrote: > 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 > On Tue, Nov 8, 2011 at 4:45 PM, Graeme Russ <graeme.russ@gmail.com> wrote: >> >> Hi Gabe, >> >> >> On Wed, Nov 9, 2011 at 9:40 AM, Gabe Black <gabeblack@chromium.org> wrote: >> > 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 > >
On Tue, Nov 8, 2011 at 5:10 PM, Graeme Russ <graeme.russ@gmail.com> wrote: > Hi Gabe, > > first up - Please stop top-posting > > On Wed, Nov 9, 2011 at 11:59 AM, Gabe Black <gabeblack@chromium.org> > wrote: > > 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 > > 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
On Wed, Nov 9, 2011 at 12:15 PM, Gabe Black <gabeblack@google.com> wrote: > > > On Tue, Nov 8, 2011 at 5:10 PM, Graeme Russ <graeme.russ@gmail.com> wrote: >> >> Hi Gabe, >> >> first up - Please stop top-posting >> >> On Wed, Nov 9, 2011 at 11:59 AM, Gabe Black <gabeblack@chromium.org> >> wrote: >> > 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 >> > > 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? > 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? > linking to the generic implementation and providing a specialized one, the > performance improves two fold. Those are the two choices. Regards, Graeme
On Tue, Nov 8, 2011 at 5:25 PM, Graeme Russ <graeme.russ@gmail.com> wrote: > On Wed, Nov 9, 2011 at 12:15 PM, Gabe Black <gabeblack@google.com> wrote: > > > > > > On Tue, Nov 8, 2011 at 5:10 PM, Graeme Russ <graeme.russ@gmail.com> > wrote: > >> > >> Hi Gabe, > >> > >> first up - Please stop top-posting > >> > >> On Wed, Nov 9, 2011 at 11:59 AM, Gabe Black <gabeblack@chromium.org> > >> wrote: > >> > 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 > >> > > > > 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. > > > 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? > 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. > > > linking to the generic implementation and providing a specialized one, > the > > performance improves two fold. Those are the two choices. > > Regards, > > Graeme >
On Wed, Nov 9, 2011 at 12:52 PM, Gabe Black <gabeblack@chromium.org> wrote: > > > On Tue, Nov 8, 2011 at 5:25 PM, Graeme Russ <graeme.russ@gmail.com> wrote: >> >> On Wed, Nov 9, 2011 at 12:15 PM, Gabe Black <gabeblack@google.com> wrote: >> > >> > >> > On Tue, Nov 8, 2011 at 5:10 PM, Graeme Russ <graeme.russ@gmail.com> >> > wrote: >> >> >> >> Hi Gabe, >> >> >> >> first up - Please stop top-posting >> >> >> >> On Wed, Nov 9, 2011 at 11:59 AM, Gabe Black <gabeblack@chromium.org> >> >> wrote: >> >> > 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 >> >> >> > >> > 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. > >> >> > 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 > 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 > 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... 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. 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... Regards, Graeme
On Tue, Nov 8, 2011 at 8:55 PM, Mike Frysinger <vapier@gentoo.org> wrote: > 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 > 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
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
Dear Graeme Russ, In message <CALButCJq9GdNb5ZtaUb7nepQMV7ULPcR01fon2SxcLdRoPzg+g@mail.gmail.com> you wrote: > > 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
Hi Wolfgang, On Wed, Nov 9, 2011 at 4:36 PM, Wolfgang Denk <wd@denx.de> wrote: > Dear Graeme Russ, > > In message <CALButCJq9GdNb5ZtaUb7nepQMV7ULPcR01fon2SxcLdRoPzg+g@mail.gmail.com> you wrote: >> >> 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
Hi Gabe, [snip] > 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 > 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
On 11/08/2011 10:55 PM, Mike Frysinger wrote: > 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. > 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: > 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. > (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
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
On Wed, Nov 9, 2011 at 12:47 PM, Mike Frysinger <vapier@gentoo.org> 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@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
Hi Gabe, On 12/11/11 17:19, Gabe Black wrote: > On Wed, Nov 9, 2011 at 12:47 PM, Mike Frysinger <vapier@gentoo.org > <mailto:vapier@gentoo.org>> 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@lists.denx.de <mailto:U-Boot@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. I have no issue with this patch - It will look to apply shortly Regards, Graeme
diff --git a/arch/x86/include/asm/string.h b/arch/x86/include/asm/string.h index 3643a79..3aa6c11 100644 --- a/arch/x86/include/asm/string.h +++ b/arch/x86/include/asm/string.h @@ -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 diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile index 71e94f7..d9cabdd 100644 --- a/arch/x86/lib/Makefile +++ b/arch/x86/lib/Makefile @@ -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 diff --git a/arch/x86/lib/string.c b/arch/x86/lib/string.c new file mode 100644 index 0000000..1346173 --- /dev/null +++ b/arch/x86/lib/string.c @@ -0,0 +1,87 @@ +/* + * Copyright (C) 1991,1992,1993,1997,1998,2003, 2005 Free Software Foundation, Inc. + * This file is part of the GNU C Library. + * Copyright (c) 2011 The Chromium OS Authors. All rights reserved. + * + * See file CREDITS for list of people who contributed to this + * project. + * + * 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; either version 2 of + * the License, or (at your option) any later version. + * + * 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., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +/* From glibc-2.14, sysdeps/i386/memset.c */ + +#include <compiler.h> +#include <asm/string.h> +#include <linux/types.h> + +typedef uint32_t op_t; + +void *memset(void *dstpp, int c, size_t len) +{ + int d0; + unsigned long int dstp = (unsigned long int) dstpp; + + /* This explicit register allocation improves code very much indeed. */ + register op_t x asm("ax"); + + x = (unsigned char) c; + + /* Clear the direction flag, so filling will move forward. */ + asm volatile("cld"); + + /* This threshold value is optimal. */ + if (len >= 12) { + /* Fill X with four copies of the char we want to fill with. */ + x |= (x << 8); + x |= (x << 16); + + /* Adjust LEN for the bytes handled in the first loop. */ + len -= (-dstp) % sizeof(op_t); + + /* + * There are at least some bytes to set. No need to test for + * LEN == 0 in this alignment loop. + */ + + /* Fill bytes until DSTP is aligned on a longword boundary. */ + asm volatile( + "rep\n" + "stosb" /* %0, %2, %3 */ : + "=D" (dstp), "=c" (d0) : + "0" (dstp), "1" ((-dstp) % sizeof(op_t)), "a" (x) : + "memory"); + + /* Fill longwords. */ + asm volatile( + "rep\n" + "stosl" /* %0, %2, %3 */ : + "=D" (dstp), "=c" (d0) : + "0" (dstp), "1" (len / sizeof(op_t)), "a" (x) : + "memory"); + len %= sizeof(op_t); + } + + /* Write the last few bytes. */ + asm volatile( + "rep\n" + "stosb" /* %0, %2, %3 */ : + "=D" (dstp), "=c" (d0) : + "0" (dstp), "1" (len), "a" (x) : + "memory"); + + return dstpp; +}
The new implementation is about twice as fast as the old. Signed-off-by: Gabe Black <gabeblack@chromium.org> --- 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