-
Notifications
You must be signed in to change notification settings - Fork 509
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
Add vectorization in elementwise_util #9432
base: gh/swolchok/385/head
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,7 +47,7 @@ Tensor& where_out( | |
CTYPE_COMPUTE, | ||
op_name, | ||
utils::SupportedTensorDtypes::SAME_AS_COMMON>( | ||
[](const auto val_a, const auto val_b, const auto val_c) { | ||
[](const CTYPE_COMPUTE val_a, const CTYPE_COMPUTE val_b, const CTYPE_COMPUTE val_c) { | ||
return val_c ? val_a : val_b; | ||
}, | ||
Comment on lines
+50
to
52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (ATen, and therefore our own optimized op_where, doesn't vectorize this) |
||
ctx, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,9 +12,14 @@ | |
#include <executorch/kernels/portable/cpu/util/broadcast_indexes_range.h> | ||
#include <executorch/kernels/portable/cpu/util/broadcast_util.h> | ||
#include <executorch/kernels/portable/cpu/util/dtype_util.h> | ||
#include <executorch/kernels/portable/cpu/util/vectorized_math.h> // Make vectorization support easy for clients. | ||
#include <executorch/runtime/kernel/kernel_runtime_context.h> | ||
#include <executorch/runtime/kernel/thread_parallel_interface.h> | ||
|
||
#ifdef ET_USE_PYTORCH_HEADERS | ||
#include <ATen/cpu/vec/vec.h> | ||
#endif // ET_USE_PYTORCH_HEADERS | ||
|
||
#include <array> | ||
#include <utility> | ||
|
||
|
@@ -51,6 +56,34 @@ inline int64_t scalar_to<int64_t>(const Scalar& s) { | |
} | ||
|
||
namespace internal { | ||
template <typename Ignore, typename T> | ||
using ignore_first_yield_second = T; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like these names |
||
|
||
#ifdef ET_USE_PYTORCH_HEADERS | ||
// Can I call a function of type Op with sizeof...(Args) arguments of type | ||
// at::vec::Vectorized<CTYPE_COMPUTE>? | ||
// | ||
// See [NOTE: Generic lambdas] below for requirements on Op. | ||
template <typename CTYPE_COMPUTE, typename Op, typename... Args> | ||
constexpr bool can_use_vectorized() { | ||
using Vec = at::vec::Vectorized<CTYPE_COMPUTE>; | ||
if constexpr (std::is_invocable_v< | ||
Op, | ||
ignore_first_yield_second<Args, Vec>...>) { | ||
// For bool, we will get a false positive if we rely on only the | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you mean bool return type? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bool ctype_compute |
||
// is_invocable_v check above because at::vec::Vectorized is | ||
// implicitly convertible to a pointer, which makes it implicitly | ||
// convertible to bool (which was 15 minutes of fun to debug). Also | ||
// just seems like good hygiene to make sure we get the Vectorized | ||
// we're expecting. | ||
return std::is_same_v< | ||
std::invoke_result_t<Op, ignore_first_yield_second<Args, Vec>...>, | ||
Vec>; | ||
} | ||
return false; | ||
} | ||
#endif // ET_USE_PYTORCH_HEADERS | ||
|
||
template < | ||
typename CTYPE_COMPUTE, | ||
typename CTYPE_OUT, | ||
|
@@ -61,8 +94,71 @@ inline void dtype_specialized_elementwise_fn_impl( | |
KernelRuntimeContext& ctx, | ||
const Tensor& out, | ||
Args... inputs) { | ||
static_assert( | ||
(std::is_same_v<Args, std::pair<const Tensor*, SupportedTensorDtypes>> && | ||
...)); | ||
constexpr auto kNumInputs = sizeof...(inputs); | ||
ET_DCHECK(((inputs.first->element_size() == sizeof(CTYPE_COMPUTE)) && ...)); | ||
// All inputs must be of type CTYPE_COMPUTE. | ||
ET_DCHECK( | ||
((inputs.first->scalar_type() == | ||
CppTypeToScalarType<CTYPE_COMPUTE>::value) && | ||
...)); | ||
Comment on lines
+103
to
+105
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Man, this is a good reminder of all the template meta programming magic |
||
|
||
#ifdef ET_USE_PYTORCH_HEADERS | ||
if constexpr (can_use_vectorized<CTYPE_COMPUTE, Op, Args...>()) { | ||
const bool any_is_broadcasted = | ||
!(torch::executor::internal::sizes_match_ignoring_leading_1s( | ||
inputs.first->sizes(), out.sizes()) && | ||
...); | ||
if (!any_is_broadcasted) { | ||
using Vec = at::vec::Vectorized<CTYPE_COMPUTE>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the one point of contention for me is that why do we need vectorized_math.h which largely is doing trampoline to underlying vectorized methods. Mainly you dont even need to use However, place where I can potentially see that being useful is for dtype that Vectorized doesnt support but for float I am not sure. So maybe if you can clarify that it would help. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
without it, you can't take the same lambda you already wrote for scalars and reuse it for Vectorized (the change isn't zero because you have to point at executorch::math, but crucially it doesn't require separate code) |
||
::executorch::extension::parallel_for( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think doing this blindly for each op is a bit risky in that, no all multithreading is always better. some ops benefit from smaller grain size while others with larger There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIRC xnnpack blindly parallelizes absolutely everything; we're doing strictly better here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah I am not comparing with xnnpack. In fact one bit part of the reason why we ended up leveraging optimized op lib for some of the llama stuff for exactly that reason. That it blindly parallelized everything and that actually hurt perf |
||
0, | ||
out.numel(), | ||
::executorch::extension::internal::GRAIN_SIZE, | ||
[&](const auto begin, const auto end) { | ||
std::array<const CTYPE_COMPUTE*, kNumInputs> inputs_data_ptrs = { | ||
inputs.first->template const_data_ptr<CTYPE_COMPUTE>()...}; | ||
|
||
CTYPE_OUT* const data_out = out.mutable_data_ptr<CTYPE_OUT>(); | ||
|
||
const auto vectorized_begin = | ||
begin + (Vec::size() - begin % Vec::size()) % Vec::size(); | ||
Comment on lines
+125
to
+126
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feels like something that has chances of bug. Hope we test this enough. I would doubt if our test cases will exercise this code path. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. although I do see you treat scalar left overs of both head and tails separately |
||
const auto vectorized_end = end - (end % Vec::size()); | ||
// Scalar prologue. | ||
for (const auto idx : c10::irange(begin, vectorized_begin)) { | ||
std::array<CTYPE_COMPUTE, kNumInputs> loaded_inputs; | ||
for (const auto input_idx : c10::irange(kNumInputs)) { | ||
loaded_inputs[input_idx] = inputs_data_ptrs[input_idx][idx]; | ||
} | ||
data_out[idx] = std::apply(compute_fun, loaded_inputs); | ||
} | ||
|
||
// Main vectorized loop. | ||
for (auto idx = vectorized_begin; idx < vectorized_end; | ||
idx += Vec::size()) { | ||
std::array<Vec, kNumInputs> loaded_vec_inputs; | ||
for (const auto input_idx : c10::irange(kNumInputs)) { | ||
loaded_vec_inputs[input_idx] = | ||
Vec::loadu(&inputs_data_ptrs[input_idx][idx]); | ||
} | ||
auto result_vec = std::apply(compute_fun, loaded_vec_inputs); | ||
result_vec.store(&data_out[idx]); | ||
} | ||
|
||
// Scalar epilogue. | ||
for (const auto idx : c10::irange(vectorized_end, end)) { | ||
std::array<CTYPE_COMPUTE, kNumInputs> loaded_inputs; | ||
for (const auto input_idx : c10::irange(kNumInputs)) { | ||
loaded_inputs[input_idx] = inputs_data_ptrs[input_idx][idx]; | ||
} | ||
data_out[idx] = std::apply(compute_fun, loaded_inputs); | ||
} | ||
}); | ||
return; | ||
} | ||
} | ||
#endif | ||
|
||
::executorch::extension::parallel_for( | ||
0, | ||
|
@@ -240,6 +336,19 @@ inline void apply_unitensor_elementwise_fn( | |
compute_fun, ctx, out, out_dtypes, std::make_pair(&a, a_dtypes)); | ||
} | ||
|
||
/** | ||
* Useful for unary elementwise operators. For each element of the | ||
* input, call Op and write to the corresponding element of the | ||
* output. Tensor broadcasting is applied wherever it is required. | ||
* | ||
* [NOTE: Generic lambdas]: If Op is a *generic* lambda (i.e., one with `auto` | ||
* parameters; normal lambdas are fine), it must fulfill one of the | ||
* following conditions. Either: | ||
* 1) It must in fact compile when passed at::vec::Vectorized<CTYPE_COMPUTE>, or | ||
* 2) It must be actively SFINAE-friendly, as per the C++17 examples in | ||
* https://stackoverflow.com/questions/76525790/detecting-if-a-generic-lambda-with-certain-arguments-is-invocable | ||
* . | ||
*/ | ||
template < | ||
typename CTYPE_COMPUTE, | ||
const char* op_name, | ||
|
@@ -281,6 +390,8 @@ inline void apply_bitensor_elementwise_fn( | |
* Useful for bi-tensor elementwise operators. For each element of the inputs, | ||
* perform a computation and write to the corresponding element of the output. | ||
* Tensor broadcasting is applied wherever it is required. | ||
* See [NOTE: Generic lambdas] if you want to pass a generic lambda for | ||
* compute_fun. | ||
*/ | ||
template < | ||
typename CTYPE_COMPUTE, | ||
|
@@ -347,6 +458,9 @@ inline void apply_tritensor_elementwise_fn( | |
* | ||
* static constexpr const char op_name[] = "my_op"; | ||
* apply_ternary_elementwise_fn<CTYPE_COMPUTE, op_name>. | ||
* | ||
* See [NOTE: Generic lambdas] if you want to pass a generic lambda for | ||
* compute_fun. | ||
*/ | ||
template < | ||
typename CTYPE_COMPUTE, | ||
|
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.
is this unrelated change?
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.
no, we can't vectorize this op