* Re: [PATCH] lib: Add shared copy of __lshrti3 from libgcc
2019-03-15 22:06 ` Nick Desaulniers
@ 2019-03-15 22:08 ` hpa
2019-03-15 23:34 ` Matthias Kaehlcke
2019-03-15 22:12 ` hpa
` (2 subsequent siblings)
3 siblings, 1 reply; 21+ messages in thread
From: hpa @ 2019-03-15 22:08 UTC (permalink / raw)
To: Nick Desaulniers, Matthias Kaehlcke
Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
x86, LKML, Manoj Gupta, Tiancong Wang, Stephen Hines,
clang-built-linux, Miguel Ojeda
On March 15, 2019 3:06:37 PM PDT, Nick Desaulniers <ndesaulniers@google.com> wrote:
>On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <mka@chromium.org>
>wrote:
>>
>> The compiler may emit calls to __lshrti3 from the compiler runtime
>> library, which results in undefined references:
>>
>> arch/x86/kvm/x86.o: In function `mul_u64_u64_shr':
>> include/linux/math64.h:186: undefined reference to `__lshrti3'
>
>Looks like Clang will emit this at -Oz (but not -O2):
>https://godbolt.org/z/w1_2YC
>
>>
>> Add a copy of the __lshrti3 libgcc routine (from gcc v4.9.2).
>
>Has it changed since? If so why not a newer version of libgcc_s?
>
>>
>> Include the function for x86 builds with clang, which is the
>> environment where the above error was observed.
>>
>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>> ---
>> arch/x86/Kconfig | 1 +
>> include/linux/libgcc.h | 16 ++++++++++++++++
>> lib/Kconfig | 3 +++
>> lib/Makefile | 1 +
>> lib/lshrti3.c | 31 +++++++++++++++++++++++++++++++
>> 5 files changed, 52 insertions(+)
>> create mode 100644 lib/lshrti3.c
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index c1f9b3cf437c..a5e0d923845d 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -105,6 +105,7 @@ config X86
>> select GENERIC_IRQ_PROBE
>> select GENERIC_IRQ_RESERVATION_MODE
>> select GENERIC_IRQ_SHOW
>> + select GENERIC_LIB_LSHRTI3 if CC_IS_CLANG
>> select GENERIC_PENDING_IRQ if SMP
>> select GENERIC_SMP_IDLE_THREAD
>> select GENERIC_STRNCPY_FROM_USER
>> diff --git a/include/linux/libgcc.h b/include/linux/libgcc.h
>> index 32e1e0f4b2d0..a71036471838 100644
>> --- a/include/linux/libgcc.h
>> +++ b/include/linux/libgcc.h
>> @@ -22,15 +22,26 @@
>> #include <asm/byteorder.h>
>>
>> typedef int word_type __attribute__ ((mode (__word__)));
>> +typedef int TItype __attribute__ ((mode (TI)));
>
>Well that's an interesting new compiler attribute.
>https://stackoverflow.com/questions/4559025/what-does-gcc-attribute-modexx-actually-do
>Please use `__mode(TI)` from include/linux/compiler_attributes.h ex.
>typedef int TItype __mode(TI);
>
>>
>> #ifdef __BIG_ENDIAN
>> struct DWstruct {
>> int high, low;
>> };
>> +
>> +struct DWstruct128 {
>> + long long high, low;
>> +};
>> +
>> #elif defined(__LITTLE_ENDIAN)
>> struct DWstruct {
>> int low, high;
>> };
>> +
>> +struct DWstruct128 {
>> + long long low, high;
>> +};
>> +
>> #else
>> #error I feel sick.
>> #endif
>> @@ -40,4 +51,9 @@ typedef union {
>> long long ll;
>> } DWunion;
>>
>> +typedef union {
>> + struct DWstruct128 s;
>> + TItype ll;
>> +} DWunion128;
>> +
>> #endif /* __ASM_LIBGCC_H */
>> diff --git a/lib/Kconfig b/lib/Kconfig
>> index a9e56539bd11..369e10259ea6 100644
>> --- a/lib/Kconfig
>> +++ b/lib/Kconfig
>> @@ -624,6 +624,9 @@ config GENERIC_LIB_ASHRDI3
>> config GENERIC_LIB_LSHRDI3
>> bool
>>
>> +config GENERIC_LIB_LSHRTI3
>> + bool
>> +
>> config GENERIC_LIB_MULDI3
>> bool
>>
>> diff --git a/lib/Makefile b/lib/Makefile
>> index 4e066120a0d6..42648411f451 100644
>> --- a/lib/Makefile
>> +++ b/lib/Makefile
>> @@ -277,6 +277,7 @@ obj-$(CONFIG_PARMAN) += parman.o
>> obj-$(CONFIG_GENERIC_LIB_ASHLDI3) += ashldi3.o
>> obj-$(CONFIG_GENERIC_LIB_ASHRDI3) += ashrdi3.o
>> obj-$(CONFIG_GENERIC_LIB_LSHRDI3) += lshrdi3.o
>> +obj-$(CONFIG_GENERIC_LIB_LSHRTI3) += lshrti3.o
>> obj-$(CONFIG_GENERIC_LIB_MULDI3) += muldi3.o
>> obj-$(CONFIG_GENERIC_LIB_CMPDI2) += cmpdi2.o
>> obj-$(CONFIG_GENERIC_LIB_UCMPDI2) += ucmpdi2.o
>> diff --git a/lib/lshrti3.c b/lib/lshrti3.c
>> new file mode 100644
>> index 000000000000..2d2123bb3030
>> --- /dev/null
>> +++ b/lib/lshrti3.c
>> @@ -0,0 +1,31 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <linux/export.h>
>> +#include <linux/libgcc.h>
>> +
>> +long long __lshrti3(long long u, word_type b)
>> +{
>> + DWunion128 uu, w;
>> + word_type bm;
>> +
>> + if (b == 0)
>> + return u;
>> +
>> + uu.ll = u;
>> + bm = 64 - b;
>> +
>> + if (bm <= 0) {
>> + w.s.high = 0;
>> + w.s.low = (unsigned long long) uu.s.high >> -bm;
>> + } else {
>> + const unsigned long long carries =
>> + (unsigned long long) uu.s.high << bm;
>> + w.s.high = (unsigned long long) uu.s.high >> b;
>> + w.s.low = ((unsigned long long) uu.s.low >> b) |
>carries;
>> + }
>> +
>> + return w.ll;
>> +}
>> +#ifndef BUILD_VDSO
>> +EXPORT_SYMBOL(__lshrti3);
>> +#endif
>
>I don't think you want this. Maybe that was carried over from
>https://lkml.org/lkml/2019/3/15/669
>by accident? The above linkage error mentions arch/x86/kvm/x86.o
>which I wouldn't expect to be linked into the VDSO image?
If we compile with the default ABI we can simply link with libgcc; with -mregparm=3 all versions of gcc are broken.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] lib: Add shared copy of __lshrti3 from libgcc
2019-03-15 22:08 ` hpa
@ 2019-03-15 23:34 ` Matthias Kaehlcke
2019-03-15 23:51 ` hpa
0 siblings, 1 reply; 21+ messages in thread
From: Matthias Kaehlcke @ 2019-03-15 23:34 UTC (permalink / raw)
To: hpa
Cc: Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, LKML, Manoj Gupta, Tiancong Wang,
Stephen Hines, clang-built-linux, Miguel Ojeda
Hi Peter,
On Fri, Mar 15, 2019 at 03:08:57PM -0700, hpa@zytor.com wrote:
> On March 15, 2019 3:06:37 PM PDT, Nick Desaulniers <ndesaulniers@google.com> wrote:
> >On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <mka@chromium.org>
> >wrote:
> >>
> >> The compiler may emit calls to __lshrti3 from the compiler runtime
> >> library, which results in undefined references:
> >>
> >> arch/x86/kvm/x86.o: In function `mul_u64_u64_shr':
> >> include/linux/math64.h:186: undefined reference to `__lshrti3'
> >
> >Looks like Clang will emit this at -Oz (but not -O2):
> >https://godbolt.org/z/w1_2YC
> >
> >>
> >> Add a copy of the __lshrti3 libgcc routine (from gcc v4.9.2).
> >
> >Has it changed since? If so why not a newer version of libgcc_s?
> >
> >>
> >> Include the function for x86 builds with clang, which is the
> >> environment where the above error was observed.
> >>
> >> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> >> ---
> >> arch/x86/Kconfig | 1 +
> >> include/linux/libgcc.h | 16 ++++++++++++++++
> >> lib/Kconfig | 3 +++
> >> lib/Makefile | 1 +
> >> lib/lshrti3.c | 31 +++++++++++++++++++++++++++++++
> >> 5 files changed, 52 insertions(+)
> >> create mode 100644 lib/lshrti3.c
> >>
> >> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> >> index c1f9b3cf437c..a5e0d923845d 100644
> >> --- a/arch/x86/Kconfig
> >> +++ b/arch/x86/Kconfig
> >> @@ -105,6 +105,7 @@ config X86
> >> select GENERIC_IRQ_PROBE
> >> select GENERIC_IRQ_RESERVATION_MODE
> >> select GENERIC_IRQ_SHOW
> >> + select GENERIC_LIB_LSHRTI3 if CC_IS_CLANG
> >> select GENERIC_PENDING_IRQ if SMP
> >> select GENERIC_SMP_IDLE_THREAD
> >> select GENERIC_STRNCPY_FROM_USER
> >> diff --git a/include/linux/libgcc.h b/include/linux/libgcc.h
> >> index 32e1e0f4b2d0..a71036471838 100644
> >> --- a/include/linux/libgcc.h
> >> +++ b/include/linux/libgcc.h
> >> @@ -22,15 +22,26 @@
> >> #include <asm/byteorder.h>
> >>
> >> typedef int word_type __attribute__ ((mode (__word__)));
> >> +typedef int TItype __attribute__ ((mode (TI)));
> >
> >Well that's an interesting new compiler attribute.
> >https://stackoverflow.com/questions/4559025/what-does-gcc-attribute-modexx-actually-do
> >Please use `__mode(TI)` from include/linux/compiler_attributes.h ex.
> >typedef int TItype __mode(TI);
> >
> >>
> >> #ifdef __BIG_ENDIAN
> >> struct DWstruct {
> >> int high, low;
> >> };
> >> +
> >> +struct DWstruct128 {
> >> + long long high, low;
> >> +};
> >> +
> >> #elif defined(__LITTLE_ENDIAN)
> >> struct DWstruct {
> >> int low, high;
> >> };
> >> +
> >> +struct DWstruct128 {
> >> + long long low, high;
> >> +};
> >> +
> >> #else
> >> #error I feel sick.
> >> #endif
> >> @@ -40,4 +51,9 @@ typedef union {
> >> long long ll;
> >> } DWunion;
> >>
> >> +typedef union {
> >> + struct DWstruct128 s;
> >> + TItype ll;
> >> +} DWunion128;
> >> +
> >> #endif /* __ASM_LIBGCC_H */
> >> diff --git a/lib/Kconfig b/lib/Kconfig
> >> index a9e56539bd11..369e10259ea6 100644
> >> --- a/lib/Kconfig
> >> +++ b/lib/Kconfig
> >> @@ -624,6 +624,9 @@ config GENERIC_LIB_ASHRDI3
> >> config GENERIC_LIB_LSHRDI3
> >> bool
> >>
> >> +config GENERIC_LIB_LSHRTI3
> >> + bool
> >> +
> >> config GENERIC_LIB_MULDI3
> >> bool
> >>
> >> diff --git a/lib/Makefile b/lib/Makefile
> >> index 4e066120a0d6..42648411f451 100644
> >> --- a/lib/Makefile
> >> +++ b/lib/Makefile
> >> @@ -277,6 +277,7 @@ obj-$(CONFIG_PARMAN) += parman.o
> >> obj-$(CONFIG_GENERIC_LIB_ASHLDI3) += ashldi3.o
> >> obj-$(CONFIG_GENERIC_LIB_ASHRDI3) += ashrdi3.o
> >> obj-$(CONFIG_GENERIC_LIB_LSHRDI3) += lshrdi3.o
> >> +obj-$(CONFIG_GENERIC_LIB_LSHRTI3) += lshrti3.o
> >> obj-$(CONFIG_GENERIC_LIB_MULDI3) += muldi3.o
> >> obj-$(CONFIG_GENERIC_LIB_CMPDI2) += cmpdi2.o
> >> obj-$(CONFIG_GENERIC_LIB_UCMPDI2) += ucmpdi2.o
> >> diff --git a/lib/lshrti3.c b/lib/lshrti3.c
> >> new file mode 100644
> >> index 000000000000..2d2123bb3030
> >> --- /dev/null
> >> +++ b/lib/lshrti3.c
> >> @@ -0,0 +1,31 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +
> >> +#include <linux/export.h>
> >> +#include <linux/libgcc.h>
> >> +
> >> +long long __lshrti3(long long u, word_type b)
> >> +{
> >> + DWunion128 uu, w;
> >> + word_type bm;
> >> +
> >> + if (b == 0)
> >> + return u;
> >> +
> >> + uu.ll = u;
> >> + bm = 64 - b;
> >> +
> >> + if (bm <= 0) {
> >> + w.s.high = 0;
> >> + w.s.low = (unsigned long long) uu.s.high >> -bm;
> >> + } else {
> >> + const unsigned long long carries =
> >> + (unsigned long long) uu.s.high << bm;
> >> + w.s.high = (unsigned long long) uu.s.high >> b;
> >> + w.s.low = ((unsigned long long) uu.s.low >> b) |
> >carries;
> >> + }
> >> +
> >> + return w.ll;
> >> +}
> >> +#ifndef BUILD_VDSO
> >> +EXPORT_SYMBOL(__lshrti3);
> >> +#endif
> >
> >I don't think you want this. Maybe that was carried over from
> >https://lkml.org/lkml/2019/3/15/669
> >by accident? The above linkage error mentions arch/x86/kvm/x86.o
> >which I wouldn't expect to be linked into the VDSO image?
>
> If we compile with the default ABI we can simply link with libgcc;
> with -mregparm=3 all versions of gcc are broken.
I suppose you refer to linking the VDSO against libgcc? Would this be
acceptable (as opposed to linking the kernel against it)?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] lib: Add shared copy of __lshrti3 from libgcc
2019-03-15 23:34 ` Matthias Kaehlcke
@ 2019-03-15 23:51 ` hpa
0 siblings, 0 replies; 21+ messages in thread
From: hpa @ 2019-03-15 23:51 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, LKML, Manoj Gupta, Tiancong Wang,
Stephen Hines, clang-built-linux, Miguel Ojeda
On March 15, 2019 4:34:10 PM PDT, Matthias Kaehlcke <mka@chromium.org> wrote:
>Hi Peter,
>
>On Fri, Mar 15, 2019 at 03:08:57PM -0700, hpa@zytor.com wrote:
>> On March 15, 2019 3:06:37 PM PDT, Nick Desaulniers
><ndesaulniers@google.com> wrote:
>> >On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <mka@chromium.org>
>> >wrote:
>> >>
>> >> The compiler may emit calls to __lshrti3 from the compiler runtime
>> >> library, which results in undefined references:
>> >>
>> >> arch/x86/kvm/x86.o: In function `mul_u64_u64_shr':
>> >> include/linux/math64.h:186: undefined reference to `__lshrti3'
>> >
>> >Looks like Clang will emit this at -Oz (but not -O2):
>> >https://godbolt.org/z/w1_2YC
>> >
>> >>
>> >> Add a copy of the __lshrti3 libgcc routine (from gcc v4.9.2).
>> >
>> >Has it changed since? If so why not a newer version of libgcc_s?
>> >
>> >>
>> >> Include the function for x86 builds with clang, which is the
>> >> environment where the above error was observed.
>> >>
>> >> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>> >> ---
>> >> arch/x86/Kconfig | 1 +
>> >> include/linux/libgcc.h | 16 ++++++++++++++++
>> >> lib/Kconfig | 3 +++
>> >> lib/Makefile | 1 +
>> >> lib/lshrti3.c | 31 +++++++++++++++++++++++++++++++
>> >> 5 files changed, 52 insertions(+)
>> >> create mode 100644 lib/lshrti3.c
>> >>
>> >> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> >> index c1f9b3cf437c..a5e0d923845d 100644
>> >> --- a/arch/x86/Kconfig
>> >> +++ b/arch/x86/Kconfig
>> >> @@ -105,6 +105,7 @@ config X86
>> >> select GENERIC_IRQ_PROBE
>> >> select GENERIC_IRQ_RESERVATION_MODE
>> >> select GENERIC_IRQ_SHOW
>> >> + select GENERIC_LIB_LSHRTI3 if CC_IS_CLANG
>> >> select GENERIC_PENDING_IRQ if SMP
>> >> select GENERIC_SMP_IDLE_THREAD
>> >> select GENERIC_STRNCPY_FROM_USER
>> >> diff --git a/include/linux/libgcc.h b/include/linux/libgcc.h
>> >> index 32e1e0f4b2d0..a71036471838 100644
>> >> --- a/include/linux/libgcc.h
>> >> +++ b/include/linux/libgcc.h
>> >> @@ -22,15 +22,26 @@
>> >> #include <asm/byteorder.h>
>> >>
>> >> typedef int word_type __attribute__ ((mode (__word__)));
>> >> +typedef int TItype __attribute__ ((mode (TI)));
>> >
>> >Well that's an interesting new compiler attribute.
>>
>>https://stackoverflow.com/questions/4559025/what-does-gcc-attribute-modexx-actually-do
>> >Please use `__mode(TI)` from include/linux/compiler_attributes.h ex.
>> >typedef int TItype __mode(TI);
>> >
>> >>
>> >> #ifdef __BIG_ENDIAN
>> >> struct DWstruct {
>> >> int high, low;
>> >> };
>> >> +
>> >> +struct DWstruct128 {
>> >> + long long high, low;
>> >> +};
>> >> +
>> >> #elif defined(__LITTLE_ENDIAN)
>> >> struct DWstruct {
>> >> int low, high;
>> >> };
>> >> +
>> >> +struct DWstruct128 {
>> >> + long long low, high;
>> >> +};
>> >> +
>> >> #else
>> >> #error I feel sick.
>> >> #endif
>> >> @@ -40,4 +51,9 @@ typedef union {
>> >> long long ll;
>> >> } DWunion;
>> >>
>> >> +typedef union {
>> >> + struct DWstruct128 s;
>> >> + TItype ll;
>> >> +} DWunion128;
>> >> +
>> >> #endif /* __ASM_LIBGCC_H */
>> >> diff --git a/lib/Kconfig b/lib/Kconfig
>> >> index a9e56539bd11..369e10259ea6 100644
>> >> --- a/lib/Kconfig
>> >> +++ b/lib/Kconfig
>> >> @@ -624,6 +624,9 @@ config GENERIC_LIB_ASHRDI3
>> >> config GENERIC_LIB_LSHRDI3
>> >> bool
>> >>
>> >> +config GENERIC_LIB_LSHRTI3
>> >> + bool
>> >> +
>> >> config GENERIC_LIB_MULDI3
>> >> bool
>> >>
>> >> diff --git a/lib/Makefile b/lib/Makefile
>> >> index 4e066120a0d6..42648411f451 100644
>> >> --- a/lib/Makefile
>> >> +++ b/lib/Makefile
>> >> @@ -277,6 +277,7 @@ obj-$(CONFIG_PARMAN) += parman.o
>> >> obj-$(CONFIG_GENERIC_LIB_ASHLDI3) += ashldi3.o
>> >> obj-$(CONFIG_GENERIC_LIB_ASHRDI3) += ashrdi3.o
>> >> obj-$(CONFIG_GENERIC_LIB_LSHRDI3) += lshrdi3.o
>> >> +obj-$(CONFIG_GENERIC_LIB_LSHRTI3) += lshrti3.o
>> >> obj-$(CONFIG_GENERIC_LIB_MULDI3) += muldi3.o
>> >> obj-$(CONFIG_GENERIC_LIB_CMPDI2) += cmpdi2.o
>> >> obj-$(CONFIG_GENERIC_LIB_UCMPDI2) += ucmpdi2.o
>> >> diff --git a/lib/lshrti3.c b/lib/lshrti3.c
>> >> new file mode 100644
>> >> index 000000000000..2d2123bb3030
>> >> --- /dev/null
>> >> +++ b/lib/lshrti3.c
>> >> @@ -0,0 +1,31 @@
>> >> +// SPDX-License-Identifier: GPL-2.0
>> >> +
>> >> +#include <linux/export.h>
>> >> +#include <linux/libgcc.h>
>> >> +
>> >> +long long __lshrti3(long long u, word_type b)
>> >> +{
>> >> + DWunion128 uu, w;
>> >> + word_type bm;
>> >> +
>> >> + if (b == 0)
>> >> + return u;
>> >> +
>> >> + uu.ll = u;
>> >> + bm = 64 - b;
>> >> +
>> >> + if (bm <= 0) {
>> >> + w.s.high = 0;
>> >> + w.s.low = (unsigned long long) uu.s.high >> -bm;
>> >> + } else {
>> >> + const unsigned long long carries =
>> >> + (unsigned long long) uu.s.high << bm;
>> >> + w.s.high = (unsigned long long) uu.s.high >> b;
>> >> + w.s.low = ((unsigned long long) uu.s.low >> b) |
>> >carries;
>> >> + }
>> >> +
>> >> + return w.ll;
>> >> +}
>> >> +#ifndef BUILD_VDSO
>> >> +EXPORT_SYMBOL(__lshrti3);
>> >> +#endif
>> >
>> >I don't think you want this. Maybe that was carried over from
>> >https://lkml.org/lkml/2019/3/15/669
>> >by accident? The above linkage error mentions arch/x86/kvm/x86.o
>> >which I wouldn't expect to be linked into the VDSO image?
>>
>> If we compile with the default ABI we can simply link with libgcc;
>> with -mregparm=3 all versions of gcc are broken.
>
>I suppose you refer to linking the VDSO against libgcc? Would this be
>acceptable (as opposed to linking the kernel against it)?
Yes.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] lib: Add shared copy of __lshrti3 from libgcc
2019-03-15 22:06 ` Nick Desaulniers
2019-03-15 22:08 ` hpa
@ 2019-03-15 22:12 ` hpa
2019-03-15 23:47 ` Matthias Kaehlcke
2019-03-15 23:29 ` Matthias Kaehlcke
2019-03-18 9:14 ` Peter Zijlstra
3 siblings, 1 reply; 21+ messages in thread
From: hpa @ 2019-03-15 22:12 UTC (permalink / raw)
To: Nick Desaulniers, Matthias Kaehlcke
Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
x86, LKML, Manoj Gupta, Tiancong Wang, Stephen Hines,
clang-built-linux, Miguel Ojeda
On March 15, 2019 3:06:37 PM PDT, Nick Desaulniers <ndesaulniers@google.com> wrote:
>On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <mka@chromium.org>
>wrote:
>>
>> The compiler may emit calls to __lshrti3 from the compiler runtime
>> library, which results in undefined references:
>>
>> arch/x86/kvm/x86.o: In function `mul_u64_u64_shr':
>> include/linux/math64.h:186: undefined reference to `__lshrti3'
>
>Looks like Clang will emit this at -Oz (but not -O2):
>https://godbolt.org/z/w1_2YC
>
>>
>> Add a copy of the __lshrti3 libgcc routine (from gcc v4.9.2).
>
>Has it changed since? If so why not a newer version of libgcc_s?
>
>>
>> Include the function for x86 builds with clang, which is the
>> environment where the above error was observed.
>>
>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>> ---
>> arch/x86/Kconfig | 1 +
>> include/linux/libgcc.h | 16 ++++++++++++++++
>> lib/Kconfig | 3 +++
>> lib/Makefile | 1 +
>> lib/lshrti3.c | 31 +++++++++++++++++++++++++++++++
>> 5 files changed, 52 insertions(+)
>> create mode 100644 lib/lshrti3.c
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index c1f9b3cf437c..a5e0d923845d 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -105,6 +105,7 @@ config X86
>> select GENERIC_IRQ_PROBE
>> select GENERIC_IRQ_RESERVATION_MODE
>> select GENERIC_IRQ_SHOW
>> + select GENERIC_LIB_LSHRTI3 if CC_IS_CLANG
>> select GENERIC_PENDING_IRQ if SMP
>> select GENERIC_SMP_IDLE_THREAD
>> select GENERIC_STRNCPY_FROM_USER
>> diff --git a/include/linux/libgcc.h b/include/linux/libgcc.h
>> index 32e1e0f4b2d0..a71036471838 100644
>> --- a/include/linux/libgcc.h
>> +++ b/include/linux/libgcc.h
>> @@ -22,15 +22,26 @@
>> #include <asm/byteorder.h>
>>
>> typedef int word_type __attribute__ ((mode (__word__)));
>> +typedef int TItype __attribute__ ((mode (TI)));
>
>Well that's an interesting new compiler attribute.
>https://stackoverflow.com/questions/4559025/what-does-gcc-attribute-modexx-actually-do
>Please use `__mode(TI)` from include/linux/compiler_attributes.h ex.
>typedef int TItype __mode(TI);
>
>>
>> #ifdef __BIG_ENDIAN
>> struct DWstruct {
>> int high, low;
>> };
>> +
>> +struct DWstruct128 {
>> + long long high, low;
>> +};
>> +
>> #elif defined(__LITTLE_ENDIAN)
>> struct DWstruct {
>> int low, high;
>> };
>> +
>> +struct DWstruct128 {
>> + long long low, high;
>> +};
>> +
>> #else
>> #error I feel sick.
>> #endif
>> @@ -40,4 +51,9 @@ typedef union {
>> long long ll;
>> } DWunion;
>>
>> +typedef union {
>> + struct DWstruct128 s;
>> + TItype ll;
>> +} DWunion128;
>> +
>> #endif /* __ASM_LIBGCC_H */
>> diff --git a/lib/Kconfig b/lib/Kconfig
>> index a9e56539bd11..369e10259ea6 100644
>> --- a/lib/Kconfig
>> +++ b/lib/Kconfig
>> @@ -624,6 +624,9 @@ config GENERIC_LIB_ASHRDI3
>> config GENERIC_LIB_LSHRDI3
>> bool
>>
>> +config GENERIC_LIB_LSHRTI3
>> + bool
>> +
>> config GENERIC_LIB_MULDI3
>> bool
>>
>> diff --git a/lib/Makefile b/lib/Makefile
>> index 4e066120a0d6..42648411f451 100644
>> --- a/lib/Makefile
>> +++ b/lib/Makefile
>> @@ -277,6 +277,7 @@ obj-$(CONFIG_PARMAN) += parman.o
>> obj-$(CONFIG_GENERIC_LIB_ASHLDI3) += ashldi3.o
>> obj-$(CONFIG_GENERIC_LIB_ASHRDI3) += ashrdi3.o
>> obj-$(CONFIG_GENERIC_LIB_LSHRDI3) += lshrdi3.o
>> +obj-$(CONFIG_GENERIC_LIB_LSHRTI3) += lshrti3.o
>> obj-$(CONFIG_GENERIC_LIB_MULDI3) += muldi3.o
>> obj-$(CONFIG_GENERIC_LIB_CMPDI2) += cmpdi2.o
>> obj-$(CONFIG_GENERIC_LIB_UCMPDI2) += ucmpdi2.o
>> diff --git a/lib/lshrti3.c b/lib/lshrti3.c
>> new file mode 100644
>> index 000000000000..2d2123bb3030
>> --- /dev/null
>> +++ b/lib/lshrti3.c
>> @@ -0,0 +1,31 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <linux/export.h>
>> +#include <linux/libgcc.h>
>> +
>> +long long __lshrti3(long long u, word_type b)
>> +{
>> + DWunion128 uu, w;
>> + word_type bm;
>> +
>> + if (b == 0)
>> + return u;
>> +
>> + uu.ll = u;
>> + bm = 64 - b;
>> +
>> + if (bm <= 0) {
>> + w.s.high = 0;
>> + w.s.low = (unsigned long long) uu.s.high >> -bm;
>> + } else {
>> + const unsigned long long carries =
>> + (unsigned long long) uu.s.high << bm;
>> + w.s.high = (unsigned long long) uu.s.high >> b;
>> + w.s.low = ((unsigned long long) uu.s.low >> b) |
>carries;
>> + }
>> +
>> + return w.ll;
>> +}
>> +#ifndef BUILD_VDSO
>> +EXPORT_SYMBOL(__lshrti3);
>> +#endif
>
>I don't think you want this. Maybe that was carried over from
>https://lkml.org/lkml/2019/3/15/669
>by accident? The above linkage error mentions arch/x86/kvm/x86.o
>which I wouldn't expect to be linked into the VDSO image?
Or just "u64"...
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] lib: Add shared copy of __lshrti3 from libgcc
2019-03-15 22:12 ` hpa
@ 2019-03-15 23:47 ` Matthias Kaehlcke
2019-03-15 23:53 ` hpa
0 siblings, 1 reply; 21+ messages in thread
From: Matthias Kaehlcke @ 2019-03-15 23:47 UTC (permalink / raw)
To: hpa
Cc: Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, LKML, Manoj Gupta, Tiancong Wang,
Stephen Hines, clang-built-linux, Miguel Ojeda
On Fri, Mar 15, 2019 at 03:12:08PM -0700, hpa@zytor.com wrote:
> On March 15, 2019 3:06:37 PM PDT, Nick Desaulniers <ndesaulniers@google.com> wrote:
> >On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <mka@chromium.org>
> >wrote:
> >>
> >> The compiler may emit calls to __lshrti3 from the compiler runtime
> >> library, which results in undefined references:
> >>
> >> arch/x86/kvm/x86.o: In function `mul_u64_u64_shr':
> >> include/linux/math64.h:186: undefined reference to `__lshrti3'
> >
> >Looks like Clang will emit this at -Oz (but not -O2):
> >https://godbolt.org/z/w1_2YC
> >
> >>
> >> Add a copy of the __lshrti3 libgcc routine (from gcc v4.9.2).
> >
> >Has it changed since? If so why not a newer version of libgcc_s?
> >
> >>
> >> Include the function for x86 builds with clang, which is the
> >> environment where the above error was observed.
> >>
> >> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> >> ---
> >> arch/x86/Kconfig | 1 +
> >> include/linux/libgcc.h | 16 ++++++++++++++++
> >> lib/Kconfig | 3 +++
> >> lib/Makefile | 1 +
> >> lib/lshrti3.c | 31 +++++++++++++++++++++++++++++++
> >> 5 files changed, 52 insertions(+)
> >> create mode 100644 lib/lshrti3.c
> >>
> >> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> >> index c1f9b3cf437c..a5e0d923845d 100644
> >> --- a/arch/x86/Kconfig
> >> +++ b/arch/x86/Kconfig
> >> @@ -105,6 +105,7 @@ config X86
> >> select GENERIC_IRQ_PROBE
> >> select GENERIC_IRQ_RESERVATION_MODE
> >> select GENERIC_IRQ_SHOW
> >> + select GENERIC_LIB_LSHRTI3 if CC_IS_CLANG
> >> select GENERIC_PENDING_IRQ if SMP
> >> select GENERIC_SMP_IDLE_THREAD
> >> select GENERIC_STRNCPY_FROM_USER
> >> diff --git a/include/linux/libgcc.h b/include/linux/libgcc.h
> >> index 32e1e0f4b2d0..a71036471838 100644
> >> --- a/include/linux/libgcc.h
> >> +++ b/include/linux/libgcc.h
> >> @@ -22,15 +22,26 @@
> >> #include <asm/byteorder.h>
> >>
> >> typedef int word_type __attribute__ ((mode (__word__)));
> >> +typedef int TItype __attribute__ ((mode (TI)));
> >
> >Well that's an interesting new compiler attribute.
> >https://stackoverflow.com/questions/4559025/what-does-gcc-attribute-modexx-actually-do
> >Please use `__mode(TI)` from include/linux/compiler_attributes.h ex.
> >typedef int TItype __mode(TI);
> >
> >>
> >> #ifdef __BIG_ENDIAN
> >> struct DWstruct {
> >> int high, low;
> >> };
> >> +
> >> +struct DWstruct128 {
> >> + long long high, low;
> >> +};
> >> +
> >> #elif defined(__LITTLE_ENDIAN)
> >> struct DWstruct {
> >> int low, high;
> >> };
> >> +
> >> +struct DWstruct128 {
> >> + long long low, high;
> >> +};
> >> +
> >> #else
> >> #error I feel sick.
> >> #endif
> >> @@ -40,4 +51,9 @@ typedef union {
> >> long long ll;
> >> } DWunion;
> >>
> >> +typedef union {
> >> + struct DWstruct128 s;
> >> + TItype ll;
> >> +} DWunion128;
> >> +
> >> #endif /* __ASM_LIBGCC_H */
> >> diff --git a/lib/Kconfig b/lib/Kconfig
> >> index a9e56539bd11..369e10259ea6 100644
> >> --- a/lib/Kconfig
> >> +++ b/lib/Kconfig
> >> @@ -624,6 +624,9 @@ config GENERIC_LIB_ASHRDI3
> >> config GENERIC_LIB_LSHRDI3
> >> bool
> >>
> >> +config GENERIC_LIB_LSHRTI3
> >> + bool
> >> +
> >> config GENERIC_LIB_MULDI3
> >> bool
> >>
> >> diff --git a/lib/Makefile b/lib/Makefile
> >> index 4e066120a0d6..42648411f451 100644
> >> --- a/lib/Makefile
> >> +++ b/lib/Makefile
> >> @@ -277,6 +277,7 @@ obj-$(CONFIG_PARMAN) += parman.o
> >> obj-$(CONFIG_GENERIC_LIB_ASHLDI3) += ashldi3.o
> >> obj-$(CONFIG_GENERIC_LIB_ASHRDI3) += ashrdi3.o
> >> obj-$(CONFIG_GENERIC_LIB_LSHRDI3) += lshrdi3.o
> >> +obj-$(CONFIG_GENERIC_LIB_LSHRTI3) += lshrti3.o
> >> obj-$(CONFIG_GENERIC_LIB_MULDI3) += muldi3.o
> >> obj-$(CONFIG_GENERIC_LIB_CMPDI2) += cmpdi2.o
> >> obj-$(CONFIG_GENERIC_LIB_UCMPDI2) += ucmpdi2.o
> >> diff --git a/lib/lshrti3.c b/lib/lshrti3.c
> >> new file mode 100644
> >> index 000000000000..2d2123bb3030
> >> --- /dev/null
> >> +++ b/lib/lshrti3.c
> >> @@ -0,0 +1,31 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +
> >> +#include <linux/export.h>
> >> +#include <linux/libgcc.h>
> >> +
> >> +long long __lshrti3(long long u, word_type b)
> >> +{
> >> + DWunion128 uu, w;
> >> + word_type bm;
> >> +
> >> + if (b == 0)
> >> + return u;
> >> +
> >> + uu.ll = u;
> >> + bm = 64 - b;
> >> +
> >> + if (bm <= 0) {
> >> + w.s.high = 0;
> >> + w.s.low = (unsigned long long) uu.s.high >> -bm;
> >> + } else {
> >> + const unsigned long long carries =
> >> + (unsigned long long) uu.s.high << bm;
> >> + w.s.high = (unsigned long long) uu.s.high >> b;
> >> + w.s.low = ((unsigned long long) uu.s.low >> b) |
> >carries;
> >> + }
> >> +
> >> + return w.ll;
> >> +}
> >> +#ifndef BUILD_VDSO
> >> +EXPORT_SYMBOL(__lshrti3);
> >> +#endif
> >
> >I don't think you want this. Maybe that was carried over from
> >https://lkml.org/lkml/2019/3/15/669
> >by accident? The above linkage error mentions arch/x86/kvm/x86.o
> >which I wouldn't expect to be linked into the VDSO image?
>
> Or just "u64"...
Are you referring to TItype? It currenty is a 128-bit value.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] lib: Add shared copy of __lshrti3 from libgcc
2019-03-15 23:47 ` Matthias Kaehlcke
@ 2019-03-15 23:53 ` hpa
2019-03-18 16:38 ` Matthias Kaehlcke
0 siblings, 1 reply; 21+ messages in thread
From: hpa @ 2019-03-15 23:53 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, LKML, Manoj Gupta, Tiancong Wang,
Stephen Hines, clang-built-linux, Miguel Ojeda
On March 15, 2019 4:47:01 PM PDT, Matthias Kaehlcke <mka@chromium.org> wrote:
>On Fri, Mar 15, 2019 at 03:12:08PM -0700, hpa@zytor.com wrote:
>> On March 15, 2019 3:06:37 PM PDT, Nick Desaulniers
><ndesaulniers@google.com> wrote:
>> >On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <mka@chromium.org>
>> >wrote:
>> >>
>> >> The compiler may emit calls to __lshrti3 from the compiler runtime
>> >> library, which results in undefined references:
>> >>
>> >> arch/x86/kvm/x86.o: In function `mul_u64_u64_shr':
>> >> include/linux/math64.h:186: undefined reference to `__lshrti3'
>> >
>> >Looks like Clang will emit this at -Oz (but not -O2):
>> >https://godbolt.org/z/w1_2YC
>> >
>> >>
>> >> Add a copy of the __lshrti3 libgcc routine (from gcc v4.9.2).
>> >
>> >Has it changed since? If so why not a newer version of libgcc_s?
>> >
>> >>
>> >> Include the function for x86 builds with clang, which is the
>> >> environment where the above error was observed.
>> >>
>> >> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>> >> ---
>> >> arch/x86/Kconfig | 1 +
>> >> include/linux/libgcc.h | 16 ++++++++++++++++
>> >> lib/Kconfig | 3 +++
>> >> lib/Makefile | 1 +
>> >> lib/lshrti3.c | 31 +++++++++++++++++++++++++++++++
>> >> 5 files changed, 52 insertions(+)
>> >> create mode 100644 lib/lshrti3.c
>> >>
>> >> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> >> index c1f9b3cf437c..a5e0d923845d 100644
>> >> --- a/arch/x86/Kconfig
>> >> +++ b/arch/x86/Kconfig
>> >> @@ -105,6 +105,7 @@ config X86
>> >> select GENERIC_IRQ_PROBE
>> >> select GENERIC_IRQ_RESERVATION_MODE
>> >> select GENERIC_IRQ_SHOW
>> >> + select GENERIC_LIB_LSHRTI3 if CC_IS_CLANG
>> >> select GENERIC_PENDING_IRQ if SMP
>> >> select GENERIC_SMP_IDLE_THREAD
>> >> select GENERIC_STRNCPY_FROM_USER
>> >> diff --git a/include/linux/libgcc.h b/include/linux/libgcc.h
>> >> index 32e1e0f4b2d0..a71036471838 100644
>> >> --- a/include/linux/libgcc.h
>> >> +++ b/include/linux/libgcc.h
>> >> @@ -22,15 +22,26 @@
>> >> #include <asm/byteorder.h>
>> >>
>> >> typedef int word_type __attribute__ ((mode (__word__)));
>> >> +typedef int TItype __attribute__ ((mode (TI)));
>> >
>> >Well that's an interesting new compiler attribute.
>>
>>https://stackoverflow.com/questions/4559025/what-does-gcc-attribute-modexx-actually-do
>> >Please use `__mode(TI)` from include/linux/compiler_attributes.h ex.
>> >typedef int TItype __mode(TI);
>> >
>> >>
>> >> #ifdef __BIG_ENDIAN
>> >> struct DWstruct {
>> >> int high, low;
>> >> };
>> >> +
>> >> +struct DWstruct128 {
>> >> + long long high, low;
>> >> +};
>> >> +
>> >> #elif defined(__LITTLE_ENDIAN)
>> >> struct DWstruct {
>> >> int low, high;
>> >> };
>> >> +
>> >> +struct DWstruct128 {
>> >> + long long low, high;
>> >> +};
>> >> +
>> >> #else
>> >> #error I feel sick.
>> >> #endif
>> >> @@ -40,4 +51,9 @@ typedef union {
>> >> long long ll;
>> >> } DWunion;
>> >>
>> >> +typedef union {
>> >> + struct DWstruct128 s;
>> >> + TItype ll;
>> >> +} DWunion128;
>> >> +
>> >> #endif /* __ASM_LIBGCC_H */
>> >> diff --git a/lib/Kconfig b/lib/Kconfig
>> >> index a9e56539bd11..369e10259ea6 100644
>> >> --- a/lib/Kconfig
>> >> +++ b/lib/Kconfig
>> >> @@ -624,6 +624,9 @@ config GENERIC_LIB_ASHRDI3
>> >> config GENERIC_LIB_LSHRDI3
>> >> bool
>> >>
>> >> +config GENERIC_LIB_LSHRTI3
>> >> + bool
>> >> +
>> >> config GENERIC_LIB_MULDI3
>> >> bool
>> >>
>> >> diff --git a/lib/Makefile b/lib/Makefile
>> >> index 4e066120a0d6..42648411f451 100644
>> >> --- a/lib/Makefile
>> >> +++ b/lib/Makefile
>> >> @@ -277,6 +277,7 @@ obj-$(CONFIG_PARMAN) += parman.o
>> >> obj-$(CONFIG_GENERIC_LIB_ASHLDI3) += ashldi3.o
>> >> obj-$(CONFIG_GENERIC_LIB_ASHRDI3) += ashrdi3.o
>> >> obj-$(CONFIG_GENERIC_LIB_LSHRDI3) += lshrdi3.o
>> >> +obj-$(CONFIG_GENERIC_LIB_LSHRTI3) += lshrti3.o
>> >> obj-$(CONFIG_GENERIC_LIB_MULDI3) += muldi3.o
>> >> obj-$(CONFIG_GENERIC_LIB_CMPDI2) += cmpdi2.o
>> >> obj-$(CONFIG_GENERIC_LIB_UCMPDI2) += ucmpdi2.o
>> >> diff --git a/lib/lshrti3.c b/lib/lshrti3.c
>> >> new file mode 100644
>> >> index 000000000000..2d2123bb3030
>> >> --- /dev/null
>> >> +++ b/lib/lshrti3.c
>> >> @@ -0,0 +1,31 @@
>> >> +// SPDX-License-Identifier: GPL-2.0
>> >> +
>> >> +#include <linux/export.h>
>> >> +#include <linux/libgcc.h>
>> >> +
>> >> +long long __lshrti3(long long u, word_type b)
>> >> +{
>> >> + DWunion128 uu, w;
>> >> + word_type bm;
>> >> +
>> >> + if (b == 0)
>> >> + return u;
>> >> +
>> >> + uu.ll = u;
>> >> + bm = 64 - b;
>> >> +
>> >> + if (bm <= 0) {
>> >> + w.s.high = 0;
>> >> + w.s.low = (unsigned long long) uu.s.high >> -bm;
>> >> + } else {
>> >> + const unsigned long long carries =
>> >> + (unsigned long long) uu.s.high << bm;
>> >> + w.s.high = (unsigned long long) uu.s.high >> b;
>> >> + w.s.low = ((unsigned long long) uu.s.low >> b) |
>> >carries;
>> >> + }
>> >> +
>> >> + return w.ll;
>> >> +}
>> >> +#ifndef BUILD_VDSO
>> >> +EXPORT_SYMBOL(__lshrti3);
>> >> +#endif
>> >
>> >I don't think you want this. Maybe that was carried over from
>> >https://lkml.org/lkml/2019/3/15/669
>> >by accident? The above linkage error mentions arch/x86/kvm/x86.o
>> >which I wouldn't expect to be linked into the VDSO image?
>>
>> Or just "u64"...
>
>Are you referring to TItype? It currenty is a 128-bit value.
You're right, so "unsigned __int128"...
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] lib: Add shared copy of __lshrti3 from libgcc
2019-03-15 23:53 ` hpa
@ 2019-03-18 16:38 ` Matthias Kaehlcke
0 siblings, 0 replies; 21+ messages in thread
From: Matthias Kaehlcke @ 2019-03-18 16:38 UTC (permalink / raw)
To: hpa
Cc: Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, LKML, Manoj Gupta, Tiancong Wang,
Stephen Hines, clang-built-linux, Miguel Ojeda
On Fri, Mar 15, 2019 at 04:53:40PM -0700, hpa@zytor.com wrote:
> On March 15, 2019 4:47:01 PM PDT, Matthias Kaehlcke <mka@chromium.org> wrote:
> >On Fri, Mar 15, 2019 at 03:12:08PM -0700, hpa@zytor.com wrote:
> >> On March 15, 2019 3:06:37 PM PDT, Nick Desaulniers
> ><ndesaulniers@google.com> wrote:
> >> >On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <mka@chromium.org>
> >> >wrote:
> >> >>
> >> >> The compiler may emit calls to __lshrti3 from the compiler runtime
> >> >> library, which results in undefined references:
> >> >>
> >> >> arch/x86/kvm/x86.o: In function `mul_u64_u64_shr':
> >> >> include/linux/math64.h:186: undefined reference to `__lshrti3'
> >> >
> >> >Looks like Clang will emit this at -Oz (but not -O2):
> >> >https://godbolt.org/z/w1_2YC
> >> >
> >> >>
> >> >> Add a copy of the __lshrti3 libgcc routine (from gcc v4.9.2).
> >> >
> >> >Has it changed since? If so why not a newer version of libgcc_s?
> >> >
> >> >>
> >> >> Include the function for x86 builds with clang, which is the
> >> >> environment where the above error was observed.
> >> >>
> >> >> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> >> >> ---
> >> >> arch/x86/Kconfig | 1 +
> >> >> include/linux/libgcc.h | 16 ++++++++++++++++
> >> >> lib/Kconfig | 3 +++
> >> >> lib/Makefile | 1 +
> >> >> lib/lshrti3.c | 31 +++++++++++++++++++++++++++++++
> >> >> 5 files changed, 52 insertions(+)
> >> >> create mode 100644 lib/lshrti3.c
> >> >>
> >> >> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> >> >> index c1f9b3cf437c..a5e0d923845d 100644
> >> >> --- a/arch/x86/Kconfig
> >> >> +++ b/arch/x86/Kconfig
> >> >> @@ -105,6 +105,7 @@ config X86
> >> >> select GENERIC_IRQ_PROBE
> >> >> select GENERIC_IRQ_RESERVATION_MODE
> >> >> select GENERIC_IRQ_SHOW
> >> >> + select GENERIC_LIB_LSHRTI3 if CC_IS_CLANG
> >> >> select GENERIC_PENDING_IRQ if SMP
> >> >> select GENERIC_SMP_IDLE_THREAD
> >> >> select GENERIC_STRNCPY_FROM_USER
> >> >> diff --git a/include/linux/libgcc.h b/include/linux/libgcc.h
> >> >> index 32e1e0f4b2d0..a71036471838 100644
> >> >> --- a/include/linux/libgcc.h
> >> >> +++ b/include/linux/libgcc.h
> >> >> @@ -22,15 +22,26 @@
> >> >> #include <asm/byteorder.h>
> >> >>
> >> >> typedef int word_type __attribute__ ((mode (__word__)));
> >> >> +typedef int TItype __attribute__ ((mode (TI)));
> >> >
> >> >Well that's an interesting new compiler attribute.
> >>
> >>https://stackoverflow.com/questions/4559025/what-does-gcc-attribute-modexx-actually-do
> >> >Please use `__mode(TI)` from include/linux/compiler_attributes.h ex.
> >> >typedef int TItype __mode(TI);
> >> >
> >> >>
> >> >> #ifdef __BIG_ENDIAN
> >> >> struct DWstruct {
> >> >> int high, low;
> >> >> };
> >> >> +
> >> >> +struct DWstruct128 {
> >> >> + long long high, low;
> >> >> +};
> >> >> +
> >> >> #elif defined(__LITTLE_ENDIAN)
> >> >> struct DWstruct {
> >> >> int low, high;
> >> >> };
> >> >> +
> >> >> +struct DWstruct128 {
> >> >> + long long low, high;
> >> >> +};
> >> >> +
> >> >> #else
> >> >> #error I feel sick.
> >> >> #endif
> >> >> @@ -40,4 +51,9 @@ typedef union {
> >> >> long long ll;
> >> >> } DWunion;
> >> >>
> >> >> +typedef union {
> >> >> + struct DWstruct128 s;
> >> >> + TItype ll;
> >> >> +} DWunion128;
> >> >> +
> >> >> #endif /* __ASM_LIBGCC_H */
> >> >> diff --git a/lib/Kconfig b/lib/Kconfig
> >> >> index a9e56539bd11..369e10259ea6 100644
> >> >> --- a/lib/Kconfig
> >> >> +++ b/lib/Kconfig
> >> >> @@ -624,6 +624,9 @@ config GENERIC_LIB_ASHRDI3
> >> >> config GENERIC_LIB_LSHRDI3
> >> >> bool
> >> >>
> >> >> +config GENERIC_LIB_LSHRTI3
> >> >> + bool
> >> >> +
> >> >> config GENERIC_LIB_MULDI3
> >> >> bool
> >> >>
> >> >> diff --git a/lib/Makefile b/lib/Makefile
> >> >> index 4e066120a0d6..42648411f451 100644
> >> >> --- a/lib/Makefile
> >> >> +++ b/lib/Makefile
> >> >> @@ -277,6 +277,7 @@ obj-$(CONFIG_PARMAN) += parman.o
> >> >> obj-$(CONFIG_GENERIC_LIB_ASHLDI3) += ashldi3.o
> >> >> obj-$(CONFIG_GENERIC_LIB_ASHRDI3) += ashrdi3.o
> >> >> obj-$(CONFIG_GENERIC_LIB_LSHRDI3) += lshrdi3.o
> >> >> +obj-$(CONFIG_GENERIC_LIB_LSHRTI3) += lshrti3.o
> >> >> obj-$(CONFIG_GENERIC_LIB_MULDI3) += muldi3.o
> >> >> obj-$(CONFIG_GENERIC_LIB_CMPDI2) += cmpdi2.o
> >> >> obj-$(CONFIG_GENERIC_LIB_UCMPDI2) += ucmpdi2.o
> >> >> diff --git a/lib/lshrti3.c b/lib/lshrti3.c
> >> >> new file mode 100644
> >> >> index 000000000000..2d2123bb3030
> >> >> --- /dev/null
> >> >> +++ b/lib/lshrti3.c
> >> >> @@ -0,0 +1,31 @@
> >> >> +// SPDX-License-Identifier: GPL-2.0
> >> >> +
> >> >> +#include <linux/export.h>
> >> >> +#include <linux/libgcc.h>
> >> >> +
> >> >> +long long __lshrti3(long long u, word_type b)
> >> >> +{
> >> >> + DWunion128 uu, w;
> >> >> + word_type bm;
> >> >> +
> >> >> + if (b == 0)
> >> >> + return u;
> >> >> +
> >> >> + uu.ll = u;
> >> >> + bm = 64 - b;
> >> >> +
> >> >> + if (bm <= 0) {
> >> >> + w.s.high = 0;
> >> >> + w.s.low = (unsigned long long) uu.s.high >> -bm;
> >> >> + } else {
> >> >> + const unsigned long long carries =
> >> >> + (unsigned long long) uu.s.high << bm;
> >> >> + w.s.high = (unsigned long long) uu.s.high >> b;
> >> >> + w.s.low = ((unsigned long long) uu.s.low >> b) |
> >> >carries;
> >> >> + }
> >> >> +
> >> >> + return w.ll;
> >> >> +}
> >> >> +#ifndef BUILD_VDSO
> >> >> +EXPORT_SYMBOL(__lshrti3);
> >> >> +#endif
> >> >
> >> >I don't think you want this. Maybe that was carried over from
> >> >https://lkml.org/lkml/2019/3/15/669
> >> >by accident? The above linkage error mentions arch/x86/kvm/x86.o
> >> >which I wouldn't expect to be linked into the VDSO image?
> >>
> >> Or just "u64"...
> >
> >Are you referring to TItype? It currenty is a 128-bit value.
>
> You're right, so "unsigned __int128"...
I liked the idea, however __int128 isn't supported by all platforms:
include/linux/libgcc.h:57:11: error: __int128 is not supported on this target
unsigned __int128 ll;
That's from building the 32-bit VDSO for x86.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] lib: Add shared copy of __lshrti3 from libgcc
2019-03-15 22:06 ` Nick Desaulniers
2019-03-15 22:08 ` hpa
2019-03-15 22:12 ` hpa
@ 2019-03-15 23:29 ` Matthias Kaehlcke
2019-03-18 9:14 ` Peter Zijlstra
3 siblings, 0 replies; 21+ messages in thread
From: Matthias Kaehlcke @ 2019-03-15 23:29 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
H . Peter Anvin, x86, LKML, Manoj Gupta, Tiancong Wang,
Stephen Hines, clang-built-linux, Miguel Ojeda
On Fri, Mar 15, 2019 at 03:06:37PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote:
> On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > The compiler may emit calls to __lshrti3 from the compiler runtime
> > library, which results in undefined references:
> >
> > arch/x86/kvm/x86.o: In function `mul_u64_u64_shr':
> > include/linux/math64.h:186: undefined reference to `__lshrti3'
>
> Looks like Clang will emit this at -Oz (but not -O2):
> https://godbolt.org/z/w1_2YC
>
> >
> > Add a copy of the __lshrti3 libgcc routine (from gcc v4.9.2).
>
> Has it changed since? If so why not a newer version of libgcc_s?
Our compiler folks who maintain this gcc version dug this up for
me. In the gcc sources there is no direct implementation of __lshrti3,
I was told it is somehow derived from __lshrdi3.
> > Include the function for x86 builds with clang, which is the
> > environment where the above error was observed.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> > arch/x86/Kconfig | 1 +
> > include/linux/libgcc.h | 16 ++++++++++++++++
> > lib/Kconfig | 3 +++
> > lib/Makefile | 1 +
> > lib/lshrti3.c | 31 +++++++++++++++++++++++++++++++
> > 5 files changed, 52 insertions(+)
> > create mode 100644 lib/lshrti3.c
> >
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index c1f9b3cf437c..a5e0d923845d 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -105,6 +105,7 @@ config X86
> > select GENERIC_IRQ_PROBE
> > select GENERIC_IRQ_RESERVATION_MODE
> > select GENERIC_IRQ_SHOW
> > + select GENERIC_LIB_LSHRTI3 if CC_IS_CLANG
> > select GENERIC_PENDING_IRQ if SMP
> > select GENERIC_SMP_IDLE_THREAD
> > select GENERIC_STRNCPY_FROM_USER
> > diff --git a/include/linux/libgcc.h b/include/linux/libgcc.h
> > index 32e1e0f4b2d0..a71036471838 100644
> > --- a/include/linux/libgcc.h
> > +++ b/include/linux/libgcc.h
> > @@ -22,15 +22,26 @@
> > #include <asm/byteorder.h>
> >
> > typedef int word_type __attribute__ ((mode (__word__)));
> > +typedef int TItype __attribute__ ((mode (TI)));
>
> Well that's an interesting new compiler attribute.
> https://stackoverflow.com/questions/4559025/what-does-gcc-attribute-modexx-actually-do
> Please use `__mode(TI)` from include/linux/compiler_attributes.h ex.
> typedef int TItype __mode(TI);
ok
> > #ifdef __BIG_ENDIAN
> > struct DWstruct {
> > int high, low;
> > };
> > +
> > +struct DWstruct128 {
> > + long long high, low;
> > +};
> > +
> > #elif defined(__LITTLE_ENDIAN)
> > struct DWstruct {
> > int low, high;
> > };
> > +
> > +struct DWstruct128 {
> > + long long low, high;
> > +};
> > +
> > #else
> > #error I feel sick.
> > #endif
> > @@ -40,4 +51,9 @@ typedef union {
> > long long ll;
> > } DWunion;
> >
> > +typedef union {
> > + struct DWstruct128 s;
> > + TItype ll;
> > +} DWunion128;
> > +
> > #endif /* __ASM_LIBGCC_H */
> > diff --git a/lib/Kconfig b/lib/Kconfig
> > index a9e56539bd11..369e10259ea6 100644
> > --- a/lib/Kconfig
> > +++ b/lib/Kconfig
> > @@ -624,6 +624,9 @@ config GENERIC_LIB_ASHRDI3
> > config GENERIC_LIB_LSHRDI3
> > bool
> >
> > +config GENERIC_LIB_LSHRTI3
> > + bool
> > +
> > config GENERIC_LIB_MULDI3
> > bool
> >
> > diff --git a/lib/Makefile b/lib/Makefile
> > index 4e066120a0d6..42648411f451 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -277,6 +277,7 @@ obj-$(CONFIG_PARMAN) += parman.o
> > obj-$(CONFIG_GENERIC_LIB_ASHLDI3) += ashldi3.o
> > obj-$(CONFIG_GENERIC_LIB_ASHRDI3) += ashrdi3.o
> > obj-$(CONFIG_GENERIC_LIB_LSHRDI3) += lshrdi3.o
> > +obj-$(CONFIG_GENERIC_LIB_LSHRTI3) += lshrti3.o
> > obj-$(CONFIG_GENERIC_LIB_MULDI3) += muldi3.o
> > obj-$(CONFIG_GENERIC_LIB_CMPDI2) += cmpdi2.o
> > obj-$(CONFIG_GENERIC_LIB_UCMPDI2) += ucmpdi2.o
> > diff --git a/lib/lshrti3.c b/lib/lshrti3.c
> > new file mode 100644
> > index 000000000000..2d2123bb3030
> > --- /dev/null
> > +++ b/lib/lshrti3.c
> > @@ -0,0 +1,31 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <linux/export.h>
> > +#include <linux/libgcc.h>
> > +
> > +long long __lshrti3(long long u, word_type b)
This signature matches with the gcc documentation
(https://gcc.gnu.org/onlinedocs/gccint/Integer-library-routines.html),
however the gcc implementation of the function has 128-bit values as
input/output, so I guess to meet gcc's expectations the 'long long's
need to be changed to TItype.
> > +{
> > + DWunion128 uu, w;
> > + word_type bm;
> > +
> > + if (b == 0)
> > + return u;
> > +
> > + uu.ll = u;
> > + bm = 64 - b;
> > +
> > + if (bm <= 0) {
> > + w.s.high = 0;
> > + w.s.low = (unsigned long long) uu.s.high >> -bm;
> > + } else {
> > + const unsigned long long carries =
> > + (unsigned long long) uu.s.high << bm;
> > + w.s.high = (unsigned long long) uu.s.high >> b;
> > + w.s.low = ((unsigned long long) uu.s.low >> b) | carries;
> > + }
> > +
> > + return w.ll;
> > +}
> > +#ifndef BUILD_VDSO
> > +EXPORT_SYMBOL(__lshrti3);
> > +#endif
>
> I don't think you want this. Maybe that was carried over from
> https://lkml.org/lkml/2019/3/15/669
> by accident? The above linkage error mentions arch/x86/kvm/x86.o
> which I wouldn't expect to be linked into the VDSO image?
I put it preemptively, since I know from
https://lkml.org/lkml/2019/3/15/669 that the vDSO build is unhappy
about EXPORT_SYMBOL. Obviously we can also wait until a vDSO needs to
include this file.
Thanks
Matthias
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] lib: Add shared copy of __lshrti3 from libgcc
2019-03-15 22:06 ` Nick Desaulniers
` (2 preceding siblings ...)
2019-03-15 23:29 ` Matthias Kaehlcke
@ 2019-03-18 9:14 ` Peter Zijlstra
2019-03-18 14:43 ` David Laight
3 siblings, 1 reply; 21+ messages in thread
From: Peter Zijlstra @ 2019-03-18 9:14 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Matthias Kaehlcke, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H . Peter Anvin, x86, LKML, Manoj Gupta,
Tiancong Wang, Stephen Hines, clang-built-linux, Miguel Ojeda
On Fri, Mar 15, 2019 at 03:06:37PM -0700, Nick Desaulniers wrote:
> On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > The compiler may emit calls to __lshrti3 from the compiler runtime
> > library, which results in undefined references:
> >
> > arch/x86/kvm/x86.o: In function `mul_u64_u64_shr':
> > include/linux/math64.h:186: undefined reference to `__lshrti3'
>
> Looks like Clang will emit this at -Oz (but not -O2):
> https://godbolt.org/z/w1_2YC
*OMG*, what is that compiler smoking and why do we want that?
It doesn't even do that for "-Os".
So where "-Os" is "Optimize for Sadness" this "-Oz" thing is like a
downright depression. Please just take it out back. Don't enable crap
like this.
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] lib: Add shared copy of __lshrti3 from libgcc
2019-03-18 9:14 ` Peter Zijlstra
@ 2019-03-18 14:43 ` David Laight
2019-03-18 14:54 ` Peter Zijlstra
0 siblings, 1 reply; 21+ messages in thread
From: David Laight @ 2019-03-18 14:43 UTC (permalink / raw)
To: 'Peter Zijlstra', Nick Desaulniers
Cc: Matthias Kaehlcke, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H . Peter Anvin, x86, LKML, Manoj Gupta,
Tiancong Wang, Stephen Hines, clang-built-linux, Miguel Ojeda
From: Peter Zijlstra
> Sent: 18 March 2019 09:14
> On Fri, Mar 15, 2019 at 03:06:37PM -0700, Nick Desaulniers wrote:
> > On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <mka@chromium.org> wrote:
> > >
> > > The compiler may emit calls to __lshrti3 from the compiler runtime
> > > library, which results in undefined references:
> > >
> > > arch/x86/kvm/x86.o: In function `mul_u64_u64_shr':
> > > include/linux/math64.h:186: undefined reference to `__lshrti3'
> >
> > Looks like Clang will emit this at -Oz (but not -O2):
> > https://godbolt.org/z/w1_2YC
>
> *OMG*, what is that compiler smoking and why do we want that?
>
> It doesn't even do that for "-Os".
I like the way it moves %edx to %ecx, then %cl to %ecx and finally %ecx back to %edx.
I'm guessing this is all made worse by the prototype containing 'char' not 'int'.
I'm sure the register tracking gets worse in every version of gcc.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] lib: Add shared copy of __lshrti3 from libgcc
2019-03-18 14:43 ` David Laight
@ 2019-03-18 14:54 ` Peter Zijlstra
0 siblings, 0 replies; 21+ messages in thread
From: Peter Zijlstra @ 2019-03-18 14:54 UTC (permalink / raw)
To: David Laight
Cc: Nick Desaulniers, Matthias Kaehlcke, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H . Peter Anvin,
x86, LKML, Manoj Gupta, Tiancong Wang, Stephen Hines,
clang-built-linux, Miguel Ojeda
On Mon, Mar 18, 2019 at 02:43:41PM +0000, David Laight wrote:
> From: Peter Zijlstra
> > Sent: 18 March 2019 09:14
> > On Fri, Mar 15, 2019 at 03:06:37PM -0700, Nick Desaulniers wrote:
> > > On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <mka@chromium.org> wrote:
> > > >
> > > > The compiler may emit calls to __lshrti3 from the compiler runtime
> > > > library, which results in undefined references:
> > > >
> > > > arch/x86/kvm/x86.o: In function `mul_u64_u64_shr':
> > > > include/linux/math64.h:186: undefined reference to `__lshrti3'
> > >
> > > Looks like Clang will emit this at -Oz (but not -O2):
> > > https://godbolt.org/z/w1_2YC
> >
> > *OMG*, what is that compiler smoking and why do we want that?
> >
> > It doesn't even do that for "-Os".
>
> I like the way it moves %edx to %ecx, then %cl to %ecx and finally %ecx back to %edx.
> I'm guessing this is all made worse by the prototype containing 'char' not 'int'.
>
> I'm sure the register tracking gets worse in every version of gcc.
This is clang, no GCC on that list comes even close to generating
anything as 'brilliant' as that.
^ permalink raw reply [flat|nested] 21+ messages in thread