-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[libc] Implement vector 'split' and 'concat' routines #157537
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
@llvm/pr-subscribers-libc Author: Joseph Huber (jhuber6) ChangesSummary: The concat function is more limited than what would be ideal, but the Full diff: https://github.com/llvm/llvm-project/pull/157537.diff 4 Files Affected:
diff --git a/libc/src/__support/CPP/CMakeLists.txt b/libc/src/__support/CPP/CMakeLists.txt
index a389a6d1702fe..53b0b17774dac 100644
--- a/libc/src/__support/CPP/CMakeLists.txt
+++ b/libc/src/__support/CPP/CMakeLists.txt
@@ -211,8 +211,19 @@ add_object_library(
libc.src.__support.macros.properties.os
)
+add_header_library(
+ tuple
+ HDRS
+ tuple.h
+ DEPENDS
+ utility
+)
+
add_header_library(
simd
HDRS
simd.h
+ DEPENDS
+ utility
+ tuple
)
diff --git a/libc/src/__support/CPP/simd.h b/libc/src/__support/CPP/simd.h
index 449455c5c0390..f4cdcdb820acb 100644
--- a/libc/src/__support/CPP/simd.h
+++ b/libc/src/__support/CPP/simd.h
@@ -16,7 +16,9 @@
#include "hdr/stdint_proxy.h"
#include "src/__support/CPP/algorithm.h"
#include "src/__support/CPP/limits.h"
+#include "src/__support/CPP/tuple.h"
#include "src/__support/CPP/type_traits.h"
+#include "src/__support/CPP/utility/integer_sequence.h"
#include "src/__support/macros/attributes.h"
#include "src/__support/macros/config.h"
@@ -51,6 +53,7 @@ template <typename T> inline constexpr size_t native_vector_size = 1;
template <typename T> LIBC_INLINE constexpr T poison() {
return __builtin_nondeterministic_value(T());
}
+
} // namespace internal
// Type aliases.
@@ -267,6 +270,40 @@ LIBC_INLINE constexpr simd<T, N> select(simd<bool, N> m, simd<T, N> x,
return m ? x : y;
}
+namespace internal {
+template <typename T, size_t N, size_t M, size_t... I>
+LIBC_INLINE constexpr cpp::simd<T, N + M>
+concat(cpp::simd<T, N> x, cpp::simd<T, M> y, index_sequence<I...>) {
+ return __builtin_shufflevector(x, y, I...);
+}
+
+template <typename T, size_t N, size_t Count, size_t Offset, size_t... I>
+LIBC_INLINE constexpr cpp::simd<T, Count> slice(cpp::simd<T, N> x,
+ cpp::index_sequence<I...>) {
+ return __builtin_shufflevector(x, x, (Offset + I)...);
+}
+template <typename T, size_t N, size_t Offset, size_t Head, size_t... Tail>
+LIBC_INLINE constexpr auto split(cpp::simd<T, N> x) {
+ auto first = cpp::make_tuple(
+ slice<T, N, Head, Offset>(x, cpp::make_index_sequence<Head>{}));
+ if constexpr (sizeof...(Tail) > 0)
+ return cpp::tuple_cat(first, split<T, N, Offset + Head, Tail...>(x));
+ else
+ return first;
+}
+
+} // namespace internal
+
+// Shuffling helpers.
+template <typename T, size_t N>
+cpp::simd<T, N + N> concat(cpp::simd<T, N> x, cpp::simd<T, N> y) {
+ return internal::concat(x, y, make_index_sequence<N + N>{});
+}
+template <size_t... Sizes, typename T, size_t N> auto split(cpp::simd<T, N> x) {
+ static_assert((... + Sizes) == N, "split sizes must sum to vector size");
+ return internal::split<T, N, 0, Sizes...>(x);
+}
+
// TODO: where expressions, scalar overloads, ABI types.
} // namespace cpp
diff --git a/libc/src/__support/CPP/tuple.h b/libc/src/__support/CPP/tuple.h
new file mode 100644
index 0000000000000..01d374a0841aa
--- /dev/null
+++ b/libc/src/__support/CPP/tuple.h
@@ -0,0 +1,106 @@
+//===-- tuple utility -------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC___SUPPORT_CPP_UTILITY_TUPLE_H
+#define LLVM_LIBC_SRC___SUPPORT_CPP_UTILITY_TUPLE_H
+
+#include "src/__support/CPP/type_traits/decay.h"
+#include "src/__support/CPP/utility/integer_sequence.h"
+
+namespace LIBC_NAMESPACE_DECL {
+namespace cpp {
+
+template <typename... Ts> struct tuple;
+template <> struct tuple<> {};
+
+template <typename Head, typename... Tail>
+struct tuple<Head, Tail...> : tuple<Tail...> {
+ Head head;
+
+ LIBC_INLINE constexpr tuple() = default;
+
+ LIBC_INLINE constexpr tuple(const Head &h, const Tail &...t)
+ : tuple<Tail...>(t...), head(h) {}
+
+ LIBC_INLINE constexpr Head &get_head() { return head; }
+ LIBC_INLINE constexpr const Head &get_head() const { return head; }
+
+ LIBC_INLINE constexpr tuple<Tail...> &get_tail() { return *this; }
+ LIBC_INLINE constexpr const tuple<Tail...> &get_tail() const { return *this; }
+};
+
+template <typename... Ts> LIBC_INLINE constexpr auto make_tuple(Ts &&...args) {
+ return tuple<cpp::decay_t<Ts>...>(static_cast<Ts &&>(args)...);
+}
+template <typename... Ts> LIBC_INLINE constexpr auto tie(Ts &...args) {
+ return tuple<Ts &...>(args...);
+}
+
+template <size_t I, typename Head, typename... Tail>
+LIBC_INLINE constexpr auto &get(tuple<Head, Tail...> &t) {
+ if constexpr (I == 0)
+ return t.get_head();
+ else
+ return get<I - 1>(t.get_tail());
+}
+template <size_t I, typename Head, typename... Tail>
+LIBC_INLINE constexpr const auto &get(const tuple<Head, Tail...> &t) {
+ if constexpr (I == 0)
+ return t.get_head();
+ else
+ return get<I - 1>(t.get_tail());
+}
+
+template <typename T> struct tuple_size;
+template <typename... Ts> struct tuple_size<tuple<Ts...>> {
+ static constexpr size_t value = sizeof...(Ts);
+};
+
+template <size_t I, typename T> struct tuple_element;
+template <size_t I, typename Head, typename... Tail>
+struct tuple_element<I, tuple<Head, Tail...>>
+ : tuple_element<I - 1, tuple<Tail...>> {};
+template <typename Head, typename... Tail>
+struct tuple_element<0, tuple<Head, Tail...>> {
+ using type = Head;
+};
+
+namespace internal {
+template <typename... As, typename... Bs, size_t... I, size_t... J>
+LIBC_INLINE constexpr auto
+tuple_cat(const tuple<As...> &a, const tuple<Bs...> &b,
+ cpp::index_sequence<I...>, cpp::index_sequence<J...>) {
+ return tuple<As..., Bs...>(get<I>(a)..., get<J>(b)...);
+}
+
+template <typename First, typename Second, typename... Rest>
+LIBC_INLINE constexpr auto tuple_cat(const First &f, const Second &s,
+ const Rest &...rest) {
+ auto concat =
+ tuple_cat(f, s, cpp::make_index_sequence<tuple_size<First>::value>{},
+ cpp::make_index_sequence<tuple_size<Second>::value>{});
+ if constexpr (sizeof...(Rest))
+ return tuple_cat(concat, rest...);
+ else
+ return concat;
+}
+} // namespace internal
+
+template <typename... Tuples>
+LIBC_INLINE constexpr auto tuple_cat(const Tuples &...tuples) {
+ static_assert(sizeof...(Tuples) > 0, "need at least one element");
+ if constexpr (sizeof...(Tuples) == 1)
+ return (tuples, ...);
+ else
+ return internal::tuple_cat(tuples...);
+}
+
+} // namespace cpp
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC___SUPPORT_CPP_UTILITY_TUPLE_H
diff --git a/libc/src/__support/CPP/utility/integer_sequence.h b/libc/src/__support/CPP/utility/integer_sequence.h
index 06643d505aca0..37faa4f1d5aac 100644
--- a/libc/src/__support/CPP/utility/integer_sequence.h
+++ b/libc/src/__support/CPP/utility/integer_sequence.h
@@ -5,12 +5,15 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
+
#ifndef LLVM_LIBC_SRC___SUPPORT_CPP_UTILITY_INTEGER_SEQUENCE_H
#define LLVM_LIBC_SRC___SUPPORT_CPP_UTILITY_INTEGER_SEQUENCE_H
#include "src/__support/CPP/type_traits/is_integral.h"
#include "src/__support/macros/config.h"
+#include <stddef.h>
+
namespace LIBC_NAMESPACE_DECL {
namespace cpp {
@@ -30,10 +33,17 @@ template <typename T> struct make_integer_sequence<T, -1> {
};
} // namespace detail
+// index sequence
template <typename T, int N>
using make_integer_sequence =
typename detail::make_integer_sequence<T, N - 1>::type;
+template <size_t... Ints>
+using index_sequence = integer_sequence<size_t, Ints...>;
+template <int N>
+using make_index_sequence =
+ typename detail::make_integer_sequence<size_t, N - 1>::type;
+
} // namespace cpp
} // namespace LIBC_NAMESPACE_DECL
|
4a4e39e
to
92c05a6
Compare
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.
given that these SIMD utilities are getting more complex, it would be best to add unit tests to ensure that they work. It'd probably be best to do that in a separate patch to avoid having one massive patch.
} // namespace LIBC_NAMESPACE_DECL | ||
|
||
// For structured binding support. | ||
namespace std { |
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.
could you add a bit more detail on why this needs to be in namespace std?
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.
C++ standard says so
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.
will this clash with libc++ if being included?
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.
It's a specialization on the cpp::tuple_element
variant I believe, it should overload. It's not always present since we're freestanding hence the forward declaration.
Sure, I can make a base test and then merge them into this one if they can get reviewed real quick. |
Summary: This provides some helpers for the split and concatenation routines for changing the size of an existing vector. This includes a simple tuple type to do the splitting. The tuple doesn't support structured bindings yet. The concat function is more limited than what would be ideal, but the shufflevector builtin requires things of equivalent sizes and I didn't think it was worth wrangling with that just yet.
libc/src/__support/CPP/simd.h
Outdated
} | ||
|
||
namespace internal { | ||
template <typename T, size_t N, size_t O, size_t... I> |
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.
There are too many template arguments with one-character names. Could you give at least some of these more descriptive names here and below?
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.
Sure, I'll add some basic comments as well since I feel like this is impossible to follow if you don't know what it's doing.
libc/src/__support/CPP/simd.h
Outdated
// Recursively resize an input vector to the target size, increasing its size | ||
// by at most double the input size each step. |
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.
Why are the intermediate steps necessary? Does the compiler not let you expand a vector by more than 2x per step?
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.
The interface __builtin_shufflevector(x, x, {...})
cannot possibly express more indices than 2x.
x, x, (Indices < OriginalSize ? static_cast<int>(Indices) : -1)...); | ||
} | ||
|
||
template <typename T, size_t N, size_t TargetSize, size_t OriginalSize> |
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.
the value of OriginalSize
is never used
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.
It is just below,
return extend<T, 2 * N, TargetSize, OriginalSize>(
extend<T, N, 2 * N>(x, cpp::make_index_sequence<2 * N>{}));
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.
It's used to recurse there, but I don't see how the value is used since that's just another call to the same function.
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.
The function right above this is called, I guess I could call it 'do_extend' if you think that'd make it clearer, but it's just basic recurse until you get to the version of the function that doesn't expand.
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.
Nevermind, renaming the function breaks everything because I remembered I wrote it this way on purpose. It'll just need to be confusing.
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.
All good, in that case I'd say add a comment explaining that it's confusing but that's how it has to be and call it good.
return __builtin_shufflevector(x, x, (Offset + Indices)...); | ||
} | ||
|
||
template <typename T, size_t N, size_t Offset, size_t Head, size_t... Tail> |
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.
Could you add a comment explaining that first
is a tuple of one simd vector instead of a tuple containing the values in that simd vector? I misunderstood this the first time I read it.
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 after minor fixes
x, x, (Indices < OriginalSize ? static_cast<int>(Indices) : -1)...); | ||
} | ||
|
||
template <typename T, size_t N, size_t TargetSize, size_t OriginalSize> |
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.
All good, in that case I'd say add a comment explaining that it's confusing but that's how it has to be and call it good.
Summary:
This provides some helpers for the split and concatenation routines for
changing the size of an existing vector. This includes a simple tuple
type to do the splitting. The tuple doesn't support structured bindings
yet.
The concat function is more limited than what would be ideal, but the
shufflevector builtin requires things of equivalent sizes and I
didn't think it was worth wrangling with that just yet.