Messages in this thread | | | Subject | Re: [PATCH] LoongArch: Provide __lshrti3, __ashrti3, and __ashrti3 | From | Xi Ruoyao <> | Date | Sat, 27 Apr 2024 12:13:25 +0800 |
| |
On Sat, 2024-04-27 at 12:00 +0800, Xi Ruoyao wrote: > 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);
Whoops. This is still incorrect for LA32. On LA32 an "int128" (if it ever exists) should be passed as a pointer, but this is passing it in 4 GPRs. So if we want to keep the prototype correct we need to either use "struct { u64 lo, hi; }" in the parameter list too, or guard it with #ifdef CONFIG_64BIT.
So to me checking CONFIG_ARCH_SUPPORTS_INT128 is just easier.
If you insists on not checking CONFIG_ARCH_SUPPORTS_INT128 I'll just use an incorrect prototype like RISC-V but put a comment here, like:
/* The prototypes are incorrect but this file is only used by modpost which does not care. */ long long __ashlti3(long long a, int b); long long __ashrti3(long long a, int b); long long __lshrti3(long long a, int b);
How do you think?
-- Xi Ruoyao <xry111@xry111.site> School of Aerospace Science and Technology, Xidian University
| |