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

2021-08-17 Thread Matheus K. Ferst

On 17/08/2021 09:15, Peter Maydell 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 Tue, 17 Aug 2021 at 13:09, Matheus K. Ferst
 wrote:


On 17/08/2021 06:27, Philippe Mathieu-Daudé wrote:

On 8/16/21 9:13 PM, matheus.fe...@eldorado.org.br wrote:

From: Matheus Ferst 
-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.


You should be able to get away without a configure script test --
#if __has_builtin(__builtin_bswap128)
/* version with the builtin here */
#else
/* fallback */
#endif

ought to work. (Any gcc new enough to have the builtin also has
__has_builtin; clang has had __has_builtin for ages; our compiler.h
defines a fallback "always 0" __has_builtin for older compilers.)

-- PMM



Nice, I'll prepare a v2.

--
Matheus K. Ferst
Instituto de Pesquisas ELDORADO 
Analista de Software Júnior
Aviso Legal - Disclaimer 



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

2021-08-17 Thread Peter Maydell
On Tue, 17 Aug 2021 at 13:09, Matheus K. Ferst
 wrote:
>
> On 17/08/2021 06:27, Philippe Mathieu-Daudé wrote:
> > On 8/16/21 9:13 PM, matheus.fe...@eldorado.org.br wrote:
> >> From: Matheus Ferst 
> >> -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.

You should be able to get away without a configure script test --
#if __has_builtin(__builtin_bswap128)
/* version with the builtin here */
#else
/* fallback */
#endif

ought to work. (Any gcc new enough to have the builtin also has
__has_builtin; clang has had __has_builtin for ages; our compiler.h
defines a fallback "always 0" __has_builtin for older compilers.)

-- PMM



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

2021-08-17 Thread 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.fe...@eldorado.org.br wrote:

From: Matheus Ferst 

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 
---
  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 
Analista de Software Júnior
Aviso Legal - Disclaimer 



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

2021-08-17 Thread Philippe Mathieu-Daudé
On 8/16/21 9:13 PM, matheus.fe...@eldorado.org.br wrote:
> From: Matheus Ferst 
> 
> 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 
> ---
>  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().

>  #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 */
>