public inbox for linux-parisc@vger.kernel.org
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Linus Torvalds' <torvalds@linux-foundation.org>,
	Helge Deller <deller@gmx.de>,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>
Cc: "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 10:53:12 +0000	[thread overview]
Message-ID: <cc5f4f5701674b96b0009b6b9b257cc8@AcuMS.aculab.com> (raw)
In-Reply-To: <CAHk-=whVvD05T0yD5DQj803uETLD6qDq-Vx-SiLPcrL=eO77LQ@mail.gmail.com>

From: Linus Torvalds
> Sent: 25 August 2023 21:43
....
> Clang turns this:
> 
>         return __ffs64(val);
> 
> into this horror:
> 
>         pushq   %rax
>         movq    %rdi, (%rsp)
>         #APP
>         rep
>         bsfq    (%rsp), %rax
>         #NO_APP
>         popq    %rcx
> 
> which is just incredibly broken on so many levels. It *should* be a
> single instruction, like gcc does:
> 
>         rep; bsf %rdi,%rax      # tmp87, word
> 
> but clang decides that it really wants to put the argument on the
> stack, and apparently also wants to do that nonsensical stack
> alignment thing to make things even worse.
> 
> 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?

Pretty much the only time it will be worse is it the value
is in memory and loading it into a register causes a spill
to stack.

While it is possible to generate code where that happens it
is pretty unlikely.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

  parent reply	other threads:[~2023-08-28 11:00 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 [this message]
2023-08-28 16:30       ` Linus Torvalds
2023-08-28 20:14         ` Nick Desaulniers
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=cc5f4f5701674b96b0009b6b9b257cc8@AcuMS.aculab.com \
    --to=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=ndesaulniers@google.com \
    --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