[U-Boot] [PATCH v2] x86: lib: Implement standalone __udivdi3 etc instead of libgcc ones
Bin Meng
bmeng.cn at gmail.com
Wed Nov 29 08:30:11 UTC 2017
Hi Stefan,
On Tue, Nov 28, 2017 at 12:34 AM, Stefan Roese <sr at 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 <sr at 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 <sr at 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 <sr at 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
>>>>>>> <vbendeb at 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 <sr at denx.de>
>>>>>>> Cc: Simon Glass <sjg at chromium.org>
>>>>>>> Cc: Bin Meng <bmeng.cn at 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
More information about the U-Boot
mailing list