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'
Add a copy of the __lshrti3 libgcc routine (from gcc v4.9.2).
Include the function for x86 builds with clang, which is the
environment where the above error was observed.
Signed-off-by: Matthias Kaehlcke <[email protected]>
---
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)));
#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
--
2.21.0.360.g471c308f928-goog
On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <[email protected]> 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 <[email protected]>
> ---
> 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?
--
Thanks,
~Nick Desaulniers
On March 15, 2019 3:06:37 PM PDT, Nick Desaulniers <[email protected]> wrote:
>On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <[email protected]>
>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 <[email protected]>
>> ---
>> 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.
On March 15, 2019 3:06:37 PM PDT, Nick Desaulniers <[email protected]> wrote:
>On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <[email protected]>
>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 <[email protected]>
>> ---
>> 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.
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 <[email protected]> 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 <[email protected]>
> > ---
> > 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
Hi Peter,
On Fri, Mar 15, 2019 at 03:08:57PM -0700, [email protected] wrote:
> On March 15, 2019 3:06:37 PM PDT, Nick Desaulniers <[email protected]> wrote:
> >On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <[email protected]>
> >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 <[email protected]>
> >> ---
> >> 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)?
On Fri, Mar 15, 2019 at 03:12:08PM -0700, [email protected] wrote:
> On March 15, 2019 3:06:37 PM PDT, Nick Desaulniers <[email protected]> wrote:
> >On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <[email protected]>
> >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 <[email protected]>
> >> ---
> >> 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.
On March 15, 2019 4:34:10 PM PDT, Matthias Kaehlcke <[email protected]> wrote:
>Hi Peter,
>
>On Fri, Mar 15, 2019 at 03:08:57PM -0700, [email protected] wrote:
>> On March 15, 2019 3:06:37 PM PDT, Nick Desaulniers
><[email protected]> wrote:
>> >On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <[email protected]>
>> >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 <[email protected]>
>> >> ---
>> >> 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.
On March 15, 2019 4:47:01 PM PDT, Matthias Kaehlcke <[email protected]> wrote:
>On Fri, Mar 15, 2019 at 03:12:08PM -0700, [email protected] wrote:
>> On March 15, 2019 3:06:37 PM PDT, Nick Desaulniers
><[email protected]> wrote:
>> >On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <[email protected]>
>> >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 <[email protected]>
>> >> ---
>> >> 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.
On Fri, Mar 15, 2019 at 03:06:37PM -0700, Nick Desaulniers wrote:
> On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <[email protected]> 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.
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 <[email protected]> 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)
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 <[email protected]> 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.
On Fri, Mar 15, 2019 at 04:53:40PM -0700, [email protected] wrote:
> On March 15, 2019 4:47:01 PM PDT, Matthias Kaehlcke <[email protected]> wrote:
> >On Fri, Mar 15, 2019 at 03:12:08PM -0700, [email protected] wrote:
> >> On March 15, 2019 3:06:37 PM PDT, Nick Desaulniers
> ><[email protected]> wrote:
> >> >On Fri, Mar 15, 2019 at 1:54 PM Matthias Kaehlcke <[email protected]>
> >> >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 <[email protected]>
> >> >> ---
> >> >> 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.
On Fri, Mar 15, 2019 at 01:54:50PM -0700, Matthias Kaehlcke 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'
>
> Add a copy of the __lshrti3 libgcc routine (from gcc v4.9.2).
>
> Include the function for x86 builds with clang, which is the
> environment where the above error was observed.
>
> Signed-off-by: Matthias Kaehlcke <[email protected]>
With "Revert "kbuild: use -Oz instead of -Os when using clang"
(https://lore.kernel.org/patchwork/patch/1051932/) the above
error is fixed, a few comments inline for if the patch is
resurrected in the future because __lshrti3 is emitted in a
different context.
> 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)));
Consider using __int128 instead. Definition and use need a
'defined(__SIZEOF_INT128__)' guard (similar for mode (TI)), since
these 128 bit types aren't supported on all platforms.
> #ifdef __BIG_ENDIAN
> struct DWstruct {
> int high, low;
> };
> +
> +struct DWstruct128 {
> + long long high, low;
> +};
This struct isn't needed, struct DWstruct can be used.
> 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)
use TItype for input/output, which is what gcc does, though the above
matches the interface in the documentation.
> +{
> + 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;
include <linux/types.h> and use u64 instead of unsigned long long.
On March 18, 2019 2:31:13 PM PDT, Matthias Kaehlcke <[email protected]> wrote:
>On Fri, Mar 15, 2019 at 01:54:50PM -0700, Matthias Kaehlcke 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'
>>
>> Add a copy of the __lshrti3 libgcc routine (from gcc v4.9.2).
>>
>> Include the function for x86 builds with clang, which is the
>> environment where the above error was observed.
>>
>> Signed-off-by: Matthias Kaehlcke <[email protected]>
>
>With "Revert "kbuild: use -Oz instead of -Os when using clang"
>(https://lore.kernel.org/patchwork/patch/1051932/) the above
>error is fixed, a few comments inline for if the patch is
>resurrected in the future because __lshrti3 is emitted in a
>different context.
>
>> 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)));
>
>Consider using __int128 instead. Definition and use need a
>'defined(__SIZEOF_INT128__)' guard (similar for mode (TI)), since
>these 128 bit types aren't supported on all platforms.
>
>> #ifdef __BIG_ENDIAN
>> struct DWstruct {
>> int high, low;
>> };
>> +
>> +struct DWstruct128 {
>> + long long high, low;
>> +};
>
>This struct isn't needed, struct DWstruct can be used.
>
>> 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)
>
>use TItype for input/output, which is what gcc does, though the above
>matches the interface in the documentation.
>
>> +{
>> + 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;
>
>include <linux/types.h> and use u64 instead of unsigned long long.
Ok, now I'm really puzzled.
How could we need a 128-bit shift when the prototype only has 64 bits of input?!
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
On Mon, Mar 18, 2019 at 02:50:44PM -0700, [email protected] wrote:
> On March 18, 2019 2:31:13 PM PDT, Matthias Kaehlcke <[email protected]> wrote:
> >On Fri, Mar 15, 2019 at 01:54:50PM -0700, Matthias Kaehlcke 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'
> >>
> >> Add a copy of the __lshrti3 libgcc routine (from gcc v4.9.2).
> >>
> >> Include the function for x86 builds with clang, which is the
> >> environment where the above error was observed.
> >>
> >> Signed-off-by: Matthias Kaehlcke <[email protected]>
> >
> >With "Revert "kbuild: use -Oz instead of -Os when using clang"
> >(https://lore.kernel.org/patchwork/patch/1051932/) the above
> >error is fixed, a few comments inline for if the patch is
> >resurrected in the future because __lshrti3 is emitted in a
> >different context.
> >
> >> 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)));
> >
> >Consider using __int128 instead. Definition and use need a
> >'defined(__SIZEOF_INT128__)' guard (similar for mode (TI)), since
> >these 128 bit types aren't supported on all platforms.
> >
> >> #ifdef __BIG_ENDIAN
> >> struct DWstruct {
> >> int high, low;
> >> };
> >> +
> >> +struct DWstruct128 {
> >> + long long high, low;
> >> +};
> >
> >This struct isn't needed, struct DWstruct can be used.
> >
> >> 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)
> >
> >use TItype for input/output, which is what gcc does, though the above
> >matches the interface in the documentation.
> >
> >> +{
> >> + 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;
> >
> >include <linux/types.h> and use u64 instead of unsigned long long.
>
> Ok, now I'm really puzzled.
>
> How could we need a 128-bit shift when the prototype only has 64 bits of input?!
Good question, this is the code from libgcc:
TItype
__lshrti3 (TItype u, shift_count_type b)
{
if (b == 0)
return u;
const DWunion uu = {.ll = u};
const shift_count_type bm = (8 * (8)) - b;
DWunion w;
if (bm <= 0)
{
w.s.high = 0;
w.s.low = (UDItype) uu.s.high >> -bm;
}
else
{
const UDItype carries = (UDItype) uu.s.high << bm;
w.s.high = (UDItype) uu.s.high >> b;
w.s.low = ((UDItype) uu.s.low >> b) | carries;
}
return w.ll;
}
My compiler knowledge is limited, my guess is that the function is a
generic implementation, and while a long long is 64-bit wide under
Linux it could be 128-bit on other platforms.
On March 18, 2019 3:16:39 PM PDT, Matthias Kaehlcke <[email protected]> wrote:
>On Mon, Mar 18, 2019 at 02:50:44PM -0700, [email protected] wrote:
>> On March 18, 2019 2:31:13 PM PDT, Matthias Kaehlcke
><[email protected]> wrote:
>> >On Fri, Mar 15, 2019 at 01:54:50PM -0700, Matthias Kaehlcke 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'
>> >>
>> >> Add a copy of the __lshrti3 libgcc routine (from gcc v4.9.2).
>> >>
>> >> Include the function for x86 builds with clang, which is the
>> >> environment where the above error was observed.
>> >>
>> >> Signed-off-by: Matthias Kaehlcke <[email protected]>
>> >
>> >With "Revert "kbuild: use -Oz instead of -Os when using clang"
>> >(https://lore.kernel.org/patchwork/patch/1051932/) the above
>> >error is fixed, a few comments inline for if the patch is
>> >resurrected in the future because __lshrti3 is emitted in a
>> >different context.
>> >
>> >> 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)));
>> >
>> >Consider using __int128 instead. Definition and use need a
>> >'defined(__SIZEOF_INT128__)' guard (similar for mode (TI)), since
>> >these 128 bit types aren't supported on all platforms.
>> >
>> >> #ifdef __BIG_ENDIAN
>> >> struct DWstruct {
>> >> int high, low;
>> >> };
>> >> +
>> >> +struct DWstruct128 {
>> >> + long long high, low;
>> >> +};
>> >
>> >This struct isn't needed, struct DWstruct can be used.
>> >
>> >> 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)
>> >
>> >use TItype for input/output, which is what gcc does, though the
>above
>> >matches the interface in the documentation.
>> >
>> >> +{
>> >> + 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;
>> >
>> >include <linux/types.h> and use u64 instead of unsigned long long.
>>
>> Ok, now I'm really puzzled.
>>
>> How could we need a 128-bit shift when the prototype only has 64 bits
>of input?!
>
>Good question, this is the code from libgcc:
>
>TItype
>__lshrti3 (TItype u, shift_count_type b)
>{
> if (b == 0)
> return u;
>
> const DWunion uu = {.ll = u};
> const shift_count_type bm = (8 * (8)) - b;
> DWunion w;
>
> if (bm <= 0)
> {
> w.s.high = 0;
> w.s.low = (UDItype) uu.s.high >> -bm;
> }
> else
> {
> const UDItype carries = (UDItype) uu.s.high << bm;
>
> w.s.high = (UDItype) uu.s.high >> b;
> w.s.low = ((UDItype) uu.s.low >> b) | carries;
> }
>
> return w.ll;
>}
>
>
>My compiler knowledge is limited, my guess is that the function is a
>generic implementation, and while a long long is 64-bit wide under
>Linux it could be 128-bit on other platforms.
Yes, long long is just plain wrong.
How could we end up calling this function on 32 bits?!
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
On Mon, Mar 18, 2019 at 04:44:03PM -0700, [email protected] wrote:
> On March 18, 2019 3:16:39 PM PDT, Matthias Kaehlcke <[email protected]> wrote:
> >On Mon, Mar 18, 2019 at 02:50:44PM -0700, [email protected] wrote:
> >> On March 18, 2019 2:31:13 PM PDT, Matthias Kaehlcke
> ><[email protected]> wrote:
> >> >On Fri, Mar 15, 2019 at 01:54:50PM -0700, Matthias Kaehlcke 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'
> >> >>
> >> >> Add a copy of the __lshrti3 libgcc routine (from gcc v4.9.2).
> >> >>
> >> >> Include the function for x86 builds with clang, which is the
> >> >> environment where the above error was observed.
> >> >>
> >> >> Signed-off-by: Matthias Kaehlcke <[email protected]>
> >> >
> >> >With "Revert "kbuild: use -Oz instead of -Os when using clang"
> >> >(https://lore.kernel.org/patchwork/patch/1051932/) the above
> >> >error is fixed, a few comments inline for if the patch is
> >> >resurrected in the future because __lshrti3 is emitted in a
> >> >different context.
> >> >
> >> >> 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)));
> >> >
> >> >Consider using __int128 instead. Definition and use need a
> >> >'defined(__SIZEOF_INT128__)' guard (similar for mode (TI)), since
> >> >these 128 bit types aren't supported on all platforms.
> >> >
> >> >> #ifdef __BIG_ENDIAN
> >> >> struct DWstruct {
> >> >> int high, low;
> >> >> };
> >> >> +
> >> >> +struct DWstruct128 {
> >> >> + long long high, low;
> >> >> +};
> >> >
> >> >This struct isn't needed, struct DWstruct can be used.
> >> >
> >> >> 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)
> >> >
> >> >use TItype for input/output, which is what gcc does, though the
> >above
> >> >matches the interface in the documentation.
> >> >
> >> >> +{
> >> >> + 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;
> >> >
> >> >include <linux/types.h> and use u64 instead of unsigned long long.
> >>
> >> Ok, now I'm really puzzled.
> >>
> >> How could we need a 128-bit shift when the prototype only has 64 bits
> >of input?!
> >
> >Good question, this is the code from libgcc:
> >
> >TItype
> >__lshrti3 (TItype u, shift_count_type b)
> >{
> > if (b == 0)
> > return u;
> >
> > const DWunion uu = {.ll = u};
> > const shift_count_type bm = (8 * (8)) - b;
> > DWunion w;
> >
> > if (bm <= 0)
> > {
> > w.s.high = 0;
> > w.s.low = (UDItype) uu.s.high >> -bm;
> > }
> > else
> > {
> > const UDItype carries = (UDItype) uu.s.high << bm;
> >
> > w.s.high = (UDItype) uu.s.high >> b;
> > w.s.low = ((UDItype) uu.s.low >> b) | carries;
> > }
> >
> > return w.ll;
> >}
> >
> >
> >My compiler knowledge is limited, my guess is that the function is a
> >generic implementation, and while a long long is 64-bit wide under
> >Linux it could be 128-bit on other platforms.
>
> Yes, long long is just plain wrong.
>
> How could we end up calling this function on 32 bits?!
We didn't, in this case the function is called in 64-bit code
(arch/x86/kvm/x86.o: In function `mul_u64_u64_shr'), for the 32-bit
vDSO it was __lshrdi3.
On March 18, 2019 4:52:19 PM PDT, Matthias Kaehlcke <[email protected]> wrote:
>On Mon, Mar 18, 2019 at 04:44:03PM -0700, [email protected] wrote:
>> On March 18, 2019 3:16:39 PM PDT, Matthias Kaehlcke
><[email protected]> wrote:
>> >On Mon, Mar 18, 2019 at 02:50:44PM -0700, [email protected] wrote:
>> >> On March 18, 2019 2:31:13 PM PDT, Matthias Kaehlcke
>> ><[email protected]> wrote:
>> >> >On Fri, Mar 15, 2019 at 01:54:50PM -0700, Matthias Kaehlcke
>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'
>> >> >>
>> >> >> Add a copy of the __lshrti3 libgcc routine (from gcc v4.9.2).
>> >> >>
>> >> >> Include the function for x86 builds with clang, which is the
>> >> >> environment where the above error was observed.
>> >> >>
>> >> >> Signed-off-by: Matthias Kaehlcke <[email protected]>
>> >> >
>> >> >With "Revert "kbuild: use -Oz instead of -Os when using clang"
>> >> >(https://lore.kernel.org/patchwork/patch/1051932/) the above
>> >> >error is fixed, a few comments inline for if the patch is
>> >> >resurrected in the future because __lshrti3 is emitted in a
>> >> >different context.
>> >> >
>> >> >> 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)));
>> >> >
>> >> >Consider using __int128 instead. Definition and use need a
>> >> >'defined(__SIZEOF_INT128__)' guard (similar for mode (TI)),
>since
>> >> >these 128 bit types aren't supported on all platforms.
>> >> >
>> >> >> #ifdef __BIG_ENDIAN
>> >> >> struct DWstruct {
>> >> >> int high, low;
>> >> >> };
>> >> >> +
>> >> >> +struct DWstruct128 {
>> >> >> + long long high, low;
>> >> >> +};
>> >> >
>> >> >This struct isn't needed, struct DWstruct can be used.
>> >> >
>> >> >> 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)
>> >> >
>> >> >use TItype for input/output, which is what gcc does, though the
>> >above
>> >> >matches the interface in the documentation.
>> >> >
>> >> >> +{
>> >> >> + 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;
>> >> >
>> >> >include <linux/types.h> and use u64 instead of unsigned long
>long.
>> >>
>> >> Ok, now I'm really puzzled.
>> >>
>> >> How could we need a 128-bit shift when the prototype only has 64
>bits
>> >of input?!
>> >
>> >Good question, this is the code from libgcc:
>> >
>> >TItype
>> >__lshrti3 (TItype u, shift_count_type b)
>> >{
>> > if (b == 0)
>> > return u;
>> >
>> > const DWunion uu = {.ll = u};
>> > const shift_count_type bm = (8 * (8)) - b;
>> > DWunion w;
>> >
>> > if (bm <= 0)
>> > {
>> > w.s.high = 0;
>> > w.s.low = (UDItype) uu.s.high >> -bm;
>> > }
>> > else
>> > {
>> > const UDItype carries = (UDItype) uu.s.high << bm;
>> >
>> > w.s.high = (UDItype) uu.s.high >> b;
>> > w.s.low = ((UDItype) uu.s.low >> b) | carries;
>> > }
>> >
>> > return w.ll;
>> >}
>> >
>> >
>> >My compiler knowledge is limited, my guess is that the function is a
>> >generic implementation, and while a long long is 64-bit wide under
>> >Linux it could be 128-bit on other platforms.
>>
>> Yes, long long is just plain wrong.
>>
>> How could we end up calling this function on 32 bits?!
>
>We didn't, in this case the function is called in 64-bit code
>(arch/x86/kvm/x86.o: In function `mul_u64_u64_shr'), for the 32-bit
>vDSO it was __lshrdi3.
That makes more sense.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
On March 18, 2019 4:52:19 PM PDT, Matthias Kaehlcke <[email protected]> wrote:
>On Mon, Mar 18, 2019 at 04:44:03PM -0700, [email protected] wrote:
>> On March 18, 2019 3:16:39 PM PDT, Matthias Kaehlcke
><[email protected]> wrote:
>> >On Mon, Mar 18, 2019 at 02:50:44PM -0700, [email protected] wrote:
>> >> On March 18, 2019 2:31:13 PM PDT, Matthias Kaehlcke
>> ><[email protected]> wrote:
>> >> >On Fri, Mar 15, 2019 at 01:54:50PM -0700, Matthias Kaehlcke
>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'
>> >> >>
>> >> >> Add a copy of the __lshrti3 libgcc routine (from gcc v4.9.2).
>> >> >>
>> >> >> Include the function for x86 builds with clang, which is the
>> >> >> environment where the above error was observed.
>> >> >>
>> >> >> Signed-off-by: Matthias Kaehlcke <[email protected]>
>> >> >
>> >> >With "Revert "kbuild: use -Oz instead of -Os when using clang"
>> >> >(https://lore.kernel.org/patchwork/patch/1051932/) the above
>> >> >error is fixed, a few comments inline for if the patch is
>> >> >resurrected in the future because __lshrti3 is emitted in a
>> >> >different context.
>> >> >
>> >> >> 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)));
>> >> >
>> >> >Consider using __int128 instead. Definition and use need a
>> >> >'defined(__SIZEOF_INT128__)' guard (similar for mode (TI)),
>since
>> >> >these 128 bit types aren't supported on all platforms.
>> >> >
>> >> >> #ifdef __BIG_ENDIAN
>> >> >> struct DWstruct {
>> >> >> int high, low;
>> >> >> };
>> >> >> +
>> >> >> +struct DWstruct128 {
>> >> >> + long long high, low;
>> >> >> +};
>> >> >
>> >> >This struct isn't needed, struct DWstruct can be used.
>> >> >
>> >> >> 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)
>> >> >
>> >> >use TItype for input/output, which is what gcc does, though the
>> >above
>> >> >matches the interface in the documentation.
>> >> >
>> >> >> +{
>> >> >> + 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;
>> >> >
>> >> >include <linux/types.h> and use u64 instead of unsigned long
>long.
>> >>
>> >> Ok, now I'm really puzzled.
>> >>
>> >> How could we need a 128-bit shift when the prototype only has 64
>bits
>> >of input?!
>> >
>> >Good question, this is the code from libgcc:
>> >
>> >TItype
>> >__lshrti3 (TItype u, shift_count_type b)
>> >{
>> > if (b == 0)
>> > return u;
>> >
>> > const DWunion uu = {.ll = u};
>> > const shift_count_type bm = (8 * (8)) - b;
>> > DWunion w;
>> >
>> > if (bm <= 0)
>> > {
>> > w.s.high = 0;
>> > w.s.low = (UDItype) uu.s.high >> -bm;
>> > }
>> > else
>> > {
>> > const UDItype carries = (UDItype) uu.s.high << bm;
>> >
>> > w.s.high = (UDItype) uu.s.high >> b;
>> > w.s.low = ((UDItype) uu.s.low >> b) | carries;
>> > }
>> >
>> > return w.ll;
>> >}
>> >
>> >
>> >My compiler knowledge is limited, my guess is that the function is a
>> >generic implementation, and while a long long is 64-bit wide under
>> >Linux it could be 128-bit on other platforms.
>>
>> Yes, long long is just plain wrong.
>>
>> How could we end up calling this function on 32 bits?!
>
>We didn't, in this case the function is called in 64-bit code
>(arch/x86/kvm/x86.o: In function `mul_u64_u64_shr'), for the 32-bit
>vDSO it was __lshrdi3.
Again, for 64 bits we can include libgcc in the link (with --no-whole-archive).
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
On Mon, Mar 18, 2019 at 04:55:38PM -0700, [email protected] wrote:
> On March 18, 2019 4:52:19 PM PDT, Matthias Kaehlcke <[email protected]> wrote:
> >On Mon, Mar 18, 2019 at 04:44:03PM -0700, [email protected] wrote:
> >> On March 18, 2019 3:16:39 PM PDT, Matthias Kaehlcke
> ><[email protected]> wrote:
> >> >On Mon, Mar 18, 2019 at 02:50:44PM -0700, [email protected] wrote:
> >> >> On March 18, 2019 2:31:13 PM PDT, Matthias Kaehlcke
> >> ><[email protected]> wrote:
> >> >> >On Fri, Mar 15, 2019 at 01:54:50PM -0700, Matthias Kaehlcke
> >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'
> >> >> >>
> >> >> >> Add a copy of the __lshrti3 libgcc routine (from gcc v4.9.2).
> >> >> >>
> >> >> >> Include the function for x86 builds with clang, which is the
> >> >> >> environment where the above error was observed.
> >> >> >>
> >> >> >> Signed-off-by: Matthias Kaehlcke <[email protected]>
> >> >> >
> >> >> >With "Revert "kbuild: use -Oz instead of -Os when using clang"
> >> >> >(https://lore.kernel.org/patchwork/patch/1051932/) the above
> >> >> >error is fixed, a few comments inline for if the patch is
> >> >> >resurrected in the future because __lshrti3 is emitted in a
> >> >> >different context.
> >> >> >
> >> >> >> 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)));
> >> >> >
> >> >> >Consider using __int128 instead. Definition and use need a
> >> >> >'defined(__SIZEOF_INT128__)' guard (similar for mode (TI)),
> >since
> >> >> >these 128 bit types aren't supported on all platforms.
> >> >> >
> >> >> >> #ifdef __BIG_ENDIAN
> >> >> >> struct DWstruct {
> >> >> >> int high, low;
> >> >> >> };
> >> >> >> +
> >> >> >> +struct DWstruct128 {
> >> >> >> + long long high, low;
> >> >> >> +};
> >> >> >
> >> >> >This struct isn't needed, struct DWstruct can be used.
> >> >> >
> >> >> >> 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)
> >> >> >
> >> >> >use TItype for input/output, which is what gcc does, though the
> >> >above
> >> >> >matches the interface in the documentation.
> >> >> >
> >> >> >> +{
> >> >> >> + 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;
> >> >> >
> >> >> >include <linux/types.h> and use u64 instead of unsigned long
> >long.
> >> >>
> >> >> Ok, now I'm really puzzled.
> >> >>
> >> >> How could we need a 128-bit shift when the prototype only has 64
> >bits
> >> >of input?!
> >> >
> >> >Good question, this is the code from libgcc:
> >> >
> >> >TItype
> >> >__lshrti3 (TItype u, shift_count_type b)
> >> >{
> >> > if (b == 0)
> >> > return u;
> >> >
> >> > const DWunion uu = {.ll = u};
> >> > const shift_count_type bm = (8 * (8)) - b;
> >> > DWunion w;
> >> >
> >> > if (bm <= 0)
> >> > {
> >> > w.s.high = 0;
> >> > w.s.low = (UDItype) uu.s.high >> -bm;
> >> > }
> >> > else
> >> > {
> >> > const UDItype carries = (UDItype) uu.s.high << bm;
> >> >
> >> > w.s.high = (UDItype) uu.s.high >> b;
> >> > w.s.low = ((UDItype) uu.s.low >> b) | carries;
> >> > }
> >> >
> >> > return w.ll;
> >> >}
> >> >
> >> >
> >> >My compiler knowledge is limited, my guess is that the function is a
> >> >generic implementation, and while a long long is 64-bit wide under
> >> >Linux it could be 128-bit on other platforms.
> >>
> >> Yes, long long is just plain wrong.
> >>
> >> How could we end up calling this function on 32 bits?!
> >
> >We didn't, in this case the function is called in 64-bit code
> >(arch/x86/kvm/x86.o: In function `mul_u64_u64_shr'), for the 32-bit
> >vDSO it was __lshrdi3.
>
> Again, for 64 bits we can include libgcc in the link (with --no-whole-archive).
I gave this a quick try but ended up with:
VDSO arch/x86/entry/vdso/vdso64.so.dbg
/usr/x86_64-pc-linux-gnu/x86_64-cros-linux-gnu/binutils-bin/2.27.0/ld.bfd.real:
cannot find -lgcc
I guess the solution is to specify the correct directory in LDPATH,
but not sure where that would be coming from.
TBH I'd prefer to leave this to someone more fluent with binutils, I'm
not particularily efficient working in this area.