Hi Stefan, On Tue, Nov 28, 2017 at 12:34 AM, Stefan Roese <s...@denx.de> wrote: > Hi Bin, > > > On 27.11.2017 10:46, Bin Meng wrote: >> >> On Fri, Nov 24, 2017 at 5:29 PM, Stefan Roese <s...@denx.de> wrote: >>> >>> Hi Bin, >>> >>> >>> On 24.11.2017 09:29, Bin Meng wrote: >>>> >>>> >>>> On Mon, Nov 20, 2017 at 6:43 PM, Stefan Roese <s...@denx.de> wrote: >>>>> >>>>> >>>>> Hi Bin, >>>>> >>>>> >>>>> On 20.11.2017 08:24, Bin Meng wrote: >>>>>> >>>>>> >>>>>> >>>>>> On Fri, Nov 17, 2017 at 2:02 PM, Stefan Roese <s...@denx.de> wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> This patch removes the inclusion of the libgcc math functions and >>>>>>> replaces them by functions coded in C, taken from the coreboot >>>>>>> project. This makes U-Boot building more independent from the >>>>>>> toolchain >>>>>>> installed / available on the build system. >>>>>>> >>>>>>> The code taken from coreboot is authored from Vadim Bendebury >>>>>>> <vben...@chromium.org> on 2014-11-28 and committed with commit >>>>>>> ID e63990ef [libpayload: provide basic 64bit division implementation] >>>>>>> (coreboot git repository located here [1]). >>>>>>> >>>>>>> I modified the code so that its checkpatch clean without any >>>>>>> functional changes. >>>>>>> >>>>>>> [1] git://github.com/coreboot/coreboot.git >>>>>>> >>>>>>> Signed-off-by: Stefan Roese <s...@denx.de> >>>>>>> Cc: Simon Glass <s...@chromium.org> >>>>>>> Cc: Bin Meng <bmeng...@gmail.com> >>>>>>> --- >>>>>>> v2: >>>>>>> - Added coreboot git repository link to commit message >>>>>>> >>>>>>> arch/x86/config.mk | 3 -- >>>>>>> arch/x86/lib/Makefile | 2 +- >>>>>>> arch/x86/lib/div64.c | 113 >>>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>>>> arch/x86/lib/gcc.c | 29 ------------- >>>>>>> 4 files changed, 114 insertions(+), 33 deletions(-) >>>>>>> create mode 100644 arch/x86/lib/div64.c >>>>>>> delete mode 100644 arch/x86/lib/gcc.c >>>>>>> >>>>>>> diff --git a/arch/x86/config.mk b/arch/x86/config.mk >>>>>>> index 8835dcf36f..472ada5490 100644 >>>>>>> --- a/arch/x86/config.mk >>>>>>> +++ b/arch/x86/config.mk >>>>>>> @@ -34,9 +34,6 @@ PLATFORM_RELFLAGS += -ffunction-sections >>>>>>> -fvisibility=hidden >>>>>>> PLATFORM_LDFLAGS += -Bsymbolic -Bsymbolic-functions >>>>>>> PLATFORM_LDFLAGS += -m $(if $(IS_32BIT),elf_i386,elf_x86_64) >>>>>>> >>>>>>> -LDFLAGS_FINAL += --wrap=__divdi3 --wrap=__udivdi3 >>>>>>> -LDFLAGS_FINAL += --wrap=__moddi3 --wrap=__umoddi3 >>>>>>> - >>>>>>> # This is used in the top-level Makefile which does not include >>>>>>> # PLATFORM_LDFLAGS >>>>>>> LDFLAGS_EFI_PAYLOAD := -Bsymbolic -Bsymbolic-functions -shared >>>>>>> --no-undefined >>>>>>> diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile >>>>>>> index fe00d7573f..d9b23f5cc4 100644 >>>>>>> --- a/arch/x86/lib/Makefile >>>>>>> +++ b/arch/x86/lib/Makefile >>>>>>> @@ -18,7 +18,7 @@ obj-$(CONFIG_SEABIOS) += coreboot_table.o >>>>>>> obj-y += early_cmos.o >>>>>>> obj-$(CONFIG_EFI) += efi/ >>>>>>> obj-y += e820.o >>>>>>> -obj-y += gcc.o >>>>>>> +obj-y += div64.o >>>>>>> obj-y += init_helpers.o >>>>>>> obj-y += interrupts.o >>>>>>> obj-y += lpc-uclass.o >>>>>>> diff --git a/arch/x86/lib/div64.c b/arch/x86/lib/div64.c >>>>>>> new file mode 100644 >>>>>>> index 0000000000..4efed74037 >>>>>>> --- /dev/null >>>>>>> +++ b/arch/x86/lib/div64.c >>>>>>> @@ -0,0 +1,113 @@ >>>>>>> +/* >>>>>>> + * This file is copied from the coreboot repository as part of >>>>>>> + * the libpayload project: >>>>>>> + * >>>>>>> + * Copyright 2014 Google Inc. >>>>>>> + * >>>>>>> + * SPDX-License-Identifier: BSD-3-Clause >>>>>>> + */ >>>>>>> + >>>>>>> +#include <common.h> >>>>>>> + >>>>>>> +union overlay64 { >>>>>>> + u64 longw; >>>>>>> + struct { >>>>>>> + u32 lower; >>>>>>> + u32 higher; >>>>>>> + } words; >>>>>>> +}; >>>>>>> + >>>>>>> +u64 __ashldi3(u64 num, unsigned int shift) >>>>>>> +{ >>>>>>> + union overlay64 output; >>>>>>> + >>>>>>> + output.longw = num; >>>>>>> + if (shift >= 32) { >>>>>>> + output.words.higher = output.words.lower << (shift - >>>>>>> 32); >>>>>>> + output.words.lower = 0; >>>>>>> + } else { >>>>>>> + if (!shift) >>>>>>> + return num; >>>>>>> + output.words.higher = (output.words.higher << shift) >>>>>>> | >>>>>>> + (output.words.lower >> (32 - shift)); >>>>>>> + output.words.lower = output.words.lower << shift; >>>>>>> + } >>>>>>> + return output.longw; >>>>>>> +} >>>>>>> + >>>>>>> +u64 __lshrdi3(u64 num, unsigned int shift) >>>>>>> +{ >>>>>>> + union overlay64 output; >>>>>>> + >>>>>>> + output.longw = num; >>>>>>> + if (shift >= 32) { >>>>>>> + output.words.lower = output.words.higher >> (shift - >>>>>>> 32); >>>>>>> + output.words.higher = 0; >>>>>>> + } else { >>>>>>> + if (!shift) >>>>>>> + return num; >>>>>>> + output.words.lower = output.words.lower >> shift | >>>>>>> + (output.words.higher << (32 - shift)); >>>>>>> + output.words.higher = output.words.higher >> shift; >>>>>>> + } >>>>>>> + return output.longw; >>>>>>> +} >>>>>>> + >>>>>>> +#define MAX_32BIT_UINT ((((u64)1) << 32) - 1) >>>>>>> + >>>>>>> +static u64 _64bit_divide(u64 dividend, u64 divider, u64 *rem_p) >>>>>>> +{ >>>>>>> + u64 result = 0; >>>>>>> + >>>>>>> + /* >>>>>>> + * If divider is zero - let the rest of the system care about >>>>>>> the >>>>>>> + * exception. >>>>>>> + */ >>>>>>> + if (!divider) >>>>>>> + return 1 / (u32)divider; >>>>>>> + >>>>>>> + /* As an optimization, let's not use 64 bit division unless >>>>>>> we >>>>>>> must. */ >>>>>>> + if (dividend <= MAX_32BIT_UINT) { >>>>>>> + if (divider > MAX_32BIT_UINT) { >>>>>>> + result = 0; >>>>>>> + if (rem_p) >>>>>>> + *rem_p = divider; >>>>>>> + } else { >>>>>>> + result = (u32)dividend / (u32)divider; >>>>>>> + if (rem_p) >>>>>>> + *rem_p = (u32)dividend % >>>>>>> (u32)divider; >>>>>>> + } >>>>>>> + return result; >>>>>>> + } >>>>>>> + >>>>>>> + while (divider <= dividend) { >>>>>>> + u64 locald = divider; >>>>>>> + u64 limit = __lshrdi3(dividend, 1); >>>>>>> + int shifts = 0; >>>>>>> + >>>>>>> + while (locald <= limit) { >>>>>>> + shifts++; >>>>>>> + locald = locald + locald; >>>>>>> + } >>>>>>> + result |= __ashldi3(1, shifts); >>>>>>> + dividend -= locald; >>>>>>> + } >>>>>>> + >>>>>>> + if (rem_p) >>>>>>> + *rem_p = dividend; >>>>>>> + >>>>>>> + return result; >>>>>>> +} >>>>>>> + >>>>>>> +u64 __udivdi3(u64 num, u64 den) >>>>>>> +{ >>>>>>> + return _64bit_divide(num, den, NULL); >>>>>>> +} >>>>>>> + >>>>>>> +u64 __umoddi3(u64 num, u64 den) >>>>>>> +{ >>>>>>> + u64 v = 0; >>>>>>> + >>>>>>> + _64bit_divide(num, den, &v); >>>>>>> + return v; >>>>>>> +} >>>>>>> diff --git a/arch/x86/lib/gcc.c b/arch/x86/lib/gcc.c >>>>>>> deleted file mode 100644 >>>>>>> index 3c70d790d4..0000000000 >>>>>>> --- a/arch/x86/lib/gcc.c >>>>>>> +++ /dev/null >>>>>>> @@ -1,29 +0,0 @@ >>>>>>> -/* >>>>>>> - * This file is part of the coreboot project. >>>>>>> - * >>>>>>> - * Copyright (C) 2009 coresystems GmbH >>>>>>> - * >>>>>>> - * SPDX-License-Identifier: GPL-2.0 >>>>>>> - */ >>>>>>> - >>>>>>> -#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 __attribute__((no_instrument_function)) \ >>>>>>> - __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 >>>>>>> -- >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> I don't see __divdi3 and __moddi3 in the new implementation. Are they >>>>>> not needed? >>>>> >>>>> >>>>> >>>>> >>>>> Yes, at least I couldn't find any problems compiling U-Boot for x86 >>>>> with >>>>> this patch. >>>>> >>>>> BTW: The code size also did shrink a bit with this patch. >>>> >>>> >>>> >>>> But this is needed in fact. If you write some codes doing signed >>>> 64-bit integer division, you will get link errors. >>>> >>>> undefined reference to `__divdi3' >>> >>> >>> >>> You wrote some additional code to test this, that is currently not >>> in mainline, right? Writing code with divisions should be done very >>> carefully from my point of view. And as far as I know, using do_div() >>> and the functions from lib/div64 is preferred here. >>> >> >> I wrote some codes soemthing like: >> >> s64 a, b, c; >> c = a/b; >> >> The compiler will generate codes to call __divdi3. This works before your >> patch. > > > Yes, I understand this. But right now, we don't have any code > generating this error. And if this will happen at some time, I would > prefer to investigate this code sequence introducing this division, to > use the division functions / macros available in U-Boot (as mentioned > above) and in the kernel instead. > > Here a short explanation, why this new version is preferred to the > currently available functions pulled from libgcc: We are fixing an > "ugly" Yocto build problem with this patch, related to 32bit binaries > with 64bit toolchains (multilib) building by not relying on anything > from libgcc. Please see this thread for some more details: > > https://www.mail-archive.com/yocto@yoctoproject.org/msg36721.html > > I hope this helps a bit to understand the motivation behind this patch. >
Thanks for the background. I am not familiar with yocto project, but is the build broken due to the gcc on the build host does not ship with multilib? I am using a 64-bit host with gcc multilib, and there is no such build error observed. Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot