[PATCH v3 0/6] qemu/bswap: Use compiler __builtin_bswap()

[PATCH v3 0/6] qemu/bswap: Use compiler __builtin_bswap()

[Philippe Mathieu-Daudé]

Implement Richard's suggestion to use __builtin_bswap().

Convert to __builtin_bswap() one patch per OS to simplify
maintainers review.

Since v2:
- Rebased adapting ./configure changes to meson

Since v1:
- Remove the Haiku/BSD ifdef'ry (Peter)
- Include the Haiku VM image from Alexander

Philippe Mathieu-Daudé (6):
  qemu/bswap: Replace bswapXX() by compiler __builtin_bswap()
  qemu/bswap: Replace bswapXXs() by compiler __builtin_bswap()
  qemu/bswap: Remove <byteswap.h> dependency
  qemu/bswap: Use compiler __builtin_bswap() on Haiku
  qemu/bswap: Use compiler __builtin_bswap() on FreeBSD
  qemu/bswap: Use compiler __builtin_bswap() on NetBSD

 include/qemu/bswap.h | 83 ++++++++------------------------------------
 meson.build          |  6 ----
 2 files changed, 15 insertions(+), 74 deletions(-)

-- 
2.38.1

[PATCH v3 1/6] qemu/bswap: Replace bswapXX() by compiler __builtin_bswap()

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Use the compiler built-in function to byte swap values,
as the compiler is clever and will fold constants.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/qemu/bswap.h | 31 ++++++-------------------------
 1 file changed, 6 insertions(+), 25 deletions(-)

diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 346d05f2aa..ca2b4c3f15 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -37,31 +37,12 @@ static inline uint64_t bswap64(uint64_t x)
 #endif
 
 #ifdef BSWAP_FROM_FALLBACKS
-static inline uint16_t bswap16(uint16_t x)
-{
-    return (((x & 0x00ff) << 8) |
-            ((x & 0xff00) >> 8));
-}
-
-static inline uint32_t bswap32(uint32_t x)
-{
-    return (((x & 0x000000ffU) << 24) |
-            ((x & 0x0000ff00U) <<  8) |
-            ((x & 0x00ff0000U) >>  8) |
-            ((x & 0xff000000U) >> 24));
-}
-
-static inline uint64_t bswap64(uint64_t x)
-{
-    return (((x & 0x00000000000000ffULL) << 56) |
-            ((x & 0x000000000000ff00ULL) << 40) |
-            ((x & 0x0000000000ff0000ULL) << 24) |
-            ((x & 0x00000000ff000000ULL) <<  8) |
-            ((x & 0x000000ff00000000ULL) >>  8) |
-            ((x & 0x0000ff0000000000ULL) >> 24) |
-            ((x & 0x00ff000000000000ULL) >> 40) |
-            ((x & 0xff00000000000000ULL) >> 56));
-}
+#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
 
 #undef BSWAP_FROM_BYTESWAP
-- 
2.38.1

[PATCH v3 2/6] qemu/bswap: Replace bswapXXs() by compiler __builtin_bswap()

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/qemu/bswap.h | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index ca2b4c3f15..d2dafdc54c 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -50,29 +50,31 @@ static inline uint64_t bswap64(uint64_t x)
 
 static inline void bswap16s(uint16_t *s)
 {
-    *s = bswap16(*s);
+    *s = __builtin_bswap16(*s);
 }
 
 static inline void bswap32s(uint32_t *s)
 {
-    *s = bswap32(*s);
+    *s = __builtin_bswap32(*s);
 }
 
 static inline void bswap64s(uint64_t *s)
 {
-    *s = bswap64(*s);
+    *s = __builtin_bswap64(*s);
 }
 
 #if HOST_BIG_ENDIAN
 #define be_bswap(v, size) (v)
-#define le_bswap(v, size) glue(bswap, size)(v)
+#define le_bswap(v, size) glue(__builtin_bswap, size)(v)
 #define be_bswaps(v, size)
-#define le_bswaps(p, size) do { *p = glue(bswap, size)(*p); } while(0)
+#define le_bswaps(p, size) \
+            do { *p = glue(__builtin_bswap, size)(*p); } while (0)
 #else
 #define le_bswap(v, size) (v)
-#define be_bswap(v, size) glue(bswap, size)(v)
+#define be_bswap(v, size) glue(__builtin_bswap, size)(v)
 #define le_bswaps(v, size)
-#define be_bswaps(p, size) do { *p = glue(bswap, size)(*p); } while(0)
+#define be_bswaps(p, size) \
+            do { *p = glue(__builtin_bswap, size)(*p); } while (0)
 #endif
 
 /**
-- 
2.38.1

[PATCH v3 3/6] qemu/bswap: Remove <byteswap.h> dependency

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

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().
Drop the <byteswap.h> dependency.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/qemu/bswap.h | 21 ---------------------
 meson.build          |  2 --
 2 files changed, 23 deletions(-)

diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index d2dafdc54c..fd5a98125a 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -8,9 +8,6 @@
 # include <sys/endian.h>
 #elif defined(__HAIKU__)
 # include <endian.h>
-#elif defined(CONFIG_BYTESWAP_H)
-# include <byteswap.h>
-#define BSWAP_FROM_BYTESWAP
 # else
 #define BSWAP_FROM_FALLBACKS
 #endif /* ! CONFIG_MACHINE_BSWAP_H */
@@ -19,23 +16,6 @@
 extern "C" {
 #endif
 
-#ifdef BSWAP_FROM_BYTESWAP
-static inline uint16_t bswap16(uint16_t x)
-{
-    return bswap_16(x);
-}
-
-static inline uint32_t bswap32(uint32_t x)
-{
-    return bswap_32(x);
-}
-
-static inline uint64_t bswap64(uint64_t x)
-{
-    return bswap_64(x);
-}
-#endif
-
 #ifdef BSWAP_FROM_FALLBACKS
 #undef  bswap16
 #define bswap16(_x) __builtin_bswap16(_x)
@@ -45,7 +25,6 @@ static inline uint64_t bswap64(uint64_t x)
 #define bswap64(_x) __builtin_bswap64(_x)
 #endif
 
-#undef BSWAP_FROM_BYTESWAP
 #undef BSWAP_FROM_FALLBACKS
 
 static inline void bswap16s(uint16_t *s)
diff --git a/meson.build b/meson.build
index 175517eafd..697059d2c8 100644
--- a/meson.build
+++ b/meson.build
@@ -2006,8 +2006,6 @@ if rdma.found()
 endif
 
 # has_header_symbol
-config_host_data.set('CONFIG_BYTESWAP_H',
-                     cc.has_header_symbol('byteswap.h', 'bswap_32'))
 config_host_data.set('CONFIG_EPOLL_CREATE1',
                      cc.has_header_symbol('sys/epoll.h', 'epoll_create1'))
 config_host_data.set('CONFIG_FALLOCATE_PUNCH_HOLE',
-- 
2.38.1

[PATCH v3 4/6] qemu/bswap: Use compiler __builtin_bswap() on Haiku

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

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 Haiku specific ifdef'ry.

This reverts commit 652a46ebba970017c7a23767dcc983265cdb8eb7
("bswap.h: Include <endian.h> on Haiku for bswap operations").

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Cc: David Carlier <devnexen@gmail.com>
Cc: Carlo Arenas <carenas@gmail.com>
---
 include/qemu/bswap.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index fd5a98125a..8cd5a2b02e 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -6,8 +6,6 @@
 # include <machine/bswap.h>
 #elif defined(__FreeBSD__)
 # include <sys/endian.h>
-#elif defined(__HAIKU__)
-# include <endian.h>
 # else
 #define BSWAP_FROM_FALLBACKS
 #endif /* ! CONFIG_MACHINE_BSWAP_H */
-- 
2.38.1

[PATCH v3 5/6] qemu/bswap: Use compiler __builtin_bswap() on FreeBSD

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

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 FreeBSD specific ifdef'ry.

This reverts commit de03c3164accc21311c39327601fcdd95da301f3
("bswap: Fix build on FreeBSD 10.0").

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Cc: Ed Maste <emaste@freebsd.org>
---
 include/qemu/bswap.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 8cd5a2b02e..32d5cdec27 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -4,8 +4,6 @@
 #ifdef CONFIG_MACHINE_BSWAP_H
 # include <sys/endian.h>
 # include <machine/bswap.h>
-#elif defined(__FreeBSD__)
-# include <sys/endian.h>
 # else
 #define BSWAP_FROM_FALLBACKS
 #endif /* ! CONFIG_MACHINE_BSWAP_H */
-- 
2.38.1

[PATCH v3 6/6] qemu/bswap: Use compiler __builtin_bswap() on NetBSD

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

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>
---
 include/qemu/bswap.h | 11 -----------
 meson.build          |  4 ----
 2 files changed, 15 deletions(-)

diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 32d5cdec27..3cbe52246b 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -1,27 +1,16 @@
 #ifndef BSWAP_H
 #define BSWAP_H
 
-#ifdef CONFIG_MACHINE_BSWAP_H
-# include <sys/endian.h>
-# include <machine/bswap.h>
-# else
-#define BSWAP_FROM_FALLBACKS
-#endif /* ! CONFIG_MACHINE_BSWAP_H */
-
 #ifdef __cplusplus
 extern "C" {
 #endif
 
-#ifdef BSWAP_FROM_FALLBACKS
 #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
-
-#undef BSWAP_FROM_FALLBACKS
 
 static inline void bswap16s(uint16_t *s)
 {
diff --git a/meson.build b/meson.build
index 697059d2c8..f2663cfc32 100644
--- a/meson.build
+++ b/meson.build
@@ -2023,10 +2023,6 @@ config_host_data.set('CONFIG_INOTIFY',
                      cc.has_header_symbol('sys/inotify.h', 'inotify_init'))
 config_host_data.set('CONFIG_INOTIFY1',
                      cc.has_header_symbol('sys/inotify.h', 'inotify_init1'))
-config_host_data.set('CONFIG_MACHINE_BSWAP_H',
-                     cc.has_header_symbol('machine/bswap.h', 'bswap32',
-                                          prefix: '''#include <sys/endian.h>
-                                                     #include <sys/types.h>'''))
 config_host_data.set('CONFIG_PRCTL_PR_SET_TIMERSLACK',
                      cc.has_header_symbol('sys/prctl.h', 'PR_SET_TIMERSLACK'))
 config_host_data.set('CONFIG_RTNETLINK',
-- 
2.38.1

Re: [PATCH v3 0/6] qemu/bswap: Use compiler __builtin_bswap()

[Richard Henderson]

On 1/11/23 08:31, Philippe Mathieu-Daudé wrote:
> Implement Richard's suggestion to use __builtin_bswap().
> 
> Convert to __builtin_bswap() one patch per OS to simplify
> maintainers review.
> 
> Since v2:
> - Rebased adapting ./configure changes to meson
> 
> Since v1:
> - Remove the Haiku/BSD ifdef'ry (Peter)
> - Include the Haiku VM image from Alexander
> 
> Philippe Mathieu-Daudé (6):
>    qemu/bswap: Replace bswapXX() by compiler __builtin_bswap()
>    qemu/bswap: Replace bswapXXs() by compiler __builtin_bswap()
>    qemu/bswap: Remove <byteswap.h> dependency
>    qemu/bswap: Use compiler __builtin_bswap() on Haiku
>    qemu/bswap: Use compiler __builtin_bswap() on FreeBSD
>    qemu/bswap: Use compiler __builtin_bswap() on NetBSD

If this passes on all the odd OS's, great.
Failure on some oddball is what blocked my patch set years ago.


r~

Re: [PATCH v3 0/6] qemu/bswap: Use compiler __builtin_bswap()

[Philippe Mathieu-Daudé]

On 13/1/23 02:05, Richard Henderson wrote:
> On 1/11/23 08:31, Philippe Mathieu-Daudé wrote:
>> Implement Richard's suggestion to use __builtin_bswap().
>>
>> Convert to __builtin_bswap() one patch per OS to simplify
>> maintainers review.
>>
>> Since v2:
>> - Rebased adapting ./configure changes to meson
>>
>> Since v1:
>> - Remove the Haiku/BSD ifdef'ry (Peter)
>> - Include the Haiku VM image from Alexander
>>
>> Philippe Mathieu-Daudé (6):
>>    qemu/bswap: Replace bswapXX() by compiler __builtin_bswap()
>>    qemu/bswap: Replace bswapXXs() by compiler __builtin_bswap()
>>    qemu/bswap: Remove <byteswap.h> dependency
>>    qemu/bswap: Use compiler __builtin_bswap() on Haiku
>>    qemu/bswap: Use compiler __builtin_bswap() on FreeBSD
>>    qemu/bswap: Use compiler __builtin_bswap() on NetBSD
> 
> If this passes on all the odd OS's, great.
> Failure on some oddball is what blocked my patch set years ago.

OK I'll double-check.

Re: [PATCH v3 0/6] qemu/bswap: Use compiler __builtin_bswap()

[Thomas Huth]

On 13/01/2023 08.05, Philippe Mathieu-Daudé wrote:
> On 13/1/23 02:05, Richard Henderson wrote:
>> On 1/11/23 08:31, Philippe Mathieu-Daudé wrote:
>>> Implement Richard's suggestion to use __builtin_bswap().
>>>
>>> Convert to __builtin_bswap() one patch per OS to simplify
>>> maintainers review.
>>>
>>> Since v2:
>>> - Rebased adapting ./configure changes to meson
>>>
>>> Since v1:
>>> - Remove the Haiku/BSD ifdef'ry (Peter)
>>> - Include the Haiku VM image from Alexander
>>>
>>> Philippe Mathieu-Daudé (6):
>>>    qemu/bswap: Replace bswapXX() by compiler __builtin_bswap()
>>>    qemu/bswap: Replace bswapXXs() by compiler __builtin_bswap()
>>>    qemu/bswap: Remove <byteswap.h> dependency
>>>    qemu/bswap: Use compiler __builtin_bswap() on Haiku
>>>    qemu/bswap: Use compiler __builtin_bswap() on FreeBSD
>>>    qemu/bswap: Use compiler __builtin_bswap() on NetBSD
>>
>> If this passes on all the odd OS's, great.
>> Failure on some oddball is what blocked my patch set years ago.
> 
> OK I'll double-check.

Please note that "make vm-build-haiku.x86_64" is currently broken - we need 
an update to Beta 4 from Alexander first.

  Thomas

Re: [PATCH v3 0/6] qemu/bswap: Use compiler __builtin_bswap()

[Philippe Mathieu-Daudé]

On 13/1/23 08:05, Philippe Mathieu-Daudé wrote:
> On 13/1/23 02:05, Richard Henderson wrote:
>> On 1/11/23 08:31, Philippe Mathieu-Daudé wrote:
>>> Implement Richard's suggestion to use __builtin_bswap().
>>>
>>> Convert to __builtin_bswap() one patch per OS to simplify
>>> maintainers review.
>>>
>>> Since v2:
>>> - Rebased adapting ./configure changes to meson
>>>
>>> Since v1:
>>> - Remove the Haiku/BSD ifdef'ry (Peter)
>>> - Include the Haiku VM image from Alexander
>>>
>>> Philippe Mathieu-Daudé (6):
>>>    qemu/bswap: Replace bswapXX() by compiler __builtin_bswap()
>>>    qemu/bswap: Replace bswapXXs() by compiler __builtin_bswap()
>>>    qemu/bswap: Remove <byteswap.h> dependency
>>>    qemu/bswap: Use compiler __builtin_bswap() on Haiku
>>>    qemu/bswap: Use compiler __builtin_bswap() on FreeBSD
>>>    qemu/bswap: Use compiler __builtin_bswap() on NetBSD
>>
>> If this passes on all the odd OS's, great.
>> Failure on some oddball is what blocked my patch set years ago.
> 
> OK I'll double-check.

Tested successfully on:

- QEMU GitLab CI
- Darwin 22.2
- FreeBSD 12.4
- NetBSD 9.3
- OpenBSD 7.2
- Haiku r1beta4

Re: [PATCH v3 0/6] qemu/bswap: Use compiler __builtin_bswap()

[Richard Henderson]

On 1/16/23 22:24, Philippe Mathieu-Daudé wrote:
> On 13/1/23 08:05, Philippe Mathieu-Daudé wrote:
>> On 13/1/23 02:05, Richard Henderson wrote:
>>> On 1/11/23 08:31, Philippe Mathieu-Daudé wrote:
>>>> Implement Richard's suggestion to use __builtin_bswap().
>>>>
>>>> Convert to __builtin_bswap() one patch per OS to simplify
>>>> maintainers review.
>>>>
>>>> Since v2:
>>>> - Rebased adapting ./configure changes to meson
>>>>
>>>> Since v1:
>>>> - Remove the Haiku/BSD ifdef'ry (Peter)
>>>> - Include the Haiku VM image from Alexander
>>>>
>>>> Philippe Mathieu-Daudé (6):
>>>>    qemu/bswap: Replace bswapXX() by compiler __builtin_bswap()
>>>>    qemu/bswap: Replace bswapXXs() by compiler __builtin_bswap()
>>>>    qemu/bswap: Remove <byteswap.h> dependency
>>>>    qemu/bswap: Use compiler __builtin_bswap() on Haiku
>>>>    qemu/bswap: Use compiler __builtin_bswap() on FreeBSD
>>>>    qemu/bswap: Use compiler __builtin_bswap() on NetBSD
>>>
>>> If this passes on all the odd OS's, great.
>>> Failure on some oddball is what blocked my patch set years ago.
>>
>> OK I'll double-check.
> 
> Tested successfully on:
> 
> - QEMU GitLab CI
> - Darwin 22.2
> - FreeBSD 12.4
> - NetBSD 9.3
> - OpenBSD 7.2
> - Haiku r1beta4

Great.  Just in case I missed an individual patch, series:
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH v3 0/6] qemu/bswap: Use compiler __builtin_bswap()

[Thomas Huth]

On 11/01/2023 17.31, Philippe Mathieu-Daudé wrote:
> Implement Richard's suggestion to use __builtin_bswap().
> 
> Convert to __builtin_bswap() one patch per OS to simplify
> maintainers review.
> 
> Since v2:
> - Rebased adapting ./configure changes to meson
> 
> Since v1:
> - Remove the Haiku/BSD ifdef'ry (Peter)
> - Include the Haiku VM image from Alexander
> 
> Philippe Mathieu-Daudé (6):
>    qemu/bswap: Replace bswapXX() by compiler __builtin_bswap()
>    qemu/bswap: Replace bswapXXs() by compiler __builtin_bswap()
>    qemu/bswap: Remove <byteswap.h> dependency
>    qemu/bswap: Use compiler __builtin_bswap() on Haiku
>    qemu/bswap: Use compiler __builtin_bswap() on FreeBSD
>    qemu/bswap: Use compiler __builtin_bswap() on NetBSD
> 
>   include/qemu/bswap.h | 83 ++++++++------------------------------------
>   meson.build          |  6 ----
>   2 files changed, 15 insertions(+), 74 deletions(-)

Since nobody else picked this up yet, I'll take it for my next pull request. 
Queued to my staging branch now:

  https://gitlab.com/thuth/qemu/-/commits/staging

   Thomas