[U-Boot] [PATCH v2] x86: lib: Implement standalone __udivdi3 etc instead of libgcc ones
Bin Meng
bmeng.cn at gmail.com
Mon Nov 27 09:46:48 UTC 2017
Hi Stefan,
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.
> I would prefer to add this patch and once compile breakage occur because
> of such "undefined reference" error from above, we should investigate
> the root cause of fix it by replacing division operation by one of
> the provided helper functions.
>
Regards,
Bin
More information about the U-Boot
mailing list