Re: clear_bit_unlock_is_negative_byte

[Brad Boyer <flar@allandria.com>]

On Fri, Jul 21, 2023 at 08:34:55AM +0200, Andreas Schwab wrote:
> On Jul 20 2023, Matthew Wilcox wrote:
> 
> > +static inline bool clear_bit_unlock_is_negative_byte(unsigned int nr,
> > +               volatile unsigned long *p)
> > +{
> > +       char result;
> > +       char mask = 1 << nr;    /* nr guaranteed to be < 7 */
> > +
> > +       __asm__ __volatile__ ("eori %1, %2; smi %0"
> 
> Why are you using XOR if you want to clear a bit?  If it operates on a
> byte, why does it receive a pointer to long?

I presume the thing with byte is to try to save an extension word.
Using a long immediate argument makes the instruction take 6 bytes
instead of 4. Since we (apparently?) know we don't want to modify
the other three bytes, we can safely take shortcuts like that.
However, I would agree that using XOR here seems wrong. It
fundamentally presumes the bit was previously set. The other
architectures I checked with asm versions of this look like they
do the equivalent of andi with the inverted mask. We are also
presuming that nr is always a compile time constant, but it looks
like that should be true? If it can be immediate, the inversion
would be done by the compiler. The powerpc version doesn't appear to
make either assumption. It looks like it's loading and storing a
"word" (32-bit in that case) to do this. However, the instructions
it's using don't even have byte equivalents.

	Brad Boyer
	flar@allandria.com