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@xxxxxxxxxxxxx
