Re: bswap is slow on SPARC

[Valery Ushakov <uwe%stderr.spb.ru@localhost>]

On Mon, Nov 24, 2025 at 13:30:50 +0000, nia wrote:

> +#ifdef _BSWAP_IS_SLOW

Not the best name, admittedly, but naming is hard...

We have an established convention of __HAVE_feature defines for stuff
like this, though in this case we will need a negative
__HAVE_NO_FAST_BSWAP, which is kinda ugly.


> -#define	__byte_swap_u64_constant(x) \
> +#define	__byte_swap_u64(x) \
>  	(__CAST(uint64_t, \
>  	 ((((x) & 0xff00000000000000ull) >> 56) | \
>  	  (((x) & 0x00ff000000000000ull) >> 40) | \
[...]

Since you are chaning the name anyway, then, I think,
__byte_swap_u64_constexpr etc might be a better name, b/c that's what
it actually is without any additional inferences required (while the
old _constant name kinda implied that "of course an explicit
expression is the best way to bswap a constant b/c the compiler can
eval it at compile time").

> +/*
> + * The compiler always generates an expensive function call to bswap
> + * on some architectures, we want the inline versions there.
> + */
> +#ifdef _BSWAP_IS_SLOW
> +
> +static __inline uint64_t __byte_swap_u64_inline(uint64_t x) {
> +	return __byte_swap_u64(x);
> +}

To continue from the first point, I think it also makes this a bit
more clear, as "expr" is probably not an indirection to another
function call (please, no nit-picking about a function call also being
an expression, technically speaking :)

    static __inline uint64_t __byte_swap_u64_inline(uint64_t x) {
            return __byte_swap_u64_constexpr(x);
    }



> +#else

Please, add /* !_BSWAP_IS_SLOW */ on #else to make it easier to
follow without looking up the matching #if.

> +#endif /* _BSWAP_IS_SLOW */

And change this to /* !_BSWAP_IS_SLOW */ to reflect the condition for
the code it delimits.  Cf. #endif /* !_SYS_ENDIAN_H_ */ etc in include
guards.



>  #define	bswap64(x) \
>  	__CAST(uint64_t, __builtin_constant_p((x)) ? \
> -	 __byte_swap_u64_constant(x) : __BYTE_SWAP_U64_VARIABLE(x))
> +	 __byte_swap_u64(x) : __BYTE_SWAP_U64_VARIABLE(x))

With the suggested renaming this turns into:

    #define bswap64(x) \
            __CAST(uint64_t, __builtin_constant_p((x)) ? \
            __byte_swap_u64_constexpr(x) : __BYTE_SWAP_U64_VARIABLE(x))


> +/*
> + * GCC fails to generate inline calls to bswapX on sparc and instead
> + * generates function calls.
> + */
> +#define _BSWAP_IS_SLOW 1

It doesn't "fail".  There's just no builtin.  May be make these
comments match the generic comment before the #ifdef that tests it?

Also see above about __HAVE_<feature> (and note how we don't define
any expansions for them).


-uwe