{"thread":{"id":"63628","subject":"[PATCH v2 2/6] bswap.h: Add support for __BYTE_ORDER__","startedAt":"2025-06-11T22:15:01Z","lastAt":"2025-07-15T19:02:39Z","messageCount":20,"participants":["Sebastian Andrzej Siewior","brian m. carlson","Collin Funk","Junio C Hamano","Kristoffer Haugsbakk"],"isPatch":true,"patchVersion":2,"patchTotal":6},"messages":[{"id":"520125","messageId":"20250611221444.1567638-3-sebastian@breakpoint.cc","threadId":"63628","inReplyTo":"20250611221444.1567638-1-sebastian@breakpoint.cc","subject":"[PATCH v2 2/6] bswap.h: Add support for __BYTE_ORDER__","fromName":"Sebastian Andrzej Siewior","fromEmail":"sebastian@breakpoint.cc","sentAt":"2025-06-11T22:14:38Z","receivedAt":"2025-06-11T22:15:01Z","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). It is not provided by msvc as of\nv19.43 / 17.13.6.\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.49.0\n\n"},{"id":"520126","messageId":"20250611221444.1567638-2-sebastian@breakpoint.cc","threadId":"63628","inReplyTo":"20250611221444.1567638-1-sebastian@breakpoint.cc","subject":"[PATCH v2 1/6] Revert \"bswap.h: add support for built-in bswap functions\"","fromName":"Sebastian Andrzej Siewior","fromEmail":"sebastian@breakpoint.cc","sentAt":"2025-06-11T22:14:37Z","receivedAt":"2025-06-11T22:15:01Z","isPatch":true,"sender":{"key":"sebastian@breakpoint.cc","avatar":"https://avatars.githubusercontent.com/u/5932951?v=4"},"body":"Since 6547d1c9 (bswap.h: add support for built-in bswap\nfunctions, 2025-04-23) tweaked the way the bswap32/64 macros are\ndefined, on platforms with __builtin_bswap32/64 supported, the\nbswap32/64 macros are defined even on big endian platforms.\n\nHowever this file assumes that bswap31/64 are defined ONLY on\nlittle endian machines and uses that assumption to redefine\nntohl/ntohll macros. The said commit broke t4014-format-patch.sh test,\namong many others on s390x.\n\nRevert the commit.\n\nSigned-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>\n---\n compat/bswap.h | 14 +-------------\n 1 file changed, 1 insertion(+), 13 deletions(-)\n\ndiff --git a/compat/bswap.h b/compat/bswap.h\nindex 9e0f98e00b93a..b34054f2bd728 100644\n--- a/compat/bswap.h\n+++ b/compat/bswap.h\n@@ -35,19 +35,7 @@ static inline uint64_t default_bswap64(uint64_t val)\n #undef bswap32\n #undef bswap64\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-\t#define __has_builtin(x) 0\n-#endif\n-\n-#if __has_builtin(__builtin_bswap32) && __has_builtin(__builtin_bswap64)\n-#define bswap32(x) __builtin_bswap32((x))\n-#define bswap64(x) __builtin_bswap64((x))\n-\n-#elif defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))\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-- \n2.49.0\n\n"},{"id":"520127","messageId":"20250611221444.1567638-4-sebastian@breakpoint.cc","threadId":"63628","inReplyTo":"20250611221444.1567638-1-sebastian@breakpoint.cc","subject":"[PATCH v2 3/6] bswap.h: Define GIT_LITTLE_ENDIAN on msvc as little endian","fromName":"Sebastian Andrzej Siewior","fromEmail":"sebastian@breakpoint.cc","sentAt":"2025-06-11T22:14:39Z","receivedAt":"2025-06-11T22:15:01Z","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 an 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.49.0\n\n"},{"id":"520128","messageId":"20250611221444.1567638-6-sebastian@breakpoint.cc","threadId":"63628","inReplyTo":"20250611221444.1567638-1-sebastian@breakpoint.cc","subject":"[PATCH v2 5/6] bswap.h: Remove optimized x86 version of bswap32/64","fromName":"Sebastian Andrzej Siewior","fromEmail":"sebastian@breakpoint.cc","sentAt":"2025-06-11T22:14:41Z","receivedAt":"2025-06-11T22:15:01Z","isPatch":true,"sender":{"key":"sebastian@breakpoint.cc","avatar":"https://avatars.githubusercontent.com/u/5932951?v=4"},"body":"On x86 the bswap32/64 macro is implemenated based on the x86 opcode wich\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() otimized\n\n- x86, arm64 msvc v19.37\n  default_swab32() optimized\n  default_bswap64() optimized\n  _byteswap_uint64() otimized\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.49.0\n\n"},{"id":"520129","messageId":"20250611221444.1567638-1-sebastian@breakpoint.cc","threadId":"63628","inReplyTo":null,"subject":"[PATCH v2 0/6] bswap.h: Rework ntohl handling","fromName":"Sebastian Andrzej Siewior","fromEmail":"sebastian@breakpoint.cc","sentAt":"2025-06-11T22:14:36Z","receivedAt":"2025-06-11T22:15:01Z","isPatch":true,"sender":{"key":"sebastian@breakpoint.cc","avatar":"https://avatars.githubusercontent.com/u/5932951?v=4"},"body":"Hi,\n\nsince the add of bswap32/64() support based on __builtin support, the\nusage of ntohl() macros is broken on big endian machines because the\nmacros are always overwritten providing a swap.\n\nThe first patch in the series reverts the change and the following\npatches try to improve the situation by allowing to always provide an\noptimized version.\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. So it\nmight makes sense to keep it.\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\nSebastian Andrzej Siewior (6):\n  Revert \"bswap.h: add support for built-in bswap functions\"\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 | 118 ++++++++++++++++++-------------------------------\n 1 file changed, 44 insertions(+), 74 deletions(-)\n\n-- \n2.49.0\n\n"},{"id":"520130","messageId":"20250611221444.1567638-8-sebastian@breakpoint.cc","threadId":"63628","inReplyTo":"20250611221444.1567638-1-sebastian@breakpoint.cc","subject":"[PATCH v2 6/6] bswap.h: Provide a built-in based version of bswap32/64 if possible","fromName":"Sebastian Andrzej Siewior","fromEmail":"sebastian@breakpoint.cc","sentAt":"2025-06-11T22:14:43Z","receivedAt":"2025-06-11T22:15:04Z","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.49.0\n\n"},{"id":"520131","messageId":"20250611221444.1567638-7-sebastian@breakpoint.cc","threadId":"63628","inReplyTo":"20250611221444.1567638-1-sebastian@breakpoint.cc","subject":"[PATCH v2 5/6] bswap: Remove optimized x86 version of bswap32/64","fromName":"Sebastian Andrzej Siewior","fromEmail":"sebastian@breakpoint.cc","sentAt":"2025-06-11T22:14:42Z","receivedAt":"2025-06-11T22:15:04Z","isPatch":true,"sender":{"key":"sebastian@breakpoint.cc","avatar":"https://avatars.githubusercontent.com/u/5932951?v=4"},"body":"On x86 the bswap32/64 macro is implemenated based on the x86 opcode wich\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() otimized\n\n- x86, arm64 msvc v19.37\n  default_swab32() optimized\n  default_bswap64() optimized\n  _byteswap_uint64() otimized\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.49.0\n\n"},{"id":"520132","messageId":"20250611221444.1567638-5-sebastian@breakpoint.cc","threadId":"63628","inReplyTo":"20250611221444.1567638-1-sebastian@breakpoint.cc","subject":"[PATCH v2 4/6] bswap.h: Always overwrite ntohl/ ntohll macros","fromName":"Sebastian Andrzej Siewior","fromEmail":"sebastian@breakpoint.cc","sentAt":"2025-06-11T22:14:40Z","receivedAt":"2025-06-11T22:15:05Z","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.49.0\n\n"},{"id":"520137","messageId":"aEob-FXAvc2Wqmlr@fruit.crustytoothpaste.net","threadId":"63628","inReplyTo":"20250611221444.1567638-5-sebastian@breakpoint.cc","subject":"Re: [PATCH v2 4/6] bswap.h: Always overwrite ntohl/ ntohll macros","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-06-12T00:14:48Z","receivedAt":"2025-06-12T00:14:50Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-06-11 at 22:14:40, Sebastian Andrzej Siewior wrote:\n> The ntohl and htonl macros are redefined because the provided macros were\n> not always optimal. Sometimes it was a function call, sometimes it was a\n> macro which did the shifting. Using the 'bswap' opcode on x86 provides\n> probably better performance than performing the shifting.\n\nI believe that the peephole optimizer will almost always optimize them\nto the bswap or equivalent opcode, much like it recognizes how to\ngenerate rotate opcodes from two shifts and an or, so they should\nactually be equivalent.\n\nGCC and clang both emit simple bswap instructions with `-O2`, which is\nthe optimization level we use: https://godbolt.org/z/1r8P1Pqo7.\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"520140","messageId":"aEoeUCcXYByjU5HR@fruit.crustytoothpaste.net","threadId":"63628","inReplyTo":"20250611221444.1567638-3-sebastian@breakpoint.cc","subject":"Re: [PATCH v2 2/6] bswap.h: Add support for __BYTE_ORDER__","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-06-12T00:24:48Z","receivedAt":"2025-06-12T00:24:50Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-06-11 at 22:14:38, Sebastian Andrzej Siewior wrote:\n> The __BYTE_ORDER__ define is provided by gcc (since ~v4.6), clang\n> (since ~v3.2) and icc (since ~16.0.3). It is not provided by msvc as of\n> v19.43 / 17.13.6.\n> The __BYTE_ORDER and BYTE_ORDER macros are libc specific and are not\n> available on all supported platforms such as mingw.\n> \n> Add support for the __BYTE_ORDER__ macro as a fallback.\n> \n> Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>\n> ---\n>  compat/bswap.h | 6 ++++++\n>  1 file changed, 6 insertions(+)\n> \n> diff --git a/compat/bswap.h b/compat/bswap.h\n> index 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\nOne additional option you can add if you want (it's completely optional)\nis that if `__STDC_VERSION__` is 202311L or larger, then you can\n`#include <stdbit.h>`, which has `__STDC_ENDIAN_LITTLE__`,\n`__STDC_ENDIAN_BIG__`, and `__STDC_ENDIAN_NATIVE__`.\n\nThat will work on a modern GCC or clang with an appropriate compiler\nflag.  I don't know about MSVC, but it might be an option for people who\nwant to use more esoteric compilers which support standards but aren't\nvery compatible with GCC and clang.\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"520141","messageId":"87wm9hkcvr.fsf@gmail.com","threadId":"63628","inReplyTo":"aEoeUCcXYByjU5HR@fruit.crustytoothpaste.net","subject":"Re: [PATCH v2 2/6] bswap.h: Add support for __BYTE_ORDER__","fromName":"Collin Funk","fromEmail":"collin.funk1@gmail.com","sentAt":"2025-06-12T00:43:52Z","receivedAt":"2025-06-12T00:43:54Z","isPatch":true,"sender":{"key":"collin.funk1@gmail.com","avatar":"https://avatars.githubusercontent.com/u/65689063?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> One additional option you can add if you want (it's completely optional)\n> is that if `__STDC_VERSION__` is 202311L or larger, then you can\n> `#include <stdbit.h>`, which has `__STDC_ENDIAN_LITTLE__`,\n> `__STDC_ENDIAN_BIG__`, and `__STDC_ENDIAN_NATIVE__`.\n>\n> That will work on a modern GCC or clang with an appropriate compiler\n> flag.  I don't know about MSVC, but it might be an option for people who\n> want to use more esoteric compilers which support standards but aren't\n> very compatible with GCC and clang.\n\nAnother standard that is too new to be common, but POSIX.1-2024 added\n<endian.h> with BYTE_ORDER, LITTLE_ENDIAN, and BIG_ENDIAN [1].\n\nIn practice, some systems will have it because glibc had it for a long\ntime.\n\nCollin\n\n[1] https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/endian.h.html\n"},{"id":"520184","messageId":"xmqq34c4enge.fsf@gitster.g","threadId":"63628","inReplyTo":"20250611221444.1567638-2-sebastian@breakpoint.cc","subject":"Re: [PATCH v2 1/6] Revert \"bswap.h: add support for built-in bswap functions\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-12T20:04:17Z","receivedAt":"2025-06-12T20:04:20Z","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> Since 6547d1c9 (bswap.h: add support for built-in bswap\n> functions, 2025-04-23) tweaked the way the bswap32/64 macros are\n> defined, on platforms with __builtin_bswap32/64 supported, the\n> bswap32/64 macros are defined even on big endian platforms.\n>\n> However this file assumes that bswap31/64 are defined ONLY on\n\n31/64 -> 32/64?  Just sanity checking the typofix I plan to make\nlocally while applying this patch.\n\n> little endian machines and uses that assumption to redefine\n> ntohl/ntohll macros. The said commit broke t4014-format-patch.sh test,\n> among many others on s390x.\n>\n> Revert the commit.\n>\n> Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>\n> ---\n>  compat/bswap.h | 14 +-------------\n>  1 file changed, 1 insertion(+), 13 deletions(-)\n>\n> diff --git a/compat/bswap.h b/compat/bswap.h\n> index 9e0f98e00b93a..b34054f2bd728 100644\n> --- a/compat/bswap.h\n> +++ b/compat/bswap.h\n> @@ -35,19 +35,7 @@ static inline uint64_t default_bswap64(uint64_t val)\n>  #undef bswap32\n>  #undef bswap64\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> -\t#define __has_builtin(x) 0\n> -#endif\n> -\n> -#if __has_builtin(__builtin_bswap32) && __has_builtin(__builtin_bswap64)\n> -#define bswap32(x) __builtin_bswap32((x))\n> -#define bswap64(x) __builtin_bswap64((x))\n> -\n> -#elif defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))\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"},{"id":"520185","messageId":"xmqqv7p0d8m0.fsf@gitster.g","threadId":"63628","inReplyTo":"20250611221444.1567638-3-sebastian@breakpoint.cc","subject":"Re: [PATCH v2 2/6] bswap.h: Add support for __BYTE_ORDER__","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-12T20:10:15Z","receivedAt":"2025-06-12T20:10:17Z","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> The __BYTE_ORDER__ define is provided by gcc (since ~v4.6), clang\n> (since ~v3.2) and icc (since ~16.0.3). It is not provided by msvc as of\n> v19.43 / 17.13.6.\n\nI think it is more confusing than illuninating to have the last\nsentence in the above paragraph.  There probably are tons of other\ncompilers that do not support it, and this patch does not target any\nof them, including MSVC, which is the topic of the next step anyway.\n\nSo, I'm inclined to just remove the sentence, or replace it with\nsomething like \"Even recent versions of MSVC do not support it,\nwhich will be dealt with in the next patch.\"\n\n> The __BYTE_ORDER and BYTE_ORDER macros are libc specific and are not\n> available on all supported platforms such as mingw.\n>\n> Add support for the __BYTE_ORDER__ macro as a fallback.\n>\n> Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>\n> ---\n>  compat/bswap.h | 6 ++++++\n>  1 file changed, 6 insertions(+)\n>\n> diff --git a/compat/bswap.h b/compat/bswap.h\n> index 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"},{"id":"520186","messageId":"xmqqqzzod8k0.fsf@gitster.g","threadId":"63628","inReplyTo":"20250611221444.1567638-4-sebastian@breakpoint.cc","subject":"Re: [PATCH v2 3/6] bswap.h: Define GIT_LITTLE_ENDIAN on msvc as little endian","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-12T20:11:27Z","receivedAt":"2025-06-12T20:11:30Z","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> The Microsoft Visual C++ (MSVC) compiler (as of Visual Studio 2022\n> version 17.13.6) does not define __BYTE_ORDER__ and its C-library does\n> not define __BYTE_ORDER. The compiler is supported only an arm64 and x86\n> which are all little endian.\n\n\"an arm\" -> \"on arm\"?  Just sanity checking before making local\ntypofix while queueing.\n\n\n> Define GIT_BYTE_ORDER on msvc as little endian to avoid further checks.\n>\n> Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>\n> ---\n>  compat/bswap.h | 6 +++++-\n>  1 file changed, 5 insertions(+), 1 deletion(-)\n>\n> diff --git a/compat/bswap.h b/compat/bswap.h\n> index 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"},{"id":"520187","messageId":"xmqqmsacd8cc.fsf@gitster.g","threadId":"63628","inReplyTo":"aEob-FXAvc2Wqmlr@fruit.crustytoothpaste.net","subject":"Re: [PATCH v2 4/6] bswap.h: Always overwrite ntohl/ ntohll macros","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-12T20:16:03Z","receivedAt":"2025-06-12T20:16:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> On 2025-06-11 at 22:14:40, Sebastian Andrzej Siewior wrote:\n>> The ntohl and htonl macros are redefined because the provided macros were\n>> not always optimal. Sometimes it was a function call, sometimes it was a\n>> macro which did the shifting. Using the 'bswap' opcode on x86 provides\n>> probably better performance than performing the shifting.\n>\n> I believe that the peephole optimizer will almost always optimize them\n> to the bswap or equivalent opcode, much like it recognizes how to\n> generate rotate opcodes from two shifts and an or, so they should\n> actually be equivalent.\n>\n> GCC and clang both emit simple bswap instructions with `-O2`, which is\n> the optimization level we use: https://godbolt.org/z/1r8P1Pqo7.\n\nGood observation.  In short, we do not have to redefine these in\nterms of bswap32/64 for performance as the compilers should do a\nreasonable job.\n\nThe updated organization to separate the two concerns in this file,\nnamely, (1) figure out the best way to write bswap32/64, and (2)\noverride (or supply on platforms that do not offer) host-network\nbyte order helpers, does clean things up a lot, so I still like to\nsee it, though.\n\n"},{"id":"520188","messageId":"xmqqcyb8d83l.fsf@gitster.g","threadId":"63628","inReplyTo":"20250611221444.1567638-1-sebastian@breakpoint.cc","subject":"Re: [PATCH v2 0/6] bswap.h: Rework ntohl handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-12T20:21:18Z","receivedAt":"2025-06-12T20:21:21Z","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> since the add of bswap32/64() support based on __builtin support, the\n> usage of ntohl() macros is broken on big endian machines because the\n> macros are always overwritten providing a swap.\n>\n> The first patch in the series reverts the change and the following\n> patches try to improve the situation by allowing to always provide an\n> optimized version.\n\nI am inclined to take only this one for the upcoming release, as a\npure regression fix.  It's not like with the change reverted, those\nplatforms with __builtin_bswap32/64 will stop building correctly.\nThe worst is that they fall back to the implementation they have\nalways been using until any and all released versions of Git.\n\nThe rest looked promising and with very good materials, but let's\nleave them for the cycle after 2.50 ships.\n\nThanks.\n"},{"id":"520767","messageId":"e00dff7e-1920-45b6-85d2-49e6a1218882@app.fastmail.com","threadId":"63628","inReplyTo":"20250611221444.1567638-6-sebastian@breakpoint.cc","subject":"Re: [PATCH v2 5/6] bswap.h: Remove optimized x86 version of bswap32/64","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2025-06-26T15:55:43Z","receivedAt":"2025-06-26T15:56:04Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Thu, Jun 12, 2025, at 00:14, Sebastian Andrzej Siewior wrote:\n> On x86 the bswap32/64 macro is implemenated based on the x86 opcode wich\n\ns/implemenated/implemented/\n\ns/wich/which/\n\n> performs the required shifting in just one opcode.\n> The other CPUs fallback to the generic shifting as implemented by\n> default_swab32() and default_bswap64() if needed.\n>\n> I've been looking at how good a compiler is at recognizing the default\n> shift and emitting an optimized operation:\n> - x86, arm64 msvc v19.20\n>   default_swab32() optimized\n>   default_bswap64() shifts\n>   _byteswap_uint64() otimized\n>\n> - x86, arm64 msvc v19.37\n>   default_swab32() optimized\n>   default_bswap64() optimized\n>   _byteswap_uint64() otimized\n\nIs it supposed to say `otimized` twice?\n"},{"id":"521482","messageId":"xmqqo6tv1v1z.fsf@gitster.g","threadId":"63628","inReplyTo":"20250611221444.1567638-1-sebastian@breakpoint.cc","subject":"Re: [PATCH v2 0/6] bswap.h: Rework ntohl handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-07T22:43:20Z","receivedAt":"2025-07-07T22:43:22Z","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> since the add of bswap32/64() support based on __builtin support, the\n> usage of ntohl() macros is broken on big endian machines because the\n> macros are always overwritten providing a swap.\n>\n> The first patch in the series reverts the change and the following\n> patches try to improve the situation by allowing to always provide an\n> optimized version.\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. So it\n> might makes sense to keep it.\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\nNow, I see many comments and suggestions on the thread since this v2\niteration was posted:\n\nhttps://lore.kernel.org/git/20250611221444.1567638-1-sebastian@breakpoint.cc/\n\nand I think the ball is in the author's court to respond.\n\nI'll mark the topic as Stalled in the draft of the next issue of\n\"What's cooking\" report.\n\nThanks.\n\n"},{"id":"521607","messageId":"20250709060412.BnVWKovB@breakpoint.cc","threadId":"63628","inReplyTo":"xmqqo6tv1v1z.fsf@gitster.g","subject":"Re: [PATCH v2 0/6] bswap.h: Rework ntohl handling","fromName":"Sebastian Andrzej Siewior","fromEmail":"sebastian@breakpoint.cc","sentAt":"2025-07-09T06:04:12Z","receivedAt":"2025-07-09T06:04:16Z","isPatch":true,"sender":{"key":"sebastian@breakpoint.cc","avatar":"https://avatars.githubusercontent.com/u/5932951?v=4"},"body":"On 2025-07-07 15:43:20 [-0700], Junio C Hamano wrote:\n> \n> Now, I see many comments and suggestions on the thread since this v2\n> iteration was posted:\n> \n> https://lore.kernel.org/git/20250611221444.1567638-1-sebastian@breakpoint.cc/\n> \n> and I think the ball is in the author's court to respond.\n> \n> I'll mark the topic as Stalled in the draft of the next issue of\n> \"What's cooking\" report.\n\nSorry for the delay, thank you for the reminder.\n\n> Thanks.\n\nSebastian\n"},{"id":"522029","messageId":"20250715190229.ed-A7kgh@breakpoint.cc","threadId":"63628","inReplyTo":"e00dff7e-1920-45b6-85d2-49e6a1218882@app.fastmail.com","subject":"Re: [PATCH v2 5/6] bswap.h: Remove optimized x86 version of bswap32/64","fromName":"Sebastian Andrzej Siewior","fromEmail":"sebastian@breakpoint.cc","sentAt":"2025-07-15T19:02:29Z","receivedAt":"2025-07-15T19:02:39Z","isPatch":true,"sender":{"key":"sebastian@breakpoint.cc","avatar":"https://avatars.githubusercontent.com/u/5932951?v=4"},"body":"On 2025-06-26 17:55:43 [+0200], Kristoffer Haugsbakk wrote:\n> On Thu, Jun 12, 2025, at 00:14, Sebastian Andrzej Siewior wrote:\n> > On x86 the bswap32/64 macro is implemenated based on the x86 opcode wich\n> \n> s/implemenated/implemented/\n> \n> s/wich/which/\n> \n> > performs the required shifting in just one opcode.\n> > The other CPUs fallback to the generic shifting as implemented by\n> > default_swab32() and default_bswap64() if needed.\n> >\n> > I've been looking at how good a compiler is at recognizing the default\n> > shift and emitting an optimized operation:\n> > - x86, arm64 msvc v19.20\n> >   default_swab32() optimized\n> >   default_bswap64() shifts\n> >   _byteswap_uint64() otimized\n> >\n> > - x86, arm64 msvc v19.37\n> >   default_swab32() optimized\n> >   default_bswap64() optimized\n> >   _byteswap_uint64() otimized\n> \n> Is it supposed to say `otimized` twice?\n\nNope. Fixed. Thank you.\n\nSebastian\n"}]}