[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 1/6] xen: implement byteswap



Hi,

On 10/05/2022 11:15, Lin Liu wrote:
swab() is massively over complicated and can be simplified by builtins.

NIT: "by builtins" -> "by re-implementing using compiler builtins".

The compilers provide builtin function to swap bytes.
* gcc:   https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
* clang: https://clang.llvm.org/docs/LanguageExtensions.html
This patch simplify swab() with builtins and fallback for old compilers.

Signed-off-by: Lin Liu <lin.liu@xxxxxxxxxx>
---
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Cc: Julien Grall <julien@xxxxxxx>
Cc: Bertrand Marquis <bertrand.marquis@xxxxxxx>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Cc: George Dunlap <george.dunlap@xxxxxxxxxx>
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: Wei Liu <wl@xxxxxxx>
Cc: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
Changes in v3:
- Check __has_builtin instead of GNUC version

Changes in v2:
- Add fallback for compilers without __builtin_bswap
- Implement with plain C instead of macros
---
  xen/arch/arm/include/asm/byteorder.h | 14 ++-----
  xen/arch/x86/include/asm/byteorder.h | 34 ++---------------
  xen/include/xen/byteorder.h          | 56 ++++++++++++++++++++++++++++
  xen/include/xen/byteswap.h           | 44 ++++++++++++++++++++++
  xen/include/xen/compiler.h           | 12 ++++++
  5 files changed, 120 insertions(+), 40 deletions(-)
  create mode 100644 xen/include/xen/byteorder.h
  create mode 100644 xen/include/xen/byteswap.h

diff --git a/xen/arch/arm/include/asm/byteorder.h 
b/xen/arch/arm/include/asm/byteorder.h
index 9c712c4788..622eeaba07 100644
--- a/xen/arch/arm/include/asm/byteorder.h
+++ b/xen/arch/arm/include/asm/byteorder.h
@@ -1,16 +1,10 @@
  #ifndef __ASM_ARM_BYTEORDER_H__
  #define __ASM_ARM_BYTEORDER_H__
-#define __BYTEORDER_HAS_U64__
+#ifndef __BYTE_ORDER__
+   #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
+#endif
-#include <xen/byteorder/little_endian.h>
+#include <xen/byteorder.h>
#endif /* __ASM_ARM_BYTEORDER_H__ */
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */

This change looks unrelated to this patch. Can you explain it?

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.