-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[libc] Clean up mask helpers after allowing implicit conversions #158681
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
Conversation
Summary: I landed a change in clang that allows integral vectors to implicitly convert to boolean ones. This means I can simplify the interface and remove the need to cast to bool on every use. Also do some other cleanups of the traits.
@llvm/pr-subscribers-libc Author: Joseph Huber (jhuber6) ChangesSummary: Full diff: https://github.com/llvm/llvm-project/pull/158681.diff 4 Files Affected:
diff --git a/libc/src/__support/CPP/simd.h b/libc/src/__support/CPP/simd.h
index 3c7e65acc3c0a..d2a5b17fa4b9f 100644
--- a/libc/src/__support/CPP/simd.h
+++ b/libc/src/__support/CPP/simd.h
@@ -57,6 +57,29 @@ using simd = T [[clang::ext_vector_type(N)]];
template <typename T>
using simd_mask = simd<bool, internal::native_vector_size<T>>;
+// Type trait helpers.
+template <typename T>
+struct simd_size : cpp::integral_constant<size_t, __builtin_vectorelements(T)> {
+};
+template <class T> constexpr size_t simd_size_v = simd_size<T>::value;
+
+template <typename T> struct is_simd : cpp::integral_constant<bool, false> {};
+template <typename T, unsigned N>
+struct is_simd<simd<T, N>> : cpp::integral_constant<bool, true> {};
+template <class T> constexpr bool is_simd_v = is_simd<T>::value;
+
+template <typename T>
+struct is_simd_mask : cpp::integral_constant<bool, false> {};
+template <unsigned N>
+struct is_simd_mask<simd<bool, N>> : cpp::integral_constant<bool, true> {};
+template <class T> constexpr bool is_simd_mask_v = is_simd_mask<T>::value;
+
+template <typename T> struct simd_element_type;
+template <typename T, size_t N> struct simd_element_type<simd<T, N>> {
+ using type = T;
+};
+template <typename T>
+using simd_element_type_t = typename simd_element_type<T>::type;
namespace internal {
template <typename T>
@@ -123,34 +146,14 @@ LIBC_INLINE constexpr static auto split(cpp::simd<T, N> x) {
return result;
}
-} // namespace internal
-
-// Type trait helpers.
+// Helper trait
template <typename T>
-struct simd_size : cpp::integral_constant<size_t, __builtin_vectorelements(T)> {
-};
-template <class T> constexpr size_t simd_size_v = simd_size<T>::value;
-
-template <typename T> struct is_simd : cpp::integral_constant<bool, false> {};
-template <typename T, unsigned N>
-struct is_simd<simd<T, N>> : cpp::integral_constant<bool, true> {};
-template <class T> constexpr bool is_simd_v = is_simd<T>::value;
-
-template <typename T>
-struct is_simd_mask : cpp::integral_constant<bool, false> {};
-template <unsigned N>
-struct is_simd_mask<simd<bool, N>> : cpp::integral_constant<bool, true> {};
-template <class T> constexpr bool is_simd_mask_v = is_simd_mask<T>::value;
+using enable_if_integral_t = cpp::enable_if_t<cpp::is_integral_v<T>, T>;
-template <typename T> struct simd_element_type;
-template <typename T, size_t N> struct simd_element_type<simd<T, N>> {
- using type = T;
-};
template <typename T>
-using simd_element_type_t = typename simd_element_type<T>::type;
+using enable_if_simd_t = cpp::enable_if_t<is_simd_v<T>, bool>;
-template <typename T>
-using enable_if_simd_t = cpp::enable_if_t<is_simd_v<T>, T>;
+} // namespace internal
// Casting.
template <typename To, typename From, size_t N>
@@ -159,29 +162,34 @@ LIBC_INLINE constexpr static simd<To, N> simd_cast(simd<From, N> v) {
}
// SIMD mask operations.
-template <size_t N> LIBC_INLINE constexpr static bool all_of(simd<bool, N> m) {
- return __builtin_reduce_and(m);
+template <typename T, size_t N, internal::enable_if_integral_t<T> = 0>
+LIBC_INLINE constexpr static bool all_of(simd<T, N> v) {
+ return __builtin_reduce_and(simd_cast<bool>(v));
}
-template <size_t N> LIBC_INLINE constexpr static bool any_of(simd<bool, N> m) {
- return __builtin_reduce_or(m);
+template <typename T, size_t N, internal::enable_if_integral_t<T> = 0>
+LIBC_INLINE constexpr static bool any_of(simd<T, N> v) {
+ return __builtin_reduce_or(simd_cast<bool>(v));
}
-template <size_t N> LIBC_INLINE constexpr static bool none_of(simd<bool, N> m) {
- return !any_of(m);
+template <typename T, size_t N, internal::enable_if_integral_t<T> = 0>
+LIBC_INLINE constexpr static bool none_of(simd<T, N> v) {
+ return !any_of(v);
}
-template <size_t N> LIBC_INLINE constexpr static bool some_of(simd<bool, N> m) {
- return any_of(m) && !all_of(m);
+template <typename T, size_t N, internal::enable_if_integral_t<T> = 0>
+LIBC_INLINE constexpr static bool some_of(simd<T, N> v) {
+ return any_of(v) && !all_of(v);
}
-template <size_t N> LIBC_INLINE constexpr static int popcount(simd<bool, N> m) {
- return __builtin_popcountg(m);
+template <typename T, size_t N, internal::enable_if_integral_t<T> = 0>
+LIBC_INLINE constexpr static int popcount(simd<T, N> v) {
+ return __builtin_popcountg(v);
}
-template <size_t N>
-LIBC_INLINE constexpr static int find_first_set(simd<bool, N> m) {
- return __builtin_ctzg(m);
+template <typename T, size_t N, internal::enable_if_integral_t<T> = 0>
+LIBC_INLINE constexpr static int find_first_set(simd<T, N> v) {
+ return __builtin_ctzg(simd_cast<bool>(v));
}
-template <size_t N>
-LIBC_INLINE constexpr static int find_last_set(simd<bool, N> m) {
- constexpr size_t size = simd_size_v<simd<bool, N>>;
- return size - 1 - __builtin_clzg(m);
+template <typename T, size_t N, internal::enable_if_integral_t<T> = 0>
+LIBC_INLINE constexpr static int find_last_set(simd<T, N> v) {
+ constexpr size_t size = simd_size_v<simd<T, N>>;
+ return size - 1 - __builtin_clzg(simd_cast<bool>(v));
}
// Elementwise operations.
@@ -279,33 +287,32 @@ LIBC_INLINE constexpr static T hmax(simd<T, N> v) {
}
// Accessor helpers.
-template <typename T>
-LIBC_INLINE enable_if_simd_t<T> load_unaligned(const void *ptr) {
+template <typename T, internal::enable_if_simd_t<T> = 0>
+LIBC_INLINE T load_unaligned(const void *ptr) {
T tmp;
__builtin_memcpy(&tmp, ptr, sizeof(T));
return tmp;
}
-template <typename T>
-LIBC_INLINE enable_if_simd_t<T> load_aligned(const void *ptr) {
+template <typename T, internal::enable_if_simd_t<T> = 0>
+LIBC_INLINE T load_aligned(const void *ptr) {
return load_unaligned<T>(__builtin_assume_aligned(ptr, alignof(T)));
}
-template <typename T>
-LIBC_INLINE enable_if_simd_t<T> store_unaligned(T v, void *ptr) {
+template <typename T, internal::enable_if_simd_t<T> = 0>
+LIBC_INLINE T store_unaligned(T v, void *ptr) {
__builtin_memcpy(ptr, &v, sizeof(T));
}
-template <typename T>
-LIBC_INLINE enable_if_simd_t<T> store_aligned(T v, void *ptr) {
+template <typename T, internal::enable_if_simd_t<T> = 0>
+LIBC_INLINE T store_aligned(T v, void *ptr) {
store_unaligned<T>(v, __builtin_assume_aligned(ptr, alignof(T)));
}
-template <typename T>
-LIBC_INLINE enable_if_simd_t<T>
+template <typename T, internal::enable_if_simd_t<T> = 0>
+LIBC_INLINE T
masked_load(simd<bool, simd_size_v<T>> m, void *ptr,
T passthru = internal::poison<simd_element_type<T>>()) {
return __builtin_masked_load(m, ptr, passthru);
}
-template <typename T>
-LIBC_INLINE enable_if_simd_t<T> masked_store(simd<bool, simd_size_v<T>> m, T v,
- void *ptr) {
+template <typename T, internal::enable_if_simd_t<T> = 0>
+LIBC_INLINE T masked_store(simd<bool, simd_size_v<T>> m, T v, void *ptr) {
__builtin_masked_store(
m, v, static_cast<T *>(__builtin_assume_aligned(ptr, alignof(T))));
}
diff --git a/libc/src/__support/CPP/type_traits/is_unsigned.h b/libc/src/__support/CPP/type_traits/is_unsigned.h
index 3ae6337ceb50a..b4267eedd19fc 100644
--- a/libc/src/__support/CPP/type_traits/is_unsigned.h
+++ b/libc/src/__support/CPP/type_traits/is_unsigned.h
@@ -16,6 +16,8 @@
#include "src/__support/macros/attributes.h"
#include "src/__support/macros/config.h"
+#include <stddef.h>
+
namespace LIBC_NAMESPACE_DECL {
namespace cpp {
@@ -46,6 +48,10 @@ template <typename T> struct is_unsigned {
LIBC_INLINE constexpr bool operator()() const { return is_unsigned::value; }
};
#endif // LIBC_COMPILER_HAS_FIXED_POINT
+#if LIBC_HAS_VECTOR_TYPE
+template <typename T, size_t N>
+struct is_unsigned<T [[clang::ext_vector_type(N)]]> : bool_constant<false> {};
+#endif
template <typename T>
LIBC_INLINE_VAR constexpr bool is_unsigned_v = is_unsigned<T>::value;
diff --git a/libc/src/string/memory_utils/generic/inline_strlen.h b/libc/src/string/memory_utils/generic/inline_strlen.h
index 68fba2afb3a5c..5e553e301d4da 100644
--- a/libc/src/string/memory_utils/generic/inline_strlen.h
+++ b/libc/src/string/memory_utils/generic/inline_strlen.h
@@ -33,14 +33,14 @@ string_length(const char *src) {
__builtin_align_down(src, alignment));
cpp::simd<char> chars = cpp::load_aligned<cpp::simd<char>>(aligned);
- cpp::simd_mask<char> mask = cpp::simd_cast<bool>(chars == null_byte);
+ cpp::simd_mask<char> mask = chars == null_byte;
size_t offset = src - reinterpret_cast<const char *>(aligned);
if (cpp::any_of(shift_mask(mask, offset)))
return cpp::find_first_set(shift_mask(mask, offset));
for (;;) {
cpp::simd<char> chars = cpp::load_aligned<cpp::simd<char>>(++aligned);
- cpp::simd_mask<char> mask = cpp::simd_cast<bool>(chars == null_byte);
+ cpp::simd_mask<char> mask = chars == null_byte;
if (cpp::any_of(mask))
return (reinterpret_cast<const char *>(aligned) - src) +
cpp::find_first_set(mask);
diff --git a/libc/test/src/__support/CPP/simd_test.cpp b/libc/test/src/__support/CPP/simd_test.cpp
index b4f5685e3b1d1..c8f34df8ab028 100644
--- a/libc/test/src/__support/CPP/simd_test.cpp
+++ b/libc/test/src/__support/CPP/simd_test.cpp
@@ -64,23 +64,25 @@ TEST(LlvmLibcSIMDTest, MaskOperations) {
EXPECT_TRUE(cpp::any_of(mask));
EXPECT_FALSE(cpp::all_of(mask));
+ EXPECT_FALSE(cpp::none_of(mask));
EXPECT_TRUE(cpp::some_of(mask));
EXPECT_EQ(cpp::find_first_set(mask), 0);
EXPECT_EQ(cpp::find_last_set(mask), 2);
+ EXPECT_EQ(cpp::popcount(mask), 2);
}
TEST(LlvmLibcSIMDTest, SplitConcat) {
cpp::simd<char, 8> v{1, 1, 2, 2, 3, 3, 4, 4};
auto [v1, v2, v3, v4] = cpp::split<2, 2, 2, 2>(v);
- EXPECT_TRUE(cpp::all_of(cpp::simd_cast<bool>(v1 == 1)));
- EXPECT_TRUE(cpp::all_of(cpp::simd_cast<bool>(v2 == 2)));
- EXPECT_TRUE(cpp::all_of(cpp::simd_cast<bool>(v3 == 3)));
- EXPECT_TRUE(cpp::all_of(cpp::simd_cast<bool>(v4 == 4)));
+ EXPECT_TRUE(cpp::all_of(v1 == 1));
+ EXPECT_TRUE(cpp::all_of(v2 == 2));
+ EXPECT_TRUE(cpp::all_of(v3 == 3));
+ EXPECT_TRUE(cpp::all_of(v4 == 4));
cpp::simd<char, 8> m = cpp::concat(v1, v2, v3, v4);
- EXPECT_TRUE(cpp::all_of(cpp::simd_cast<bool>(m == v)));
+ EXPECT_TRUE(cpp::all_of(m == v));
cpp::simd<char, 1> c(~0);
cpp::simd<char, 8> n = cpp::concat(c, c, c, c, c, c, c, c);
- EXPECT_TRUE(cpp::all_of(cpp::simd_cast<bool>(n == ~0)));
+ EXPECT_TRUE(cpp::all_of(n == ~0));
}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The bots seem to have not picked up 134a58a yet, so I'll sit on this for a bit. |
What are the intentions of |
On the functions? It makes them internal so they don't accidentally leak incompatible ABI details. |
…m#158681) Summary: I landed a change in clang that allows integral vectors to implicitly convert to boolean ones. This means I can simplify the interface and remove the need to cast to bool on every use. Also do some other cleanups of the traits.
…m#158681) Summary: I landed a change in clang that allows integral vectors to implicitly convert to boolean ones. This means I can simplify the interface and remove the need to cast to bool on every use. Also do some other cleanups of the traits.
Summary:
I landed a change in clang that allows integral vectors to implicitly
convert to boolean ones. This means I can simplify the interface and
remove the need to cast to bool on every use. Also do some other
cleanups of the traits.