| Message ID | 1298479238-22114-1-git-send-email-Kyle.D.Moffett@boeing.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
Dear Kyle Moffett, In message <1298479238-22114-1-git-send-email-Kyle.D.Moffett@boeing.com> you wrote: > Standard Debian powerpc and powerpcspe systems only include hard-float > libgcc in their native compilers, which causes scary build warnings when > building U-Boot. > > Debian and other PowerPC-supporting distributions used to provide libgcc > and other libraries in a "nof" (soft-float) form in the "multilib" > packages. As they were completely unused by the distribution and > therefore tended to be very buggy it was decided to save some time on > the part of the maintainers and build-servers by removing them. > > Admittedly, right now the linker warnings do not indicate any problems, > as the included routines do not use any floating point at all. > > The concern is that if floating-point code were ever added it might > cause hard-float code to be unexpectedly included in U-Boot without > generating a hard error. This would cause unexplained crashes or > indeterminate results at runtime. > > The easiest way to resolve this is to borrow the routines that U-Boot > needs from the Linux kernel, which has the same issue. > > Specifically, the routines are: _ashldi3(), _ashrdi3(), and _lshrdi3(). Sorry, but I cannot follow your logic. First, we do not use floating point in U-Boot. We don't. Period. [The only exception being well-designed and hand-crafted assemby code where it is unavoidable - for example in the POST code to test the FPU, or for certain atomic 64 bit stores]. So FP support should never be a reason for such a change. What confuses me completely is why you then add some shift functions, which are completely unrelated to FP operations. [I think I know what you mean, but your explanation is confusing and should be fixed.] Best regards, Wolfgang Denk
On Apr 13, 2011, at 16:57, Wolfgang Denk wrote: > In message <1298479238-22114-1-git-send-email-Kyle.D.Moffett@boeing.com> you wrote: >> Standard Debian powerpc and powerpcspe systems only include hard-float >> libgcc in their native compilers, which causes scary build warnings when >> building U-Boot. >> >> Debian and other PowerPC-supporting distributions used to provide libgcc >> and other libraries in a "nof" (soft-float) form in the "multilib" >> packages. As they were completely unused by the distribution and >> therefore tended to be very buggy it was decided to save some time on >> the part of the maintainers and build-servers by removing them. >> >> Admittedly, right now the linker warnings do not indicate any problems, >> as the included routines do not use any floating point at all. >> >> The concern is that if floating-point code were ever added it might >> cause hard-float code to be unexpectedly included in U-Boot without >> generating a hard error. This would cause unexplained crashes or >> indeterminate results at runtime. >> >> The easiest way to resolve this is to borrow the routines that U-Boot >> needs from the Linux kernel, which has the same issue. >> >> Specifically, the routines are: _ashldi3(), _ashrdi3(), and _lshrdi3(). > > Sorry, but I cannot follow your logic. > > First, we do not use floating point in U-Boot. We don't. Period. > [The only exception being well-designed and hand-crafted assemby code > where it is unavoidable - for example in the POST code to test the > FPU, or for certain atomic 64 bit stores]. > > So FP support should never be a reason for such a change. The concern is not with floating point being used in U-Boot, but with the internal implementation of those libgcc functions. Specifically, I could very well imagine that functions such as the full 64-bit divide (_udivdi3) might internally be optimized by using specific floating point operations on PowerPC. On PowerPC with SPE extensions, the native libgcc might use the full 64-bit extensions to the regular integer registers to perform such operations. > What confuses me completely is why you then add some shift functions, > which are completely unrelated to FP operations. Since that CPU state is not necessarily set up in U-Boot, we should not use functions from a libgcc which is allowed to use those ops. Since a native PowerPC U-Boot built on a PowerPC Debian system does not have a soft-float libgcc to build against, this patch provides a minimal "libgcc" with the few libgcc functions that U-Boot seems to need (the various 64-bit shifts). This is similar to the minimal "libgcc" provided by the ARM arch from arch/arm/lib/_ash[lr]di3.S and arch/arm/lib/_divsi3.S, and is almost identical to what the Linux kernel uses on PowerPC. This is only enabled if USE_PRIVATE_LIBGCC=1 (the same config variable as the ARM private libgcc) to avoid breaking other ports. I will update the patch notes. Cheers, Kyle Moffett
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile index 724d8ee..cdd62a2 100644 --- a/arch/powerpc/lib/Makefile +++ b/arch/powerpc/lib/Makefile @@ -23,6 +23,19 @@ include $(TOPDIR)/config.mk +## Build a couple of necessary functions into a private libgcc +LIBGCC = $(obj)libgcc.o +GLSOBJS += _ashldi3.o +GLSOBJS += _ashrdi3.o +GLSOBJS += _lshrdi3.o +LGOBJS := $(addprefix $(obj),$(GLSOBJS)) \ + $(addprefix $(obj),$(GLCOBJS)) + +## But only build it if the user asked for it +ifdef USE_PRIVATE_LIBGCC +TARGETS += $(LIBGCC) +endif + LIB = $(obj)lib$(ARCH).o SOBJS-y += ppccache.o @@ -53,9 +66,14 @@ endif COBJS += $(sort $(COBJS-y)) -SRCS := $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c) +SRCS := $(GLSOBJS:.o=.S) $(GLCOBJS:.o=.c) \ + $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c) OBJS := $(addprefix $(obj),$(SOBJS-y) $(COBJS-y)) +TARGETS += $(LIB) + +all: $(TARGETS) + $(LIB): $(obj).depend $(OBJS) @if ! $(CROSS_COMPILE)readelf -S $(OBJS) | grep -q '\.fixup.*PROGBITS';\ then \ @@ -65,6 +83,9 @@ $(LIB): $(obj).depend $(OBJS) fi; $(call cmd_link_o_target, $(OBJS)) +$(LIBGCC): $(obj).depend $(LGOBJS) + $(call cmd_link_o_target, $(LGOBJS)) + ######################################################################### # defines $(obj).depend target diff --git a/arch/powerpc/lib/_ashldi3.S b/arch/powerpc/lib/_ashldi3.S new file mode 100644 index 0000000..e452f56 --- /dev/null +++ b/arch/powerpc/lib/_ashldi3.S @@ -0,0 +1,45 @@ +/* + * This code was copied from arch/powerpc/kernel/misc_32.S in the Linux + * kernel sources (commit 85e2efbb1db9a18d218006706d6e4fbeb0216213, also + * known as 2.6.38-rc5). The source file copyrights are as follows: + * + * (C) Copyright 1995-1996 Gary Thomas (gdt@linuxppc.org) + * + * Largely rewritten by Cort Dougan (cort@cs.nmt.edu) + * and Paul Mackerras. + * + * 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. + */ + +#include <ppc_asm.tmpl> +#include <ppc_defs.h> +#include <config.h> + +/* + * Extended precision shifts. + * + * Updated to be valid for shift counts from 0 to 63 inclusive. + * -- Gabriel + * + * R3/R4 has 64 bit value + * R5 has shift count + * result in R3/R4 + * + * ashrdi3: arithmetic right shift (sign propagation) + * lshrdi3: logical right shift + * ashldi3: left shift + */ + .globl __ashldi3 +__ashldi3: + subfic r6,r5,32 + slw r3,r3,r5 # MSW = count > 31 ? 0 : MSW << count + addi r7,r5,32 # could be xori, or addi with -32 + srw r6,r4,r6 # t1 = count > 31 ? 0 : LSW >> (32-count) + slw r7,r4,r7 # t2 = count < 32 ? 0 : LSW << (count-32) + or r3,r3,r6 # MSW |= t1 + slw r4,r4,r5 # LSW = LSW << count + or r3,r3,r7 # MSW |= t2 + blr diff --git a/arch/powerpc/lib/_ashrdi3.S b/arch/powerpc/lib/_ashrdi3.S new file mode 100644 index 0000000..f28ab49 --- /dev/null +++ b/arch/powerpc/lib/_ashrdi3.S @@ -0,0 +1,47 @@ +/* + * This code was copied from arch/powerpc/kernel/misc_32.S in the Linux + * kernel sources (commit 85e2efbb1db9a18d218006706d6e4fbeb0216213, also + * known as 2.6.38-rc5). The source file copyrights are as follows: + * + * (C) Copyright 1995-1996 Gary Thomas (gdt@linuxppc.org) + * + * Largely rewritten by Cort Dougan (cort@cs.nmt.edu) + * and Paul Mackerras. + * + * 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. + */ + +#include <ppc_asm.tmpl> +#include <ppc_defs.h> +#include <config.h> + +/* + * Extended precision shifts. + * + * Updated to be valid for shift counts from 0 to 63 inclusive. + * -- Gabriel + * + * R3/R4 has 64 bit value + * R5 has shift count + * result in R3/R4 + * + * ashrdi3: arithmetic right shift (sign propagation) + * lshrdi3: logical right shift + * ashldi3: left shift + */ + .globl __ashrdi3 +__ashrdi3: + subfic r6,r5,32 + srw r4,r4,r5 # LSW = count > 31 ? 0 : LSW >> count + addi r7,r5,32 # could be xori, or addi with -32 + slw r6,r3,r6 # t1 = count > 31 ? 0 : MSW << (32-count) + rlwinm r8,r7,0,32 # t3 = (count < 32) ? 32 : 0 + sraw r7,r3,r7 # t2 = MSW >> (count-32) + or r4,r4,r6 # LSW |= t1 + slw r7,r7,r8 # t2 = (count < 32) ? 0 : t2 + sraw r3,r3,r5 # MSW = MSW >> count + or r4,r4,r7 # LSW |= t2 + blr diff --git a/arch/powerpc/lib/_lshrdi3.S b/arch/powerpc/lib/_lshrdi3.S new file mode 100644 index 0000000..c1bbe7b --- /dev/null +++ b/arch/powerpc/lib/_lshrdi3.S @@ -0,0 +1,45 @@ +/* + * This code was copied from arch/powerpc/kernel/misc_32.S in the Linux + * kernel sources (commit 85e2efbb1db9a18d218006706d6e4fbeb0216213, also + * known as 2.6.38-rc5). The source file copyrights are as follows: + * + * (C) Copyright 1995-1996 Gary Thomas (gdt@linuxppc.org) + * + * Largely rewritten by Cort Dougan (cort@cs.nmt.edu) + * and Paul Mackerras. + * + * 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. + */ + +#include <ppc_asm.tmpl> +#include <ppc_defs.h> +#include <config.h> + +/* + * Extended precision shifts. + * + * Updated to be valid for shift counts from 0 to 63 inclusive. + * -- Gabriel + * + * R3/R4 has 64 bit value + * R5 has shift count + * result in R3/R4 + * + * ashrdi3: arithmetic right shift (sign propagation) + * lshrdi3: logical right shift + * ashldi3: left shift + */ + .globl __lshrdi3 +__lshrdi3: + subfic r6,r5,32 + srw r4,r4,r5 # LSW = count > 31 ? 0 : LSW >> count + addi r7,r5,32 # could be xori, or addi with -32 + slw r6,r3,r6 # t1 = count > 31 ? 0 : MSW << (32-count) + srw r7,r3,r7 # t2 = count < 32 ? 0 : MSW >> (count-32) + or r4,r4,r6 # LSW |= t1 + srw r3,r3,r5 # MSW = MSW >> count + or r4,r4,r7 # LSW |= t2 + blr
Standard Debian powerpc and powerpcspe systems only include hard-float libgcc in their native compilers, which causes scary build warnings when building U-Boot. Debian and other PowerPC-supporting distributions used to provide libgcc and other libraries in a "nof" (soft-float) form in the "multilib" packages. As they were completely unused by the distribution and therefore tended to be very buggy it was decided to save some time on the part of the maintainers and build-servers by removing them. Admittedly, right now the linker warnings do not indicate any problems, as the included routines do not use any floating point at all. The concern is that if floating-point code were ever added it might cause hard-float code to be unexpectedly included in U-Boot without generating a hard error. This would cause unexplained crashes or indeterminate results at runtime. The easiest way to resolve this is to borrow the routines that U-Boot needs from the Linux kernel, which has the same issue. Specifically, the routines are: _ashldi3(), _ashrdi3(), and _lshrdi3(). They were borrowed from arch/powerpc/kernel/misc_32.S as of v2.6.38-rc5, commit 85e2efbb1db9a18d218006706d6e4fbeb0216213, and are GPLv2+. The Makefile framework was copied from the U-Boot ARM port. Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com> Cc: Wolfgang Denk <wd@denx.de> Cc: Kim Phillips <kim.phillips@freescale.com> Cc: Andy Fleming <afleming@gmail.com> Cc: Kumar Gala <kumar.gala@freescale.com> Cc: Stefan Roese <sr@denx.de> --- Changelog: v2: Resubmitted separately from the other HWW-1U-1A patches arch/powerpc/lib/Makefile | 23 ++++++++++++++++++++- arch/powerpc/lib/_ashldi3.S | 45 +++++++++++++++++++++++++++++++++++++++++ arch/powerpc/lib/_ashrdi3.S | 47 +++++++++++++++++++++++++++++++++++++++++++ arch/powerpc/lib/_lshrdi3.S | 45 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 159 insertions(+), 1 deletions(-) create mode 100644 arch/powerpc/lib/_ashldi3.S create mode 100644 arch/powerpc/lib/_ashrdi3.S create mode 100644 arch/powerpc/lib/_lshrdi3.S