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

[libc] Remove obsolete LIBC_HAS_BUILTIN macro #86554

Merged
merged 4 commits into from Mar 27, 2024

Conversation

marcauberer
Copy link
Member

Fixes #86546 and removes the macro LIBC_HAS_BUILTIN. This was necessary to support older compilers that did not support __has_builtin. All of the compilers we support already have this builtin.
See: https://libc.llvm.org/compiler_support.html
All uses now use __has_builtin directly

cc @nickdesaulniers

@llvmbot llvmbot added the libc label Mar 25, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 25, 2024

@llvm/pr-subscribers-libc

Author: Marc Auberer (marcauberer)

Changes

Fixes #86546 and removes the macro LIBC_HAS_BUILTIN. This was necessary to support older compilers that did not support __has_builtin. All of the compilers we support already have this builtin.
See: https://libc.llvm.org/compiler_support.html
All uses now use __has_builtin directly

cc @nickdesaulniers


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

19 Files Affected:

  • (modified) libc/docs/dev/code_style.rst (+1-1)
  • (modified) libc/src/__support/CPP/atomic.h (+8-8)
  • (modified) libc/src/__support/CPP/bit.h (+7-8)
  • (modified) libc/src/__support/CPP/type_traits/is_destructible.h (+1-1)
  • (modified) libc/src/__support/CPP/type_traits/is_function.h (+1-1)
  • (modified) libc/src/__support/CPP/type_traits/is_lvalue_reference.h (+1-1)
  • (modified) libc/src/__support/CPP/type_traits/is_reference.h (+1-1)
  • (modified) libc/src/__support/CPP/type_traits/is_rvalue_reference.h (+1-1)
  • (modified) libc/src/__support/CPP/type_traits/is_trivially_destructible.h (+2-2)
  • (modified) libc/src/__support/CPP/type_traits/remove_all_extents.h (+1-1)
  • (modified) libc/src/__support/FPUtil/FEnvImpl.h (-1)
  • (modified) libc/src/__support/FPUtil/gpu/FMA.h (+3-3)
  • (modified) libc/src/__support/macros/config.h (-18)
  • (modified) libc/src/__support/macros/optimization.h (-1)
  • (modified) libc/src/__support/macros/sanitizer.h (+1-2)
  • (modified) libc/src/__support/math_extras.h (+4-5)
  • (modified) libc/src/__support/memory_size.h (+1-1)
  • (modified) libc/src/string/memory_utils/generic/builtin.h (+3-4)
  • (modified) libc/src/string/memory_utils/utils.h (+2-3)
diff --git a/libc/docs/dev/code_style.rst b/libc/docs/dev/code_style.rst
index e6fc6df5a0f6b3..22a18b7a4cc1dd 100644
--- a/libc/docs/dev/code_style.rst
+++ b/libc/docs/dev/code_style.rst
@@ -55,7 +55,7 @@ We define two kinds of macros:
    * ``src/__support/macros/config.h`` - Important compiler and platform
      features. Such macros can be used to produce portable code by
      parameterizing compilation based on the presence or lack of a given
-     feature. e.g., ``LIBC_HAS_BUILTIN``
+     feature. e.g., ``LIBC_HAS_FEATURE``
    * ``src/__support/macros/attributes.h`` - Attributes for functions, types,
      and variables. e.g., ``LIBC_UNUSED``
    * ``src/__support/macros/optimization.h`` - Portable macros for performance
diff --git a/libc/src/__support/CPP/atomic.h b/libc/src/__support/CPP/atomic.h
index b74cb5981dbaf8..bb099e09221537 100644
--- a/libc/src/__support/CPP/atomic.h
+++ b/libc/src/__support/CPP/atomic.h
@@ -71,7 +71,7 @@ template <typename T> struct Atomic {
 
   T load(MemoryOrder mem_ord = MemoryOrder::SEQ_CST,
          [[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
-    if constexpr (LIBC_HAS_BUILTIN(__scoped_atomic_load_n))
+    if constexpr (__has_builtin(__scoped_atomic_load_n))
       return __scoped_atomic_load_n(&val, int(mem_ord), (int)(mem_scope));
     else
       return __atomic_load_n(&val, int(mem_ord));
@@ -85,7 +85,7 @@ template <typename T> struct Atomic {
 
   void store(T rhs, MemoryOrder mem_ord = MemoryOrder::SEQ_CST,
              [[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
-    if constexpr (LIBC_HAS_BUILTIN(__scoped_atomic_store_n))
+    if constexpr (__has_builtin(__scoped_atomic_store_n))
       __scoped_atomic_store_n(&val, rhs, int(mem_ord), (int)(mem_scope));
     else
       __atomic_store_n(&val, rhs, int(mem_ord));
@@ -101,7 +101,7 @@ template <typename T> struct Atomic {
 
   T exchange(T desired, MemoryOrder mem_ord = MemoryOrder::SEQ_CST,
              [[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
-    if constexpr (LIBC_HAS_BUILTIN(__scoped_atomic_exchange_n))
+    if constexpr (__has_builtin(__scoped_atomic_exchange_n))
       return __scoped_atomic_exchange_n(&val, desired, int(mem_ord),
                                         (int)(mem_scope));
     else
@@ -110,7 +110,7 @@ template <typename T> struct Atomic {
 
   T fetch_add(T increment, MemoryOrder mem_ord = MemoryOrder::SEQ_CST,
               [[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
-    if constexpr (LIBC_HAS_BUILTIN(__scoped_atomic_fetch_add))
+    if constexpr (__has_builtin(__scoped_atomic_fetch_add))
       return __scoped_atomic_fetch_add(&val, increment, int(mem_ord),
                                        (int)(mem_scope));
     else
@@ -119,7 +119,7 @@ template <typename T> struct Atomic {
 
   T fetch_or(T mask, MemoryOrder mem_ord = MemoryOrder::SEQ_CST,
              [[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
-    if constexpr (LIBC_HAS_BUILTIN(__scoped_atomic_fetch_or))
+    if constexpr (__has_builtin(__scoped_atomic_fetch_or))
       return __scoped_atomic_fetch_or(&val, mask, int(mem_ord),
                                       (int)(mem_scope));
     else
@@ -128,7 +128,7 @@ template <typename T> struct Atomic {
 
   T fetch_and(T mask, MemoryOrder mem_ord = MemoryOrder::SEQ_CST,
               [[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
-    if constexpr (LIBC_HAS_BUILTIN(__scoped_atomic_fetch_and))
+    if constexpr (__has_builtin(__scoped_atomic_fetch_and))
       return __scoped_atomic_fetch_and(&val, mask, int(mem_ord),
                                        (int)(mem_scope));
     else
@@ -137,7 +137,7 @@ template <typename T> struct Atomic {
 
   T fetch_sub(T decrement, MemoryOrder mem_ord = MemoryOrder::SEQ_CST,
               [[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
-    if constexpr (LIBC_HAS_BUILTIN(__scoped_atomic_fetch_sub))
+    if constexpr (__has_builtin(__scoped_atomic_fetch_sub))
       return __scoped_atomic_fetch_sub(&val, decrement, int(mem_ord),
                                        (int)(mem_scope));
     else
@@ -166,7 +166,7 @@ LIBC_INLINE void atomic_thread_fence([[maybe_unused]] MemoryOrder mem_ord) {
 // except no instructions for memory ordering are issued. Only reordering of
 // the instructions by the compiler is suppressed as order instructs.
 LIBC_INLINE void atomic_signal_fence([[maybe_unused]] MemoryOrder mem_ord) {
-#if LIBC_HAS_BUILTIN(__atomic_signal_fence)
+#if __has_builtin(__atomic_signal_fence)
   __atomic_signal_fence(static_cast<int>(mem_ord));
 #else
   // if the builtin is not ready, use asm as a full compiler barrier.
diff --git a/libc/src/__support/CPP/bit.h b/libc/src/__support/CPP/bit.h
index 3f2fbec944054c..17d2e5b26e3532 100644
--- a/libc/src/__support/CPP/bit.h
+++ b/libc/src/__support/CPP/bit.h
@@ -14,14 +14,13 @@
 #include "src/__support/CPP/limits.h" // numeric_limits
 #include "src/__support/CPP/type_traits.h"
 #include "src/__support/macros/attributes.h"
-#include "src/__support/macros/config.h" // LIBC_HAS_BUILTIN
 #include "src/__support/macros/sanitizer.h"
 
 #include <stdint.h>
 
 namespace LIBC_NAMESPACE::cpp {
 
-#if LIBC_HAS_BUILTIN(__builtin_memcpy_inline)
+#if __has_builtin(__builtin_memcpy_inline)
 #define LLVM_LIBC_HAS_BUILTIN_MEMCPY_INLINE
 #endif
 
@@ -36,20 +35,20 @@ LIBC_INLINE constexpr cpp::enable_if_t<
     To>
 bit_cast(const From &from) {
   MSAN_UNPOISON(&from, sizeof(From));
-#if LIBC_HAS_BUILTIN(__builtin_bit_cast)
+#if __has_builtin(__builtin_bit_cast)
   return __builtin_bit_cast(To, from);
 #else
   To to;
   char *dst = reinterpret_cast<char *>(&to);
   const char *src = reinterpret_cast<const char *>(&from);
-#if LIBC_HAS_BUILTIN(__builtin_memcpy_inline)
+#if __has_builtin(__builtin_memcpy_inline)
   __builtin_memcpy_inline(dst, src, sizeof(To));
 #else
   for (unsigned i = 0; i < sizeof(To); ++i)
     dst[i] = src[i];
-#endif // LIBC_HAS_BUILTIN(__builtin_memcpy_inline)
+#endif // __has_builtin(__builtin_memcpy_inline)
   return to;
-#endif // LIBC_HAS_BUILTIN(__builtin_bit_cast)
+#endif // __has_builtin(__builtin_bit_cast)
 }
 
 template <typename T>
@@ -94,7 +93,7 @@ countr_zero(T value) {
   }
   return zero_bits;
 }
-#if LIBC_HAS_BUILTIN(__builtin_ctzs)
+#if __has_builtin(__builtin_ctzs)
 ADD_SPECIALIZATION(countr_zero, unsigned short, __builtin_ctzs)
 #endif
 ADD_SPECIALIZATION(countr_zero, unsigned int, __builtin_ctz)
@@ -124,7 +123,7 @@ countl_zero(T value) {
   }
   return zero_bits;
 }
-#if LIBC_HAS_BUILTIN(__builtin_clzs)
+#if __has_builtin(__builtin_clzs)
 ADD_SPECIALIZATION(countl_zero, unsigned short, __builtin_clzs)
 #endif
 ADD_SPECIALIZATION(countl_zero, unsigned int, __builtin_clz)
diff --git a/libc/src/__support/CPP/type_traits/is_destructible.h b/libc/src/__support/CPP/type_traits/is_destructible.h
index d47de1cc797b29..9f321c5e1d9f67 100644
--- a/libc/src/__support/CPP/type_traits/is_destructible.h
+++ b/libc/src/__support/CPP/type_traits/is_destructible.h
@@ -21,7 +21,7 @@
 namespace LIBC_NAMESPACE::cpp {
 
 // is_destructible
-#if LIBC_HAS_BUILTIN(__is_destructible)
+#if __has_builtin(__is_destructible)
 template <typename T>
 struct is_destructible : bool_constant<__is_destructible(T)> {};
 #else
diff --git a/libc/src/__support/CPP/type_traits/is_function.h b/libc/src/__support/CPP/type_traits/is_function.h
index 557b3224484bca..7fb8a36013a95d 100644
--- a/libc/src/__support/CPP/type_traits/is_function.h
+++ b/libc/src/__support/CPP/type_traits/is_function.h
@@ -17,7 +17,7 @@
 namespace LIBC_NAMESPACE::cpp {
 
 // is_function
-#if LIBC_HAS_BUILTIN(__is_function)
+#if __has_builtin(__is_function)
 template <typename T>
 struct is_function : integral_constant<bool, __is_function(T)> {};
 #else
diff --git a/libc/src/__support/CPP/type_traits/is_lvalue_reference.h b/libc/src/__support/CPP/type_traits/is_lvalue_reference.h
index f52e303afad2a5..b310aeee8efc4f 100644
--- a/libc/src/__support/CPP/type_traits/is_lvalue_reference.h
+++ b/libc/src/__support/CPP/type_traits/is_lvalue_reference.h
@@ -17,7 +17,7 @@
 namespace LIBC_NAMESPACE::cpp {
 
 // is_lvalue_reference
-#if LIBC_HAS_BUILTIN(__is_lvalue_reference)
+#if __has_builtin(__is_lvalue_reference)
 template <typename T>
 struct is_lvalue_reference : bool_constant<__is_lvalue_reference(T)> {};
 #else
diff --git a/libc/src/__support/CPP/type_traits/is_reference.h b/libc/src/__support/CPP/type_traits/is_reference.h
index c017028edf411f..5b2940e7767985 100644
--- a/libc/src/__support/CPP/type_traits/is_reference.h
+++ b/libc/src/__support/CPP/type_traits/is_reference.h
@@ -17,7 +17,7 @@
 namespace LIBC_NAMESPACE::cpp {
 
 // is_reference
-#if LIBC_HAS_BUILTIN(__is_reference)
+#if __has_builtin(__is_reference)
 template <typename T> struct is_reference : bool_constant<__is_reference(T)> {};
 #else
 template <typename T> struct is_reference : public false_type {};
diff --git a/libc/src/__support/CPP/type_traits/is_rvalue_reference.h b/libc/src/__support/CPP/type_traits/is_rvalue_reference.h
index f0487e41c998fe..cdaaa3123e6770 100644
--- a/libc/src/__support/CPP/type_traits/is_rvalue_reference.h
+++ b/libc/src/__support/CPP/type_traits/is_rvalue_reference.h
@@ -17,7 +17,7 @@
 namespace LIBC_NAMESPACE::cpp {
 
 // is_rvalue_reference
-#if LIBC_HAS_BUILTIN(__is_rvalue_reference)
+#if __has_builtin(__is_rvalue_reference)
 template <typename T>
 struct is_rvalue_reference : bool_constant<__is_rvalue_reference(T)> {};
 #else
diff --git a/libc/src/__support/CPP/type_traits/is_trivially_destructible.h b/libc/src/__support/CPP/type_traits/is_trivially_destructible.h
index 3345149433afc4..05c278123f4a35 100644
--- a/libc/src/__support/CPP/type_traits/is_trivially_destructible.h
+++ b/libc/src/__support/CPP/type_traits/is_trivially_destructible.h
@@ -16,7 +16,7 @@
 namespace LIBC_NAMESPACE::cpp {
 
 // is_trivially_destructible
-#if LIBC_HAS_BUILTIN(__is_trivially_destructible)
+#if __has_builtin(__is_trivially_destructible)
 template <typename T>
 struct is_trivially_destructible
     : public bool_constant<__is_trivially_destructible(T)> {};
@@ -25,7 +25,7 @@ template <typename T>
 struct is_trivially_destructible
     : public bool_constant<cpp::is_destructible_v<T> &&__has_trivial_destructor(
           T)> {};
-#endif // LIBC_HAS_BUILTIN(__is_trivially_destructible)
+#endif // __has_builtin(__is_trivially_destructible)
 template <typename T>
 LIBC_INLINE_VAR constexpr bool is_trivially_destructible_v =
     is_trivially_destructible<T>::value;
diff --git a/libc/src/__support/CPP/type_traits/remove_all_extents.h b/libc/src/__support/CPP/type_traits/remove_all_extents.h
index bff6341d3e4560..1813d67fe52911 100644
--- a/libc/src/__support/CPP/type_traits/remove_all_extents.h
+++ b/libc/src/__support/CPP/type_traits/remove_all_extents.h
@@ -16,7 +16,7 @@
 namespace LIBC_NAMESPACE::cpp {
 
 // remove_all_extents
-#if LIBC_HAS_BUILTIN(__remove_all_extents)
+#if __has_builtin(__remove_all_extents)
 template <typename T> using remove_all_extents_t = __remove_all_extents(T);
 template <typename T>
 struct remove_all_extents : cpp::type_identity<remove_all_extents_t<T>> {};
diff --git a/libc/src/__support/FPUtil/FEnvImpl.h b/libc/src/__support/FPUtil/FEnvImpl.h
index a6a533dcfdf4aa..6086d5d3de2dca 100644
--- a/libc/src/__support/FPUtil/FEnvImpl.h
+++ b/libc/src/__support/FPUtil/FEnvImpl.h
@@ -11,7 +11,6 @@
 
 #include "include/llvm-libc-macros/math-macros.h"
 #include "src/__support/macros/attributes.h" // LIBC_INLINE
-#include "src/__support/macros/config.h"     // LIBC_HAS_BUILTIN
 #include "src/__support/macros/properties/architectures.h"
 #include "src/errno/libc_errno.h"
 #include <fenv.h>
diff --git a/libc/src/__support/FPUtil/gpu/FMA.h b/libc/src/__support/FPUtil/gpu/FMA.h
index 86bc8603149611..d364216e4ae41e 100644
--- a/libc/src/__support/FPUtil/gpu/FMA.h
+++ b/libc/src/__support/FPUtil/gpu/FMA.h
@@ -12,10 +12,10 @@
 #include "src/__support/CPP/type_traits.h"
 #include "src/__support/macros/config.h"
 
-// These intrinsics map to the FMA instrunctions in the target ISA for the GPU.
+// These intrinsics map to the FMA instructions in the target ISA for the GPU.
 // The default rounding mode generated from these will be to the nearest even.
-static_assert(LIBC_HAS_BUILTIN(__builtin_fma), "FMA builtins must be defined");
-static_assert(LIBC_HAS_BUILTIN(__builtin_fmaf), "FMA builtins must be defined");
+static_assert(__has_builtin(__builtin_fma), "FMA builtins must be defined");
+static_assert(__has_builtin(__builtin_fmaf), "FMA builtins must be defined");
 
 namespace LIBC_NAMESPACE {
 namespace fputil {
diff --git a/libc/src/__support/macros/config.h b/libc/src/__support/macros/config.h
index fcc8f551a783fe..97599566eaa5e9 100644
--- a/libc/src/__support/macros/config.h
+++ b/libc/src/__support/macros/config.h
@@ -13,24 +13,6 @@
 #ifndef LLVM_LIBC_SRC___SUPPORT_MACROS_CONFIG_H
 #define LLVM_LIBC_SRC___SUPPORT_MACROS_CONFIG_H
 
-// LIBC_HAS_BUILTIN()
-//
-// Checks whether the compiler supports a Clang Feature Checking Macro, and if
-// so, checks whether it supports the provided builtin function "x" where x
-// is one of the functions noted in
-// https://clang.llvm.org/docs/LanguageExtensions.html
-//
-// Note: Use this macro to avoid an extra level of #ifdef __has_builtin check.
-// http://releases.llvm.org/3.3/tools/clang/docs/LanguageExtensions.html
-
-// Compiler builtin-detection.
-// clang.llvm.org/docs/LanguageExtensions.html#has-builtin
-#ifdef __has_builtin
-#define LIBC_HAS_BUILTIN(x) __has_builtin(x)
-#else
-#define LIBC_HAS_BUILTIN(x) 0
-#endif
-
 // Compiler feature-detection.
 // clang.llvm.org/docs/LanguageExtensions.html#has-feature-and-has-extension
 #ifdef __has_feature
diff --git a/libc/src/__support/macros/optimization.h b/libc/src/__support/macros/optimization.h
index ae97efcaa41706..59886ca44be12a 100644
--- a/libc/src/__support/macros/optimization.h
+++ b/libc/src/__support/macros/optimization.h
@@ -11,7 +11,6 @@
 #define LLVM_LIBC_SRC___SUPPORT_MACROS_OPTIMIZATION_H
 
 #include "src/__support/macros/attributes.h"          // LIBC_INLINE
-#include "src/__support/macros/config.h"              // LIBC_HAS_BUILTIN
 #include "src/__support/macros/properties/compiler.h" // LIBC_COMPILER_IS_CLANG
 
 // We use a template to implement likely/unlikely to make sure that we don't
diff --git a/libc/src/__support/macros/sanitizer.h b/libc/src/__support/macros/sanitizer.h
index fc66c2005c42de..bd9b62b7121a14 100644
--- a/libc/src/__support/macros/sanitizer.h
+++ b/libc/src/__support/macros/sanitizer.h
@@ -47,8 +47,7 @@
 // Functions to unpoison memory
 //-----------------------------------------------------------------------------
 
-#if defined(LIBC_HAVE_MEMORY_SANITIZER) &&                                     \
-    LIBC_HAS_BUILTIN(__builtin_constant_p)
+#if defined(LIBC_HAVE_MEMORY_SANITIZER) && __has_builtin(__builtin_constant_p)
 // Only perform MSAN unpoison in non-constexpr context.
 #include <sanitizer/msan_interface.h>
 #define MSAN_UNPOISON(addr, size)                                              \
diff --git a/libc/src/__support/math_extras.h b/libc/src/__support/math_extras.h
index 28ee1be8b99997..70a8800b285d02 100644
--- a/libc/src/__support/math_extras.h
+++ b/libc/src/__support/math_extras.h
@@ -14,7 +14,6 @@
 #include "src/__support/CPP/limits.h"        // CHAR_BIT, numeric_limits
 #include "src/__support/CPP/type_traits.h"   // is_unsigned_v
 #include "src/__support/macros/attributes.h" // LIBC_INLINE
-#include "src/__support/macros/config.h"     // LIBC_HAS_BUILTIN
 
 namespace LIBC_NAMESPACE {
 
@@ -61,7 +60,7 @@ add_with_carry(T a, T b, T carry_in) {
   return add_with_carry_const<T>(a, b, carry_in);
 }
 
-#if LIBC_HAS_BUILTIN(__builtin_addc)
+#if __has_builtin(__builtin_addc)
 // https://clang.llvm.org/docs/LanguageExtensions.html#multiprecision-arithmetic-builtins
 
 template <>
@@ -129,7 +128,7 @@ add_with_carry<unsigned long long>(unsigned long long a, unsigned long long b,
   }
 }
 
-#endif // LIBC_HAS_BUILTIN(__builtin_addc)
+#endif // __has_builtin(__builtin_addc)
 
 // Subtract with borrow
 template <typename T> struct DiffBorrow {
@@ -157,7 +156,7 @@ sub_with_borrow(T a, T b, T borrow_in) {
   return sub_with_borrow_const<T>(a, b, borrow_in);
 }
 
-#if LIBC_HAS_BUILTIN(__builtin_subc)
+#if __has_builtin(__builtin_subc)
 // https://clang.llvm.org/docs/LanguageExtensions.html#multiprecision-arithmetic-builtins
 
 template <>
@@ -225,7 +224,7 @@ sub_with_borrow<unsigned long long>(unsigned long long a, unsigned long long b,
   }
 }
 
-#endif // LIBC_HAS_BUILTIN(__builtin_subc)
+#endif // __has_builtin(__builtin_subc)
 
 template <typename T>
 [[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
diff --git a/libc/src/__support/memory_size.h b/libc/src/__support/memory_size.h
index 7bd16a1695be9a..491123bbabf308 100644
--- a/libc/src/__support/memory_size.h
+++ b/libc/src/__support/memory_size.h
@@ -19,7 +19,7 @@
 namespace LIBC_NAMESPACE {
 namespace internal {
 template <class T> LIBC_INLINE bool mul_overflow(T a, T b, T *res) {
-#if LIBC_HAS_BUILTIN(__builtin_mul_overflow)
+#if __has_builtin(__builtin_mul_overflow)
   return __builtin_mul_overflow(a, b, res);
 #else
   T max = cpp::numeric_limits<T>::max();
diff --git a/libc/src/string/memory_utils/generic/builtin.h b/libc/src/string/memory_utils/generic/builtin.h
index 5239329f653b34..a8f46aeee40a9d 100644
--- a/libc/src/string/memory_utils/generic/builtin.h
+++ b/libc/src/string/memory_utils/generic/builtin.h
@@ -10,16 +10,15 @@
 #define LLVM_LIBC_SRC_STRING_MEMORY_UTILS_GENERIC_BUILTIN_H
 
 #include "src/__support/macros/attributes.h" // LIBC_INLINE
-#include "src/__support/macros/config.h"     // LIBC_HAS_BUILTIN
 #include "src/string/memory_utils/utils.h"   // Ptr, CPtr
 
 #include <stddef.h> // size_t
 
 namespace LIBC_NAMESPACE {
 
-static_assert(LIBC_HAS_BUILTIN(__builtin_memcpy), "Builtin not defined");
-static_assert(LIBC_HAS_BUILTIN(__builtin_memset), "Builtin not defined");
-static_assert(LIBC_HAS_BUILTIN(__builtin_memmove), "Builtin not defined");
+static_assert(__has_builtin(__builtin_memcpy), "Builtin not defined");
+static_assert(__has_builtin(__builtin_memset), "Builtin not defined");
+static_assert(__has_builtin(__builtin_memmove), "Builtin not defined");
 
 [[maybe_unused]] LIBC_INLINE void
 inline_memcpy_builtin(Ptr dst, CPtr src, size_t count, size_t offset = 0) {
diff --git a/libc/src/string/memory_utils/utils.h b/libc/src/string/memory_utils/utils.h
index 79526d19c6b3dc..b3e1a26ad99610 100644
--- a/libc/src/string/memory_utils/utils.h
+++ b/libc/src/string/memory_utils/utils.h
@@ -14,7 +14,6 @@
 #include "src/__support/CPP/type_traits.h"
 #include "src/__support/endian.h"
 #include "src/__support/macros/attributes.h" // LIBC_INLINE
-#include "src/__support/macros/config.h"     // LIBC_HAS_BUILTIN
 #include "src/__support/macros/properties/architectures.h"
 
 #include <stddef.h> // size_t
@@ -71,11 +70,11 @@ LIBC_INLINE bool is_disjoint(const void *p1, const void *p2, size_t size) {
   return sdiff >= 0 ? size <= udiff : size <= neg_udiff;
 }
 
-#if LIBC_HAS_BUILTIN(__builtin_memcpy_inline)
+#if __has_builtin(__builtin_memcpy_inline)
 #define LLVM_LIBC_HAS_BUILTIN_MEMCPY_INLINE
 #endif
 
-#if LIBC_HAS_BUILTIN(__builtin_memset_inline)
+#if __has_builtin(__builtin_memset_inline)
 #define LLVM_LIBC_HAS_BUILTIN_MEMSET_INLINE
 #endif
 

Copy link

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link

✅ With the latest revision this PR passed the Python code formatter.

Copy link
Member

@nickdesaulniers nickdesaulniers left a comment

Choose a reason for hiding this comment

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

thanks for the patch!

libc/src/__support/FPUtil/gpu/FMA.h Outdated Show resolved Hide resolved
libc/src/string/memory_utils/generic/builtin.h Outdated Show resolved Hide resolved
libc/src/__support/CPP/atomic.h Outdated Show resolved Hide resolved
@marcauberer marcauberer force-pushed the libc/remove-libc-has-builtin branch 2 times, most recently from 506251a to f4d91a3 Compare March 25, 2024 19:59
@jhuber6
Copy link
Contributor

jhuber6 commented Mar 25, 2024

Okay I vaguely recall one niche use-case that this will break. Currently I build the RPC loader / server utilities in the main LLVM build similar to how we build libc-hdrgen. The minimum GCC compiler right now is 7.4 which doesn't support this https://godbolt.org/z/zaTvbz1os. These utilities include a handful of files, which I believe use LIBC_HAS_BUILTIN (like atomic.h). If someone were to build LLVM with an older GCC this would no longer work and it would error out.

This is technically out of scope of what we list as the minimum supported GCC version, but so far it's just been hacked around. We could potentially stipulate that people need to use newer GCC, but it's a little annoying when this worked around it previously.

@SchrodingerZhu
Copy link
Contributor

SchrodingerZhu commented Mar 25, 2024

But 7.4.0 does not support -std=c++20 anyway (which is added somewhere in our codebase)

add_entrypoint_object(
  atexit
  SRCS
    atexit.cpp
  HDRS
    atexit.h
  CXX_STANDARD
    20 # For constinit of the atexit callback list.
  DEPENDS
    libc.src.__support.fixedvector
    libc.src.__support.blockstore
    libc.src.__support.threads.mutex
    libc.src.__support.CPP.new
)

@jhuber6
Copy link
Contributor

jhuber6 commented Mar 25, 2024

But 7.4.0 does not support -std=c++20 anyway (which is added somewhere in our codebase)

Yeah, what I mean is that there was a handful of files that it included which didn't exercise anything GCC 7.4 didn't support. We have C++20 elsewhere, but not in the code path that printf or rpc.h take.

It simply included a few headers to communicate with some libc stuff to do the portions of the work on the CPU. We had to use LIBC_HAS_BUILTIN in a few cases because it would define that to false if it wasn't present I believe. So, this change will make it to where people can no long build the GPU libc unless they're on GCC10 and that's quite inconvenient for me.

@nickdesaulniers
Copy link
Member

Okay I vaguely recall one niche use-case that this will break. Currently I build the RPC loader / server utilities in the main LLVM build similar to how we build libc-hdrgen. The minimum GCC compiler right now is 7.4 which doesn't support this https://godbolt.org/z/zaTvbz1os. These utilities include a handful of files, which I believe use LIBC_HAS_BUILTIN (like atomic.h). If someone were to build LLVM with an older GCC this would no longer work and it would error out.

Ah, thanks for the note. I'm curious, when you build the RPC loader / server utilities, what version of GCC are you using?

@jhuber6
Copy link
Contributor

jhuber6 commented Mar 25, 2024

Ah, thanks for the note. I'm curious, when you build the RPC loader / server utilities, what version of GCC are you using?

I use GCC 13.2 personally, but the lower bound is what LLVM supports as it's built as a part of LLVM. This is somewhat out of scope since it's "technically" not supported but I got away with it since I happened to only touch a few files that were compatible (until this patch). Somewhat hacky so I apologize for the inconvenience. We're just hitting the intersection of what's really annoying to order with CMake and this was the easiest solution.

I think in the future I really want to move the RPC implementation out of libc so it's not locked down here. That would get rid of one thing and I could make sure that's compatible. The other remaining use of libc utilities would be the file.h wrappers for the GPU and the printf handling.

@nickdesaulniers
Copy link
Member

Okay I vaguely recall one niche use-case that this will break. Currently I build the RPC loader / server utilities in the main LLVM build similar to how we build libc-hdrgen.

This is code in utils/gpu/ ?

@jhuber6
Copy link
Contributor

jhuber6 commented Mar 25, 2024

Okay I vaguely recall one niche use-case that this will break. Currently I build the RPC loader / server utilities in the main LLVM build similar to how we build libc-hdrgen.

This is code in utils/gpu/ ?

https://github.com/llvm/llvm-project/blob/main/libc/utils/gpu/server/rpc_server.cpp#L11 Yeah, from these include directories. There will also be another one once #85331 lands.

@llvmbot llvmbot added the bazel "Peripheral" support tier build system: utils/bazel label Mar 25, 2024
@jhuber6
Copy link
Contributor

jhuber6 commented Mar 25, 2024

We could probably hack around this by putting this in the server if we want to do this patch. Think you could add this to the patch?

#ifndef __has_builtin
#define __has_builtin 0
#endif

@nickdesaulniers
Copy link
Member

There's a latent inclusion of src/__support/macros/config.h in libc/src/__support/macros/optimization.h that looks like it can be removed. We can save that for another PR; I've filed #86579 for now.

It's ok to add commits on top rather than amend+force push; this helps reviewers see better what changed between revisions.

@jhuber6
Copy link
Contributor

jhuber6 commented Mar 25, 2024

Thanks, LG from my side.

@nickdesaulniers
Copy link
Member

We could potentially stipulate that people need to use newer GCC, but it's a little annoying when this worked around it previously.

ACK'ing this point. (I don't want it to seem that I'm ignoring it)

Such a policy would effectively replace our stated compiler support window with LLVM's much larger compiler support window. I would feel more comfortable pursuing this expanding this window with additional build bot coverage for specific compiler versions.

There's certainly a chick-and-egg problem of having a too modern of a compiler support window preventing you from growing your userbase. Though unless we have significant known users stuck with older toolchains, my slight preference for now is to be more aggressive with requiring newer toolchains.

I think the additions to the rpc server mitigate the need to make a decision here presently. Do consider requesting changes to signal N'ACK'ing this patch if you strongly about it @jhuber6 . I really don't feel strongly either way (despite the exposition dump lol).

I'll wait for presubmit results and @gchatelet 's feedback when he's comes back online, but LGTM.

@nickdesaulniers
Copy link
Member

I'll wait for presubmit results and @gchatelet 's feedback when he's comes back online, but LGTM.

There's some bazel breakage from this change. Let me figure out what's going on.

@nickdesaulniers
Copy link
Member

nickdesaulniers commented Mar 25, 2024

I'll wait for presubmit results and @gchatelet 's feedback when he's comes back online, but LGTM.

There's some bazel breakage from this change. Let me figure out what's going on.

Ok, I needed to remove the include of src/__support/macros/config.h from:

  • libc/src/__support/CPP/type_traits/add_pointer.h
  • libc/src/__support/CPP/type_traits/decay.h
  • libc/src/__support/CPP/type_traits/is_trivially_copyable.h

Then the build succeeds. I was hoping to save all that cleanup for #86579, but I think they should just be cleaned up in this PR.

$ cd utils/bazel/llvm-project-overlay
$ bazel query @llvm-project//libc/... | xargs bazel test --config=generic_clang --test_output=errors --test_tag_filters=-nobuildkite --build_tag_filters=-nobuildkite --@llvm-project//libc:mpfr=system

is how I test bazel.

@marcauberer
Copy link
Member Author

I'll wait for presubmit results and @gchatelet 's feedback when he's comes back online, but LGTM.

There's some bazel breakage from this change. Let me figure out what's going on.

Should be fixed now. Why is this not covered by the test runs on GH?

@nickdesaulniers
Copy link
Member

I'm fat+lazy.

JK, but It's complicated. I'm not sure that llvm even endorses/requires bazel to build, but if we land changes to llvm-libc that break the build with bazel, my coworkers yell at me.

Getting bazel runs in presubmit is a good idea though.

Filed: #86601

libc/src/__support/CPP/atomic.h Show resolved Hide resolved
libc/src/__support/CPP/atomic.h Show resolved Hide resolved
Copy link
Member

@nickdesaulniers nickdesaulniers left a comment

Choose a reason for hiding this comment

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

@marcauberer thanks for the patch? Are you able to land this or do you need one of us to?

@marcauberer
Copy link
Member Author

I can land it, thank y'all for reviewing.

@marcauberer marcauberer merged commit 7711853 into llvm:main Mar 27, 2024
5 checks passed
@marcauberer marcauberer deleted the libc/remove-libc-has-builtin branch March 27, 2024 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bazel "Peripheral" support tier build system: utils/bazel libc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libc] remove LIBC_HAS_BUILTIN
6 participants