| Message ID | 20210816191316.1163622-2-matheus.ferst@eldorado.org.br |
|---|---|
| State | New |
| Headers | show |
| Series | target/ppc: Fix vector registers access in gdbstub for little-endian | expand |
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(). > #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 */ >
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 */ >> >
On Tue, 17 Aug 2021 at 13:09, Matheus K. Ferst <matheus.ferst@eldorado.org.br> wrote: > > On 17/08/2021 06:27, Philippe Mathieu-Daudé wrote: > > On 8/16/21 9:13 PM, matheus.ferst@eldorado.org.br wrote: > >> From: Matheus Ferst <matheus.ferst@eldorado.org.br> > >> -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
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 > <matheus.ferst@eldorado.org.br> wrote: >> >> On 17/08/2021 06:27, Philippe Mathieu-Daudé wrote: >>> On 8/16/21 9:13 PM, matheus.ferst@eldorado.org.br wrote: >>>> From: Matheus Ferst <matheus.ferst@eldorado.org.br> >>>> -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.
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))); -} - #else /* !CONFIG_INT128 */ typedef struct Int128 Int128; @@ -338,4 +333,15 @@ static inline void int128_subfrom(Int128 *a, Int128 b) } #endif /* CONFIG_INT128 */ + +static inline Int128 bswap128(Int128 a) +{ + return int128_make128(bswap64(int128_gethi(a)), bswap64(int128_getlo(a))); +} + +static inline void bswap128s(Int128 *s) +{ + *s = bswap128(*s); +} + #endif /* INT128_H */