Re: [PATCH for-6.2 1/2] include/qemu/int128.h: introduce bswap128s

[Matheus K. Ferst]

On 17/08/2021 06:27, Philippe Mathieu-Daudé wrote:
> [E-MAIL EXTERNO] Não clique em links ou abra anexos, a menos que você possa 
> confirmar o remetente e saber que o conteúdo é seguro. Em caso de e-mail 
> suspeito entre imediatamente em contato com o DTI.
>
> On 8/16/21 9:13 PM, matheus.ferst@eldorado.org.br wrote:
> > From: Matheus Ferst <matheus.ferst@eldorado.org.br>
> >
> > Introduces bswap128s based on bswap128. Since bswap128 is defined using
> > int128_* methods available in either CONFIG_INT128 or !CONFIG_INT128
> > builds, place both outside of #ifdef CONFIG_INT128.
> >
> > Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> > ---
> >   include/qemu/int128.h | 16 +++++++++++-----
> >   1 file changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/qemu/int128.h b/include/qemu/int128.h
> > index 64500385e3..e0d385628c 100644
> > --- a/include/qemu/int128.h
> > +++ b/include/qemu/int128.h
> > @@ -153,11 +153,6 @@ static inline void int128_subfrom(Int128 *a, Int128 b)
> >       *a -= b;
> >   }
> >
> > -static inline Int128 bswap128(Int128 a)
> > -{
> > -    return int128_make128(bswap64(int128_gethi(a)), bswap64(int128_getlo(a)));
> > -}
>
> Personally I'd move this one to the other #ifdef side,
> and implement here with __builtin_bswap128().

I saw this builtin, but I couldn't test it on my system. It seems that 
Clang doesn't implement it, and it's only available on GCC 11: 
https://godbolt.org/z/T6vhd5a38 . I think we can use it, but I'd need to 
figure how to add a test for it in the configure script.

> >   #else /* !CONFIG_INT128 */
> >
> >   typedef struct Int128 Int128;
> > @@ -338,4 +333,15 @@ static inline void int128_subfrom(Int128 *a, Int128 b)
> >   }
>
> > +static inline Int128 bswap128(Int128 a)
> > +{
> > +    return int128_make128(bswap64(int128_gethi(a)), bswap64(int128_getlo(a)));
> > +}
>
>     #endif /* CONFIG_INT128 */
>
> And add this generic one here indeed:
>
> > +static inline void bswap128s(Int128 *s)
> > +{
> > +    *s = bswap128(*s);
> > +}
> > +
> >   #endif /* INT128_H */

--
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software Júnior
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>