| Message ID | 20250319-riscv-swab-v2-2-d53b6d6ab915@iencinas.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Implement endianness swap macros for RISC-V | expand |
| Context | Check | Description |
|---|---|---|
| bjorn/pre-ci_am | success | Success |
On Wed, Mar 19, 2025 at 10:09:46PM +0100, Ignacio Encinas wrote: > +#define ARCH_SWAB(size) \ > +static __always_inline unsigned long __arch_swab##size(__u##size value) \ > +{ \ > + unsigned long x = value; \ > + \ > + asm goto(ALTERNATIVE("j %l[legacy]", "nop", 0, \ > + RISCV_ISA_EXT_ZBB, 1) \ > + :::: legacy); \ Is there a reason to use this instead of riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) which seems to do the same thing, including using a static branch? - Eric
On 21/3/25 4:37, Eric Biggers wrote: > On Wed, Mar 19, 2025 at 10:09:46PM +0100, Ignacio Encinas wrote: >> +#define ARCH_SWAB(size) \ >> +static __always_inline unsigned long __arch_swab##size(__u##size value) \ >> +{ \ >> + unsigned long x = value; \ >> + \ >> + asm goto(ALTERNATIVE("j %l[legacy]", "nop", 0, \ >> + RISCV_ISA_EXT_ZBB, 1) \ >> + :::: legacy); \ > > Is there a reason to use this instead of > riscv_has_extension_likely(RISCV_ISA_EXT_ZBB) which seems to do the same thing, > including using a static branch? I just followed what's already in arch/riscv/include/asm/bitops.h However, I changed it to if(riscv_has_extension_likely(RISCV_ISA_EXT_ZBB)) { asm volatile (".option push\n" ".option arch,+zbb\n" "rev8 %0, %1\n" ".option pop\n" : "=r" (x) : "r" (x)); return x >> (BITS_PER_LONG - size); } return ___constant_swab##size(value); and it seems gcc generates the exact same code. I tested it with arch/riscv/lib/csum.c (which uses swab32) and both versions generate the exact same object file. This certainly looks easier to read. If there are no complaints I'll send a v3 using a plain if with riscv_has_extension_likely. Thanks for pointing it out!
diff --git a/arch/riscv/include/asm/swab.h b/arch/riscv/include/asm/swab.h new file mode 100644 index 0000000000000000000000000000000000000000..6cb40e8108c956dd445746d59bc1dd0a53475212 --- /dev/null +++ b/arch/riscv/include/asm/swab.h @@ -0,0 +1,48 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef _ASM_RISCV_SWAB_H +#define _ASM_RISCV_SWAB_H + +#include <linux/types.h> +#include <linux/compiler.h> +#include <asm/alternative-macros.h> +#include <asm/hwcap.h> +#include <asm-generic/swab.h> + +#if defined(CONFIG_RISCV_ISA_ZBB) && !defined(NO_ALTERNATIVE) + +#define ARCH_SWAB(size) \ +static __always_inline unsigned long __arch_swab##size(__u##size value) \ +{ \ + unsigned long x = value; \ + \ + asm goto(ALTERNATIVE("j %l[legacy]", "nop", 0, \ + RISCV_ISA_EXT_ZBB, 1) \ + :::: legacy); \ + \ + asm volatile (".option push\n" \ + ".option arch,+zbb\n" \ + "rev8 %0, %1\n" \ + ".option pop\n" \ + : "=r" (x) : "r" (x)); \ + \ + return x >> (BITS_PER_LONG - size); \ + \ +legacy: \ + return ___constant_swab##size(value); \ +} + +#ifdef CONFIG_64BIT +ARCH_SWAB(64) +#define __arch_swab64 __arch_swab64 +#endif + +ARCH_SWAB(32) +#define __arch_swab32 __arch_swab32 + +ARCH_SWAB(16) +#define __arch_swab16 __arch_swab16 + +#undef ARCH_SWAB + +#endif /* defined(CONFIG_RISCV_ISA_ZBB) && !defined(NO_ALTERNATIVE) */ +#endif /* _ASM_RISCV_SWAB_H */
Implement endianness swap macros for RISC-V. Use the rev8 instruction when Zbb is available. Otherwise, rely on the default mask-and-shift implementation. Signed-off-by: Ignacio Encinas <ignacio@iencinas.com> --- arch/riscv/include/asm/swab.h | 48 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)