{"thread":{"id":"63800","subject":"[PATCH v3 1/5] bswap.h: Add support for __BYTE_ORDER__","startedAt":"2025-07-15T19:12:53Z","lastAt":"2025-07-16T05:01:15Z","messageCount":9,"participants":["Sebastian Andrzej Siewior","Junio C Hamano","brian m. carlson"],"isPatch":true,"patchVersion":3,"patchTotal":5},"messages":[{"id":"522030","messageId":"20250715191230.12081-2-sebastian@breakpoint.cc","threadId":"63800","inReplyTo":"20250715191230.12081-1-sebastian@breakpoint.cc","subject":"[PATCH v3 1/5] bswap.h: Add support for __BYTE_ORDER__","fromName":"Sebastian Andrzej Siewior","fromEmail":"sebastian@breakpoint.cc","sentAt":"2025-07-15T19:12:26Z","receivedAt":"2025-07-15T19:12:53Z","isPatch":true,"sender":{"key":"sebastian@breakpoint.cc","avatar":"https://avatars.githubusercontent.com/u/5932951?v=4"},"body":"The __BYTE_ORDER__ define is provided by gcc (since ~v4.6), clang\n(since ~v3.2) and icc (since ~16.0.3).\n\nThe __BYTE_ORDER and BYTE_ORDER macros are libc specific and are not\navailable on all supported platforms such as mingw.\n\nAdd support for the __BYTE_ORDER__ macro as a fallback.\n\nSigned-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>\n---\n compat/bswap.h | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/compat/bswap.h b/compat/bswap.h\nindex b34054f2bd728..0a457542dd76a 100644\n--- a/compat/bswap.h\n+++ b/compat/bswap.h\n@@ -116,6 +116,12 @@ static inline uint64_t git_bswap64(uint64_t x)\n # define GIT_LITTLE_ENDIAN LITTLE_ENDIAN\n # define GIT_BIG_ENDIAN BIG_ENDIAN\n \n+#elif defined(__BYTE_ORDER__) && defined(__ORDER_LITTLE_ENDIAN__) && defined(__ORDER_BIG_ENDIAN__)\n+\n+# define GIT_BYTE_ORDER __BYTE_ORDER__\n+# define GIT_LITTLE_ENDIAN __ORDER_LITTLE_ENDIAN__\n+# define GIT_BIG_ENDIAN __ORDER_BIG_ENDIAN__\n+\n #else\n \n # define GIT_BIG_ENDIAN 4321\n-- \n2.50.0\n\n"},{"id":"522031","messageId":"20250715191230.12081-1-sebastian@breakpoint.cc","threadId":"63800","inReplyTo":null,"subject":"[PATCH v3 0/5] bswap.h: Rework ntohl handling","fromName":"Sebastian Andrzej Siewior","fromEmail":"sebastian@breakpoint.cc","sentAt":"2025-07-15T19:12:25Z","receivedAt":"2025-07-15T19:12:53Z","isPatch":true,"sender":{"key":"sebastian@breakpoint.cc","avatar":"https://avatars.githubusercontent.com/u/5932951?v=4"},"body":"Hi,\n\nthis series continues the rework of the bswap32/64()/ nothl() handling. \n\nI've been looking at recent compiler and they manage to recognize the\nmanual shifting and use an optimize opcode if available. The ntohl\nversion provided by glibc already provides an \"optimized\" version which\nmakes an optimisation in git almost not needed.\nOne of the motivation behind overwriting/ providing an optimized\nversion was to provide a macro instead of using a function call. One\nlibc that is still providing ntohl as a function call is musl.\n\nWhile ntohl() is provided by the libc, the ntohll() is not. I found it\nonly on Windows provided by winsock.h.\n\nI haven't touched the put/get_be*() macros. gcc & clang are both smart\nenough to swap the content accordingly and perform a single store/ load.\nOnly the msvc seems to strugle here and performs multiple bytes stores/\nloads and shifts.\n\nv2…v3 https://lore.kernel.org/all/20250611221444.1567638-1-sebastian@breakpoint.cc/\n  - Fixed typos in the patch description\n\nSebastian Andrzej Siewior (5):\n  bswap.h: Add support for __BYTE_ORDER__\n  bswap.h: Define GIT_LITTLE_ENDIAN on msvc as little endian\n  bswap.h: Always overwrite ntohl/ ntohll macros\n  bswap.h: Remove optimized x86 version of bswap32/64\n  bswap.h: Provide a built-in based version of bswap32/64 if possible\n\n compat/bswap.h | 114 +++++++++++++++++++++----------------------------\n 1 file changed, 48 insertions(+), 66 deletions(-)\n\n-- \n2.50.0\n\n"},{"id":"522032","messageId":"20250715191230.12081-5-sebastian@breakpoint.cc","threadId":"63800","inReplyTo":"20250715191230.12081-1-sebastian@breakpoint.cc","subject":"[PATCH v3 4/5] bswap.h: Remove optimized x86 version of bswap32/64","fromName":"Sebastian Andrzej Siewior","fromEmail":"sebastian@breakpoint.cc","sentAt":"2025-07-15T19:12:29Z","receivedAt":"2025-07-15T19:12:53Z","isPatch":true,"sender":{"key":"sebastian@breakpoint.cc","avatar":"https://avatars.githubusercontent.com/u/5932951?v=4"},"body":"On x86 the bswap32/64 macro is implemented based on the x86 opcode which\nperforms the required shifting in just one opcode.\nThe other CPUs fallback to the generic shifting as implemented by\ndefault_swab32() and default_bswap64() if needed.\n\nI've been looking at how good a compiler is at recognizing the default\nshift and emitting an optimized operation:\n- x86, arm64 msvc v19.20\n  default_swab32() optimized\n  default_bswap64() shifts\n  _byteswap_uint64() optimized\n\n- x86, arm64 msvc v19.37\n  default_swab32() optimized\n  default_bswap64() optimized\n  _byteswap_uint64() optimized\n\n- arm64, gcc-4.9.4: optimized\n- x86-64, gcc-4.4.7: shifts\n- x86-64, gcc-4.5.3: optimized\n- x86-64, clang-3.0: optimized\n\nGiven that gcc-4.5 and clang-3.0 are fairly old, any recent compiler\nshould recognize the shift.\n\nRemove the optimized x86 version and rely on the compiler.\n\nSigned-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>\n---\n compat/bswap.h | 41 +----------------------------------------\n 1 file changed, 1 insertion(+), 40 deletions(-)\n\ndiff --git a/compat/bswap.h b/compat/bswap.h\nindex aeef304f671f5..ed00f6d1d53f3 100644\n--- a/compat/bswap.h\n+++ b/compat/bswap.h\n@@ -35,46 +35,7 @@ static inline uint64_t default_bswap64(uint64_t val)\n #undef bswap32\n #undef bswap64\n \n-#if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))\n-\n-#define bswap32 git_bswap32\n-static inline uint32_t git_bswap32(uint32_t x)\n-{\n-\tuint32_t result;\n-\tif (__builtin_constant_p(x))\n-\t\tresult = default_swab32(x);\n-\telse\n-\t\t__asm__(\"bswap %0\" : \"=r\" (result) : \"0\" (x));\n-\treturn result;\n-}\n-\n-#define bswap64 git_bswap64\n-#if defined(__x86_64__)\n-static inline uint64_t git_bswap64(uint64_t x)\n-{\n-\tuint64_t result;\n-\tif (__builtin_constant_p(x))\n-\t\tresult = default_bswap64(x);\n-\telse\n-\t\t__asm__(\"bswap %q0\" : \"=r\" (result) : \"0\" (x));\n-\treturn result;\n-}\n-#else\n-static inline uint64_t git_bswap64(uint64_t x)\n-{\n-\tunion { uint64_t i64; uint32_t i32[2]; } tmp, result;\n-\tif (__builtin_constant_p(x))\n-\t\tresult.i64 = default_bswap64(x);\n-\telse {\n-\t\ttmp.i64 = x;\n-\t\tresult.i32[0] = git_bswap32(tmp.i32[1]);\n-\t\tresult.i32[1] = git_bswap32(tmp.i32[0]);\n-\t}\n-\treturn result.i64;\n-}\n-#endif\n-\n-#elif defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64) || defined(_M_ARM64))\n+#if defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64) || defined(_M_ARM64))\n \n #include <stdlib.h>\n \n-- \n2.50.0\n\n"},{"id":"522033","messageId":"20250715191230.12081-6-sebastian@breakpoint.cc","threadId":"63800","inReplyTo":"20250715191230.12081-1-sebastian@breakpoint.cc","subject":"[PATCH v3 5/5] bswap.h: Provide a built-in based version of bswap32/64 if possible","fromName":"Sebastian Andrzej Siewior","fromEmail":"sebastian@breakpoint.cc","sentAt":"2025-07-15T19:12:30Z","receivedAt":"2025-07-15T19:12:54Z","isPatch":true,"sender":{"key":"sebastian@breakpoint.cc","avatar":"https://avatars.githubusercontent.com/u/5932951?v=4"},"body":"The compiler is in general able to recognize the endian shift and\nreplace it with an optimized opcode if possible. On certain\narchitectures such as RiscV or MIPS the situation can get complicated.\nThey don't provide an optimized opcode and masking the \"higher\" bits may\nrequired loading a constant which needs shifting. This causes the\ncompiler to emit a lot of instructions for the operation.\n\nThe provided builtin directive on these architecture calls a function\nwhich does the operation instead of emitting the code for operation.\n\nBring back the change from commit 6547d1c9 (bswap.h: add support for\nbuilt-in bswap functions, 2025-04-23). The bswap32/64 macro can now be\ndefined unconditionally so it won't regress on big endian architectures.\n\nSigned-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>\n---\n compat/bswap.h | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/compat/bswap.h b/compat/bswap.h\nindex ed00f6d1d53f3..28635ebc690e3 100644\n--- a/compat/bswap.h\n+++ b/compat/bswap.h\n@@ -32,6 +32,14 @@ static inline uint64_t default_bswap64(uint64_t val)\n \t\t((val & (uint64_t)0xff00000000000000ULL) >> 56));\n }\n \n+/*\n+ * __has_builtin is available since Clang 10 and GCC 10.\n+ * Below is a fallback for older compilers.\n+ */\n+#ifndef __has_builtin\n+# define __has_builtin(x) 0\n+#endif\n+\n #undef bswap32\n #undef bswap64\n \n@@ -46,6 +54,11 @@ static inline uint64_t default_bswap64(uint64_t val)\n #define GIT_BIG_ENDIAN 4321\n #define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN\n \n+#elif __has_builtin(__builtin_bswap32) && __has_builtin(__builtin_bswap64)\n+\n+#define bswap32(x) __builtin_bswap32((x))\n+#define bswap64(x) __builtin_bswap64((x))\n+\n #endif\n \n #if defined(__BYTE_ORDER) && defined(__LITTLE_ENDIAN) && defined(__BIG_ENDIAN)\n-- \n2.50.0\n\n"},{"id":"522034","messageId":"20250715191230.12081-3-sebastian@breakpoint.cc","threadId":"63800","inReplyTo":"20250715191230.12081-1-sebastian@breakpoint.cc","subject":"[PATCH v3 2/5] bswap.h: Define GIT_LITTLE_ENDIAN on msvc as little endian","fromName":"Sebastian Andrzej Siewior","fromEmail":"sebastian@breakpoint.cc","sentAt":"2025-07-15T19:12:27Z","receivedAt":"2025-07-15T19:12:54Z","isPatch":true,"sender":{"key":"sebastian@breakpoint.cc","avatar":"https://avatars.githubusercontent.com/u/5932951?v=4"},"body":"The Microsoft Visual C++ (MSVC) compiler (as of Visual Studio 2022\nversion 17.13.6) does not define __BYTE_ORDER__ and its C-library does\nnot define __BYTE_ORDER. The compiler is supported only on arm64 and x86\nwhich are all little endian.\n\nDefine GIT_BYTE_ORDER on msvc as little endian to avoid further checks.\n\nSigned-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>\n---\n compat/bswap.h | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/compat/bswap.h b/compat/bswap.h\nindex 0a457542dd76a..fd604d9f7b74b 100644\n--- a/compat/bswap.h\n+++ b/compat/bswap.h\n@@ -81,6 +81,10 @@ static inline uint64_t git_bswap64(uint64_t x)\n #define bswap32(x) _byteswap_ulong(x)\n #define bswap64(x) _byteswap_uint64(x)\n \n+#define GIT_LITTLE_ENDIAN 1234\n+#define GIT_BIG_ENDIAN 4321\n+#define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN\n+\n #endif\n \n #if defined(bswap32)\n@@ -122,7 +126,7 @@ static inline uint64_t git_bswap64(uint64_t x)\n # define GIT_LITTLE_ENDIAN __ORDER_LITTLE_ENDIAN__\n # define GIT_BIG_ENDIAN __ORDER_BIG_ENDIAN__\n \n-#else\n+#elif !defined(GIT_BYTE_ORDER)\n \n # define GIT_BIG_ENDIAN 4321\n # define GIT_LITTLE_ENDIAN 1234\n-- \n2.50.0\n\n"},{"id":"522035","messageId":"20250715191230.12081-4-sebastian@breakpoint.cc","threadId":"63800","inReplyTo":"20250715191230.12081-1-sebastian@breakpoint.cc","subject":"[PATCH v3 3/5] bswap.h: Always overwrite ntohl/ ntohll macros","fromName":"Sebastian Andrzej Siewior","fromEmail":"sebastian@breakpoint.cc","sentAt":"2025-07-15T19:12:28Z","receivedAt":"2025-07-15T19:12:57Z","isPatch":true,"sender":{"key":"sebastian@breakpoint.cc","avatar":"https://avatars.githubusercontent.com/u/5932951?v=4"},"body":"The ntohl and htonl macros are redefined because the provided macros were\nnot always optimal. Sometimes it was a function call, sometimes it was a\nmacro which did the shifting. Using the 'bswap' opcode on x86 provides\nprobably better performance than performing the shifting.\nThese macros are only overwritten on x86 if the \"optimized\" version is\navailable.\n\nThe ntohll and htonll macros are not available on every platform (at\nleast glibc does not provide them) which means they need to be defined\nonce the endianness of the system is determined.\n\nIn order to get a more symmetrical setup, redfine the macros once the\nendianness of the system has been determined.\n\nSigned-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>\n---\n compat/bswap.h | 54 ++++++++++++++++++++++++--------------------------\n 1 file changed, 26 insertions(+), 28 deletions(-)\n\ndiff --git a/compat/bswap.h b/compat/bswap.h\nindex fd604d9f7b74b..aeef304f671f5 100644\n--- a/compat/bswap.h\n+++ b/compat/bswap.h\n@@ -87,27 +87,6 @@ static inline uint64_t git_bswap64(uint64_t x)\n \n #endif\n \n-#if defined(bswap32)\n-\n-#undef ntohl\n-#undef htonl\n-#define ntohl(x) bswap32(x)\n-#define htonl(x) bswap32(x)\n-\n-#endif\n-\n-#if defined(bswap64)\n-\n-#undef ntohll\n-#undef htonll\n-#define ntohll(x) bswap64(x)\n-#define htonll(x) bswap64(x)\n-\n-#else\n-\n-#undef ntohll\n-#undef htonll\n-\n #if defined(__BYTE_ORDER) && defined(__LITTLE_ENDIAN) && defined(__BIG_ENDIAN)\n \n # define GIT_BYTE_ORDER __BYTE_ORDER\n@@ -145,14 +124,33 @@ static inline uint64_t git_bswap64(uint64_t x)\n \n #endif\n \n-#if GIT_BYTE_ORDER == GIT_BIG_ENDIAN\n-# define ntohll(n) (n)\n-# define htonll(n) (n)\n-#else\n-# define ntohll(n) default_bswap64(n)\n-# define htonll(n) default_bswap64(n)\n-#endif\n+#undef ntohl\n+#undef htonl\n+#undef ntohll\n+#undef htonll\n \n+#if GIT_BYTE_ORDER == GIT_BIG_ENDIAN\n+# define ntohl(x) (x)\n+# define htonl(x) (x)\n+# define ntohll(x) (x)\n+# define htonll(x) (x)\n+#else\n+\n+# if defined(bswap32)\n+#  define ntohl(x) bswap32(x)\n+#  define htonl(x) bswap32(x)\n+# else\n+#  define ntohl(x) default_swab32(x)\n+#  define htonl(x) default_swab32(x)\n+# endif\n+\n+# if defined(bswap64)\n+#  define ntohll(x) bswap64(x)\n+#  define htonll(x) bswap64(x)\n+# else\n+#  define ntohll(x) default_bswap64(x)\n+#  define htonll(x) default_bswap64(x)\n+# endif\n #endif\n \n static inline uint16_t get_be16(const void *ptr)\n-- \n2.50.0\n\n"},{"id":"522050","messageId":"xmqqple1f7ge.fsf@gitster.g","threadId":"63800","inReplyTo":"20250715191230.12081-1-sebastian@breakpoint.cc","subject":"Re: [PATCH v3 0/5] bswap.h: Rework ntohl handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-15T21:52:01Z","receivedAt":"2025-07-15T21:52:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sebastian Andrzej Siewior <sebastian@breakpoint.cc> writes:\n\n> this series continues the rework of the bswap32/64()/ nothl() handling. \n>\n> I've been looking at recent compiler and they manage to recognize the\n> manual shifting and use an optimize opcode if available. The ntohl\n> version provided by glibc already provides an \"optimized\" version which\n> makes an optimisation in git almost not needed.\n> One of the motivation behind overwriting/ providing an optimized\n> version was to provide a macro instead of using a function call. One\n> libc that is still providing ntohl as a function call is musl.\n>\n> While ntohl() is provided by the libc, the ntohll() is not. I found it\n> only on Windows provided by winsock.h.\n>\n> I haven't touched the put/get_be*() macros. gcc & clang are both smart\n> enough to swap the content accordingly and perform a single store/ load.\n> Only the msvc seems to strugle here and performs multiple bytes stores/\n> loads and shifts.\n>\n> v2…v3 https://lore.kernel.org/all/20250611221444.1567638-1-sebastian@breakpoint.cc/\n>   - Fixed typos in the patch description\n\nThanks for updating the proposed log messages with typofixes.  I\nunderstand the patch text has no changes?\n\nCc'ed those who gave comments on the previous round.  How does this\nversion look to you folks?\n\nThanks.\n"},{"id":"522054","messageId":"aHbX4hIKbjO6h1R5@fruit.crustytoothpaste.net","threadId":"63800","inReplyTo":"xmqqple1f7ge.fsf@gitster.g","subject":"Re: [PATCH v3 0/5] bswap.h: Rework ntohl handling","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-07-15T22:36:18Z","receivedAt":"2025-07-15T22:36:20Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-07-15 at 21:52:01, Junio C Hamano wrote:\n> Thanks for updating the proposed log messages with typofixes.  I\n> understand the patch text has no changes?\n> \n> Cc'ed those who gave comments on the previous round.  How does this\n> version look to you folks?\n\nThis seems fine to me.\n\nI might go a little farther and simply say that recognizing the pattern\nand emitting the most efficient code, whether that's a single\ninstruction or an optimized pattern of instructions, is a\nquality-of-implementation issue in the compiler's peephole optimizer[0]\nand that we don't really need to use the builtin functions or provide\nspecial cases for MSVC still, but I realize that is not a very popular\nopinion and I think this series is fine without that.\n\n[0] Byte swapping, population count, and rotations are, in my view, the\nmost frequently open-coded functions and thus every compiler with a\npeephole optimizer should recognize and handle them.\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"522066","messageId":"20250716050111.cw6zt2Vs@breakpoint.cc","threadId":"63800","inReplyTo":"xmqqple1f7ge.fsf@gitster.g","subject":"Re: [PATCH v3 0/5] bswap.h: Rework ntohl handling","fromName":"Sebastian Andrzej Siewior","fromEmail":"sebastian@breakpoint.cc","sentAt":"2025-07-16T05:01:11Z","receivedAt":"2025-07-16T05:01:15Z","isPatch":true,"sender":{"key":"sebastian@breakpoint.cc","avatar":"https://avatars.githubusercontent.com/u/5932951?v=4"},"body":"On 2025-07-15 14:52:01 [-0700], Junio C Hamano wrote:\n> Sebastian Andrzej Siewior <sebastian@breakpoint.cc> writes:\n> \n> > this series continues the rework of the bswap32/64()/ nothl() handling. \n> >\n> > I've been looking at recent compiler and they manage to recognize the\n> > manual shifting and use an optimize opcode if available. The ntohl\n> > version provided by glibc already provides an \"optimized\" version which\n> > makes an optimisation in git almost not needed.\n> > One of the motivation behind overwriting/ providing an optimized\n> > version was to provide a macro instead of using a function call. One\n> > libc that is still providing ntohl as a function call is musl.\n> >\n> > While ntohl() is provided by the libc, the ntohll() is not. I found it\n> > only on Windows provided by winsock.h.\n> >\n> > I haven't touched the put/get_be*() macros. gcc & clang are both smart\n> > enough to swap the content accordingly and perform a single store/ load.\n> > Only the msvc seems to strugle here and performs multiple bytes stores/\n> > loads and shifts.\n> >\n> > v2…v3 https://lore.kernel.org/all/20250611221444.1567638-1-sebastian@breakpoint.cc/\n> >   - Fixed typos in the patch description\n> \n> Thanks for updating the proposed log messages with typofixes.  I\n> understand the patch text has no changes?\n\nThe patches iteself remain unchanged, only the patch description of a\nfew got updated.\nThere was only one comment regarding using endian(3). At the same time I\ngot the feeling to first get this through and then we can think how to\ncontinue further. glibc optimizes ntohl already so git might not have\nto. I only found musl that has a function call. I didn't look on *BSD.\nIt just something is needed for ntohll().\n\n> Thanks.\n\nSebastian\n"}]}