Re: [PATCH] LoongArch: Provide __lshrti3, __ashrti3, and __ashrti3

From: Xi Ruoyao
Date: Sat Apr 27 2024 - 00:00:42 EST


On Sat, 2024-04-27 at 10:50 +0800, Huacai Chen wrote:
> Hi, Ruoyao,
>
> I don't think #ifdef CONFIG_ARCH_SUPPORTS_INT128 is needed here.
> S390/ARM64/RISCV all built it unconditionally.

The problem here is RISCV and ARM64 are using an incorrect prototype for
these functions in asm-prototypes.h:

long long __lshrti3(long long a, int b);
long long __ashrti3(long long a, int b);
long long __ashlti3(long long a, int b);

where "long long" is not 128-bit. Despite this seems working for RISC-V
and ARM64 I really dislike it.

S390 seems assuming CONFIG_ARCH_SUPPORTS_INT128 is always true, but I
don't think we can assume it too (at least it'll likely to be false for
LA32, so doing so will cause trouble when we add LA32 support).

So if we don't want to check CONFIG_ARCH_SUPPORTS_INT128 and still use a
correct prototype, we'll do:

diff --git a/arch/loongarch/include/asm/asm-prototypes.h b/arch/loongarch/include/asm/asm-prototypes.h
index 51f224bcfc65..0a57db01116d 100644
--- a/arch/loongarch/include/asm/asm-prototypes.h
+++ b/arch/loongarch/include/asm/asm-prototypes.h
@@ -7,8 +7,6 @@
#include <asm/ftrace.h>
#include <asm-generic/asm-prototypes.h>

-#ifdef CONFIG_ARCH_SUPPORTS_INT128
-__int128_t __ashlti3(__int128_t a, int b);
-__int128_t __ashrti3(__int128_t a, int b);
-__int128_t __lshrti3(__int128_t a, int b);
-#endif
+struct { u64 lo, hi; } __ashlti3(u64 lo, u64 hi, int b);
+struct { u64 lo, hi; } __ashrti3(u64 lo, u64 hi, int b);
+struct { u64 lo, hi; } __lshrti3(u64 lo, u64 hi, int b);
diff --git a/arch/loongarch/lib/Makefile b/arch/loongarch/lib/Makefile
index f61af161f16e..23600c128e05 100644
--- a/arch/loongarch/lib/Makefile
+++ b/arch/loongarch/lib/Makefile
@@ -10,4 +10,4 @@ obj-$(CONFIG_CPU_HAS_LSX) += xor_simd.o xor_simd_glue.o

obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o

-obj-$(CONFIG_ARCH_SUPPORTS_INT128) += tishift.o
+obj-$(CONFIG_64BIT) += tishift.o

Is this really better than checking CONFIG_ARCH_SUPPORTS_INT128? I
don't know...

--
Xi Ruoyao <xry111@xxxxxxxxxxx>
School of Aerospace Science and Technology, Xidian University