Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Support, ADT] Move llvm::endianness to bit.h #68280

Merged
merged 2 commits into from
Oct 6, 2023

Conversation

kazutakahirata
Copy link
Contributor

@kazutakahirata kazutakahirata commented Oct 5, 2023

In C++20, std::endian comes from <bit>. Following the same spirit,
this patch moves llvm::endianness and the endian detection logic to
bit.h.

Without this patch, llvm::endianness::native is defined in terms of
llvm::sys::IsBigEndianHost. This patch reverses the dependency.
llvm::sys::IsBigEndianHost is now defined in terms of
llvm::endianness::native.

In C++20, std::endian comes from <bit>.  Following the same spirit,
this patch moves llvm::endianness and the endian detection logic to
bit.h.

Without this patch, llvm::endianness::native is defined in terms of
llvm::sys::IsBigEndianHost.  This patch reverses the dependency.
llvm::sys::IsBigEndianHost is now defined in terms of
llvm::endianness::native.
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 5, 2023

@llvm/pr-subscribers-llvm-support

@llvm/pr-subscribers-llvm-adt

Changes

In C++20, std::endian comes from <bit>. Following the same spirit,
this patch moves llvm::endianness and the endian detection logic to
bit.h.

Without this patch, llvm::endianness::native is defined in terms of
llvm::sys::IsBigEndianHost. This patch reverses the dependency.
llvm::sys::IsBigEndianHost is now defined in terms of
llvm::endianness::native.


Full diff: https://github.com/llvm/llvm-project/pull/68280.diff

3 Files Affected:

  • (modified) llvm/include/llvm/ADT/bit.h (+35)
  • (modified) llvm/include/llvm/Support/Endian.h (-7)
  • (modified) llvm/include/llvm/Support/SwapByteOrder.h (+2-30)
diff --git a/llvm/include/llvm/ADT/bit.h b/llvm/include/llvm/ADT/bit.h
index 2840c5f608d3ea8..0a228b2204d6fc2 100644
--- a/llvm/include/llvm/ADT/bit.h
+++ b/llvm/include/llvm/ADT/bit.h
@@ -27,6 +27,31 @@
 #include <cstdlib>  // for _byteswap_{ushort,ulong,uint64}
 #endif
 
+#if defined(__linux__) || defined(__GNU__) || defined(__HAIKU__) ||            \
+    defined(__Fuchsia__) || defined(__EMSCRIPTEN__)
+#include <endian.h>
+#elif defined(_AIX)
+#include <sys/machine.h>
+#elif defined(__sun)
+/* Solaris provides _BIG_ENDIAN/_LITTLE_ENDIAN selector in sys/types.h */
+#include <sys/types.h>
+#define BIG_ENDIAN 4321
+#define LITTLE_ENDIAN 1234
+#if defined(_BIG_ENDIAN)
+#define BYTE_ORDER BIG_ENDIAN
+#else
+#define BYTE_ORDER LITTLE_ENDIAN
+#endif
+#elif defined(__MVS__)
+#define BIG_ENDIAN 4321
+#define LITTLE_ENDIAN 1234
+#define BYTE_ORDER BIG_ENDIAN
+#else
+#if !defined(BYTE_ORDER) && !defined(_WIN32)
+#include <machine/endian.h>
+#endif
+#endif
+
 #ifdef _MSC_VER
 // Declare these intrinsics manually rather including intrin.h. It's very
 // expensive, and bit.h is popular via MathExtras.h.
@@ -41,6 +66,16 @@ unsigned char _BitScanReverse64(unsigned long *_Index, unsigned __int64 _Mask);
 
 namespace llvm {
 
+enum class endianness {
+  big,
+  little,
+#if defined(BYTE_ORDER) && defined(BIG_ENDIAN) && BYTE_ORDER == BIG_ENDIAN
+  native = big
+#else
+  native = little
+#endif
+};
+
 // This implementation of bit_cast is different from the C++20 one in two ways:
 //  - It isn't constexpr because that requires compiler support.
 //  - It requires trivially-constructible To, to avoid UB in the implementation.
diff --git a/llvm/include/llvm/Support/Endian.h b/llvm/include/llvm/Support/Endian.h
index e0afeabbcf34a0e..28a1d2da9bb5b15 100644
--- a/llvm/include/llvm/Support/Endian.h
+++ b/llvm/include/llvm/Support/Endian.h
@@ -22,13 +22,6 @@
 #include <type_traits>
 
 namespace llvm {
-
-enum class endianness {
-  big,
-  little,
-  native = llvm::sys::IsBigEndianHost ? big : little
-};
-
 namespace support {
 
 // TODO: Remove the following once we are done migrating to llvm::endianness,
diff --git a/llvm/include/llvm/Support/SwapByteOrder.h b/llvm/include/llvm/Support/SwapByteOrder.h
index 43fa5347d6fe851..8f26af6f68ac65f 100644
--- a/llvm/include/llvm/Support/SwapByteOrder.h
+++ b/llvm/include/llvm/Support/SwapByteOrder.h
@@ -19,40 +19,12 @@
 #include <cstdint>
 #include <type_traits>
 
-#if defined(__linux__) || defined(__GNU__) || defined(__HAIKU__) ||            \
-    defined(__Fuchsia__) || defined(__EMSCRIPTEN__)
-#include <endian.h>
-#elif defined(_AIX)
-#include <sys/machine.h>
-#elif defined(__sun)
-/* Solaris provides _BIG_ENDIAN/_LITTLE_ENDIAN selector in sys/types.h */
-#include <sys/types.h>
-#define BIG_ENDIAN 4321
-#define LITTLE_ENDIAN 1234
-#if defined(_BIG_ENDIAN)
-#define BYTE_ORDER BIG_ENDIAN
-#else
-#define BYTE_ORDER LITTLE_ENDIAN
-#endif
-#elif defined(__MVS__)
-#define BIG_ENDIAN 4321
-#define LITTLE_ENDIAN 1234
-#define BYTE_ORDER BIG_ENDIAN
-#else
-#if !defined(BYTE_ORDER) && !defined(_WIN32)
-#include <machine/endian.h>
-#endif
-#endif
-
 namespace llvm {
 
 namespace sys {
 
-#if defined(BYTE_ORDER) && defined(BIG_ENDIAN) && BYTE_ORDER == BIG_ENDIAN
-constexpr bool IsBigEndianHost = true;
-#else
-constexpr bool IsBigEndianHost = false;
-#endif
+constexpr bool IsBigEndianHost =
+    llvm::endianness::native == llvm::endianness::big;
 
 static const bool IsLittleEndianHost = !IsBigEndianHost;
 

Copy link
Member

@zero9178 zero9178 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % one comment

llvm/include/llvm/Support/Endian.h Show resolved Hide resolved
@kazutakahirata kazutakahirata merged commit 86d1f4c into llvm:main Oct 6, 2023
3 checks passed
@kazutakahirata kazutakahirata deleted the pr_endian_move_to_bit branch October 6, 2023 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants