Messages in this thread | | | From | Huacai Chen <> | Date | Sat, 27 Apr 2024 15:20:55 +0800 | Subject | Re: [PATCH] LoongArch: Provide __lshrti3, __ashrti3, and __ashrti3 |
| |
On Sat, Apr 27, 2024 at 12:13 PM Xi Ruoyao <xry111@xry111.site> wrote: > > 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? OK, then just keep the original status.
Huacai > > -- > Xi Ruoyao <xry111@xry111.site> > School of Aerospace Science and Technology, Xidian University >
| |