From: Huacai Chen <chenhuacai@kernel.org>
To: Xi Ruoyao <xry111@xry111.site>
Cc: WANG Xuerui <kernel@xen0n.name>,
loongarch@lists.linux.dev, linux-kernel@vger.kernel.org,
Tiezhu Yang <yangtiezhu@loongson.cn>
Subject: Re: [PATCH] LoongArch: Provide __lshrti3, __ashrti3, and __ashrti3
Date: Sat, 27 Apr 2024 15:20:55 +0800 [thread overview]
Message-ID: <CAAhV-H5vrCrBFhDKjTXchCGABWHT6Wxz1JK6hNsarOwp+vfHpg@mail.gmail.com> (raw)
In-Reply-To: <56414029e179d219442bde9b8eae81fa3e3ceec4.camel@xry111.site>
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
>
next prev parent reply other threads:[~2024-04-27 7:21 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-26 12:14 [PATCH] LoongArch: Provide __lshrti3, __ashrti3, and __ashrti3 Xi Ruoyao
2024-04-27 2:50 ` Huacai Chen
2024-04-27 4:00 ` Xi Ruoyao
2024-04-27 4:13 ` Xi Ruoyao
2024-04-27 7:20 ` Huacai Chen [this message]
2024-04-27 7:49 ` Xi Ruoyao
2024-04-27 8:39 ` Huacai Chen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAAhV-H5vrCrBFhDKjTXchCGABWHT6Wxz1JK6hNsarOwp+vfHpg@mail.gmail.com \
--to=chenhuacai@kernel.org \
--cc=kernel@xen0n.name \
--cc=linux-kernel@vger.kernel.org \
--cc=loongarch@lists.linux.dev \
--cc=xry111@xry111.site \
--cc=yangtiezhu@loongson.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.