| Message ID | 20200928131934.739451-8-philmd@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qemu/bswap: Use compiler __builtin_bswap() | expand |
On 28.09.2020 15:19, Philippe Mathieu-Daudé wrote: > Since commit efc6c070aca ("configure: Add a test for the minimum > compiler version") the minimum compiler version required for GCC > is 4.8, which supports __builtin_bswap(). > Remove the NetBSD specific ifdef'ry. > > This reverts commit 1360677cfe3ca8f945fa1de77823df21a77e4500 > ("makes NetBSD use the native bswap functions"). > Personally, I prefer using the system headers. but if you want to use the GCC builtins, please go for it. > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > configure | 15 --------------- > include/qemu/bswap.h | 5 ----- > 2 files changed, 20 deletions(-) > > diff --git a/configure b/configure > index bff787daea7..1b0a02a0af8 100755 > --- a/configure > +++ b/configure > @@ -4886,18 +4886,6 @@ if test "$docs" != "no" ; then > fi > fi > > -# Search for bswap32 function > -bswap_h=no > -cat > $TMPC << EOF > -#include <sys/endian.h> > -#include <sys/types.h> > -#include <machine/bswap.h> > -int main(void) { return bswap32(0); } > -EOF > -if compile_prog "" "" ; then > - bswap_h=yes > -fi > - > ########################################## > # Do we have libiscsi >= 1.9.0 > if test "$libiscsi" != "no" ; then > @@ -6779,9 +6767,6 @@ fi > if test "$st_atim" = "yes" ; then > echo "HAVE_STRUCT_STAT_ST_ATIM=y" >> $config_host_mak > fi > -if test "$bswap_h" = "yes" ; then > - echo "CONFIG_MACHINE_BSWAP_H=y" >> $config_host_mak > -fi > if test "$curl" = "yes" ; then > echo "CONFIG_CURL=y" >> $config_host_mak > echo "CURL_CFLAGS=$curl_cflags" >> $config_host_mak > diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h > index 1a297bfec22..7e586531c09 100644 > --- a/include/qemu/bswap.h > +++ b/include/qemu/bswap.h > @@ -3,17 +3,12 @@ > > #include "fpu/softfloat-types.h" > > -#ifdef CONFIG_MACHINE_BSWAP_H > -# include <sys/endian.h> > -# include <machine/bswap.h> > -#else > #undef bswap16 > #define bswap16(_x) __builtin_bswap16(_x) > #undef bswap32 > #define bswap32(_x) __builtin_bswap32(_x) > #undef bswap64 > #define bswap64(_x) __builtin_bswap64(_x) > -#endif /* ! CONFIG_MACHINE_BSWAP_H */ > > static inline void bswap16s(uint16_t *s) > { >
On Mon, 28 Sep 2020 at 23:02, Kamil Rytarowski <kamil@netbsd.org> wrote: > > Personally, I prefer using the system headers. but if you want to use > the GCC builtins, please go for it. I'd agree if the system header approach was cross-platform or if this was a BSD-only program or if we were aiming for complete compiler-implementation independence, but since we rely on GCC/clang all over the place already it seems nicer to avoid all the machinery for identifying which of the multiple different system header implementations is present, and instead just have a single implementation that works on all the hosts we care about... thanks -- PMM
On 29.09.2020 10:58, Peter Maydell wrote: > On Mon, 28 Sep 2020 at 23:02, Kamil Rytarowski <kamil@netbsd.org> wrote: >> >> Personally, I prefer using the system headers. but if you want to use >> the GCC builtins, please go for it. > > I'd agree if the system header approach was cross-platform > or if this was a BSD-only program or if we were aiming for > complete compiler-implementation independence, but since we > rely on GCC/clang all over the place already it seems nicer to > avoid all the machinery for identifying which of the multiple > different system header implementations is present, and > instead just have a single implementation that works on > all the hosts we care about... > This is already a part of POSIX: https://www.austingroupbugs.net/view.php?id=162 We have got everything needed from the standard now to implement bswap without relying on compiler builtins. Every modern enough POSIX-like OS already ships with <endian.h>. > thanks > -- PMM >
diff --git a/configure b/configure index bff787daea7..1b0a02a0af8 100755 --- a/configure +++ b/configure @@ -4886,18 +4886,6 @@ if test "$docs" != "no" ; then fi fi -# Search for bswap32 function -bswap_h=no -cat > $TMPC << EOF -#include <sys/endian.h> -#include <sys/types.h> -#include <machine/bswap.h> -int main(void) { return bswap32(0); } -EOF -if compile_prog "" "" ; then - bswap_h=yes -fi - ########################################## # Do we have libiscsi >= 1.9.0 if test "$libiscsi" != "no" ; then @@ -6779,9 +6767,6 @@ fi if test "$st_atim" = "yes" ; then echo "HAVE_STRUCT_STAT_ST_ATIM=y" >> $config_host_mak fi -if test "$bswap_h" = "yes" ; then - echo "CONFIG_MACHINE_BSWAP_H=y" >> $config_host_mak -fi if test "$curl" = "yes" ; then echo "CONFIG_CURL=y" >> $config_host_mak echo "CURL_CFLAGS=$curl_cflags" >> $config_host_mak diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h index 1a297bfec22..7e586531c09 100644 --- a/include/qemu/bswap.h +++ b/include/qemu/bswap.h @@ -3,17 +3,12 @@ #include "fpu/softfloat-types.h" -#ifdef CONFIG_MACHINE_BSWAP_H -# include <sys/endian.h> -# include <machine/bswap.h> -#else #undef bswap16 #define bswap16(_x) __builtin_bswap16(_x) #undef bswap32 #define bswap32(_x) __builtin_bswap32(_x) #undef bswap64 #define bswap64(_x) __builtin_bswap64(_x) -#endif /* ! CONFIG_MACHINE_BSWAP_H */ static inline void bswap16s(uint16_t *s) {
Since commit efc6c070aca ("configure: Add a test for the minimum compiler version") the minimum compiler version required for GCC is 4.8, which supports __builtin_bswap(). Remove the NetBSD specific ifdef'ry. This reverts commit 1360677cfe3ca8f945fa1de77823df21a77e4500 ("makes NetBSD use the native bswap functions"). Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- configure | 15 --------------- include/qemu/bswap.h | 5 ----- 2 files changed, 20 deletions(-)