Re: [PATCH 4/4] qemu/bswap: Let cpu_to_endian() functions handle constant expressions

[Richard Henderson]

On 9/17/20 9:31 AM, Philippe Mathieu-Daudé wrote:
> cpu_to_endian() and endian_to_cpu() can be extended to handle
> constant expressions. That way the programmer doesn't need to
> remember the const_X() API exists.
> 
> Suggested-by: Stefan Hajnoczi <stefanha@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/qemu/bswap.h | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
> index de256cea3ab..8827e4760b9 100644
> --- a/include/qemu/bswap.h
> +++ b/include/qemu/bswap.h
> @@ -191,12 +191,16 @@ static inline void bswap64s(uint64_t *s)
>  #define CPU_CONVERT(endian, size, type)\
>  static inline type endian ## size ## _to_cpu(type v)\
>  {\
> -    return glue(endian, _bswap)(v, size);\
> +    return __builtin_constant_p(v) ?\
> +           const_ ## endian ## size(v) :\
> +           glue(endian, _bswap)(v, size);\
>  }\
>  \
>  static inline type cpu_to_ ## endian ## size(type v)\
>  {\
> -    return glue(endian, _bswap)(v, size);\
> +    return __builtin_constant_p(v) ?\
> +           const_ ## endian ## size(v) :\
> +           glue(endian, _bswap)(v, size);\
>  }\
>  \
>  static inline void endian ## size ## _to_cpus(type *p)\
> 

How does this actually affect code generation?

If it does, then that's a mistake in our definition of bswap{16,32,64}(), which
should have been able to fold constants just fine.

Looking at all of that CONFIG_MACHINE_BSWAP_H stuff, I think we should just
ditch it all in favour of __builin_bswap*.  The final piece, __builtin_bswap16,
came in at ac868f29d7e8 in gcc-4.8.


r~