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][__support][bit] remove compiler has builtin checks #81679

Merged
merged 1 commit into from
Feb 14, 2024

Conversation

nickdesaulniers
Copy link
Member

We only support building llvmlibc with modern compilers.
https://libc.llvm.org/compiler_support.html#minimum-supported-versions

All versions of the these compilers support these builtins; GCC does not
support the short variants.

We only support building llvmlibc with modern compilers.
https://libc.llvm.org/compiler_support.html#minimum-supported-versions

All versions of the these compilers support these builtins; GCC does not
support the short variants.
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 13, 2024

@llvm/pr-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

Changes

We only support building llvmlibc with modern compilers.
https://libc.llvm.org/compiler_support.html#minimum-supported-versions

All versions of the these compilers support these builtins; GCC does not
support the short variants.


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

1 Files Affected:

  • (modified) libc/src/__support/CPP/bit.h (-18)
diff --git a/libc/src/__support/CPP/bit.h b/libc/src/__support/CPP/bit.h
index 392fbe248138ae..a8bf75a9a2efac 100644
--- a/libc/src/__support/CPP/bit.h
+++ b/libc/src/__support/CPP/bit.h
@@ -93,15 +93,9 @@ template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
 #if LIBC_HAS_BUILTIN(__builtin_ctzs)
 ADD_SPECIALIZATION(countr_zero, unsigned short, __builtin_ctzs)
 #endif
-#if LIBC_HAS_BUILTIN(__builtin_ctz)
 ADD_SPECIALIZATION(countr_zero, unsigned int, __builtin_ctz)
-#endif
-#if LIBC_HAS_BUILTIN(__builtin_ctzl)
 ADD_SPECIALIZATION(countr_zero, unsigned long, __builtin_ctzl)
-#endif
-#if LIBC_HAS_BUILTIN(__builtin_ctzll)
 ADD_SPECIALIZATION(countr_zero, unsigned long long, __builtin_ctzll)
-#endif
 
 /// Count number of 0's from the most significant bit to the least
 ///   stopping at the first 1.
@@ -128,15 +122,9 @@ template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
 #if LIBC_HAS_BUILTIN(__builtin_clzs)
 ADD_SPECIALIZATION(countl_zero, unsigned short, __builtin_clzs)
 #endif
-#if LIBC_HAS_BUILTIN(__builtin_clz)
 ADD_SPECIALIZATION(countl_zero, unsigned int, __builtin_clz)
-#endif
-#if LIBC_HAS_BUILTIN(__builtin_clzl)
 ADD_SPECIALIZATION(countl_zero, unsigned long, __builtin_clzl)
-#endif
-#if LIBC_HAS_BUILTIN(__builtin_clzll)
 ADD_SPECIALIZATION(countl_zero, unsigned long long, __builtin_clzll)
-#endif
 
 #undef ADD_SPECIALIZATION
 
@@ -256,15 +244,9 @@ template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
 #if LIBC_HAS_BUILTIN(__builtin_clzs)
 SPECIALIZE_FLZ(first_leading_zero, unsigned short, __builtin_clzs)
 #endif
-#if LIBC_HAS_BUILTIN(__builtin_clz)
 SPECIALIZE_FLZ(first_leading_zero, unsigned int, __builtin_clz)
-#endif
-#if LIBC_HAS_BUILTIN(__builtin_clzl)
 SPECIALIZE_FLZ(first_leading_zero, unsigned long, __builtin_clzl)
-#endif
-#if LIBC_HAS_BUILTIN(__builtin_clzll)
 SPECIALIZE_FLZ(first_leading_zero, unsigned long long, __builtin_clzll)
-#endif
 
 #undef SPECIALIZE_FLZ
 

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

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

I'm fine with doing this conceptually, but have you tested it on arm and risc-v? Sometimes the compilers for those are missing builtins unexpectedly

@gchatelet
Copy link
Contributor

It seems to be OK for all the compilers and environments we support.
https://godbolt.org/z/Ez3jaxKqz

@nickdesaulniers nickdesaulniers merged commit 4efbf52 into llvm:main Feb 14, 2024
7 checks passed
@nickdesaulniers nickdesaulniers deleted the bit_builtin branch February 20, 2024 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants