From: Nick Desaulniers <ndesaulniers@google.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: David Laight <David.Laight@aculab.com>,
Helge Deller <deller@gmx.de>,
Nathan Chancellor <nathan@kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-parisc@vger.kernel.org" <linux-parisc@vger.kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Chanho Min <chanho.min@lge.com>,
Geert Uytterhoeven <geert@linux-m68k.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH] lib/clz_ctz.c: Fix __clzdi2() and __ctzdi2() for 32-bit kernels
Date: Mon, 28 Aug 2023 13:14:42 -0700 [thread overview]
Message-ID: <CAKwvOdmUptr9_HsU3fz3pa=Y0-SqgHcKk7SJV7hmeBGaw1C3Aw@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=wieUuOd-aUz0dma6TnZ+TLVk5-ytJiwbMqLpjcvdzzvQg@mail.gmail.com>
On Mon, Aug 28, 2023 at 9:30 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Mon, 28 Aug 2023 at 03:53, David Laight <David.Laight@aculab.com> wrote:
> >
> > From: Linus Torvalds
> > >
> > > We use this:
> > >
> > > static __always_inline unsigned long variable__ffs(unsigned long word)
> > > {
> > > asm("rep; bsf %1,%0"
> > > : "=r" (word)
> > > : "rm" (word));
> > > return word;
> > > }
> > >
> > > for the definition, and it looks like clang royally just screws up
> > > here. Yes, "m" is _allowed_ in that input set, but it damn well
> > > shouldn't be used for something that is already in a register, since
> > > "r" is also allowed, and is the first choice.
> >
> > Why don't we just remove the "m" option?
>
> For this particular case, it would probably be the right thing to do.
> It's sad, though, because gcc handles this correctly, and always has.
>
> And in this particular case, it probably matters not at all.
>
> In many other cases where we have 'rm', we may actually be in the
> situation that having 'rm' (or other cases like "g" that also allows
> immediates) helps because register pressure can be a thing.
>
> It's mostly a thing on 32-bit x86 where you have a lot fewer
> registers, and there we've literally run into situations where we have
> had internal compiler errors because of complex inline asm statements
> running out of registers.
>
> With a simple "one input, one output" case, that just isn't an issue,
> so to work around a clang misfeature we could do it - if somebody
> finds a case where it actually matters (as opposed to "damn, when
> looking at the generted code for a function that we never actually use
> on x86, I noticed that code generation is horrendous").
>
> Linus
Yes; it's a compiler bug, and we will fix it. Then the fix will be an
incentive for folks that care to move to a newer toolchain.
--
Thanks,
~Nick Desaulniers
next prev parent reply other threads:[~2023-08-28 20:16 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-25 19:50 [PATCH] lib/clz_ctz.c: Fix __clzdi2() and __ctzdi2() for 32-bit kernels Helge Deller
2023-08-25 20:25 ` Linus Torvalds
2023-08-25 20:43 ` Linus Torvalds
2023-08-25 21:01 ` Nick Desaulniers
2023-08-25 22:33 ` Bill Wendling
2023-08-25 22:57 ` Bill Wendling
2023-08-25 23:34 ` Linus Torvalds
2023-08-26 0:08 ` Bill Wendling
2023-08-26 0:52 ` Nick Desaulniers
2023-08-26 1:07 ` Linus Torvalds
2023-08-26 3:17 ` Fangrui Song
2023-08-28 7:33 ` Geert Uytterhoeven
2023-08-28 16:24 ` Linus Torvalds
2023-08-28 20:13 ` Nick Desaulniers
2023-08-28 20:09 ` Nick Desaulniers
2023-08-28 20:08 ` Nick Desaulniers
2023-08-28 10:53 ` David Laight
2023-08-28 16:30 ` Linus Torvalds
2023-08-28 20:14 ` Nick Desaulniers [this message]
2023-08-28 20:10 ` Nick Desaulniers
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='CAKwvOdmUptr9_HsU3fz3pa=Y0-SqgHcKk7SJV7hmeBGaw1C3Aw@mail.gmail.com' \
--to=ndesaulniers@google.com \
--cc=David.Laight@aculab.com \
--cc=akpm@linux-foundation.org \
--cc=chanho.min@lge.com \
--cc=deller@gmx.de \
--cc=geert@linux-m68k.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-parisc@vger.kernel.org \
--cc=nathan@kernel.org \
--cc=torvalds@linux-foundation.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).