[U-Boot] [PATCH v2] x86: lib: Implement standalone __udivdi3 etc instead of libgcc ones
Stefan Roese
sr at denx.de
Mon Nov 27 16:34:00 UTC 2017
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,
Stefan
More information about the U-Boot
mailing list