Skip to content

Commit 2dacff2

Browse files
Swap reverse-broadcast operands once in handle_broadcast_elementwise (#24809)
### Summary On the `*ReverseArguments` paths, `handle_broadcast_elementwise` hands the broadcasting maps `(b, a)`, so every non-commutative caller had to pass a hand-swapped lambda on those paths. `op_div` and `op_add_sub_impl` did, `op_le` did not (#24485). This swaps `vec_fun`'s arguments once inside `handle_broadcast_elementwise`, so the lambda always receives `(a, b)`, and removes the per-op swaps, resolving the TODO in `op_add_sub_impl.h`. `op_mul` is commutative, so it passes `kCommutative = true` to skip compiling the swapped copy, which would otherwise add about 16 KB to `op_mul`. With it, `op_mul` is unchanged and `op_div`/`op_add`/`op_sub`/`op_le` shrink slightly. Stacked on #24486, whose tests cover `le` on all three reverse paths; land after it. Authored with AI assistance (Claude Code). ### Test plan `optimized_kernels_test` passes (1839 tests, unchanged). With the central swap disabled, 9 add/sub/div/le tests fail, so the existing tests cover the swap for every non-commutative caller. --------- Co-authored-by: SofianElmotiem <sofianelmotiem@gmail.com>
1 parent 32d6f8b commit 2dacff2

4 files changed

Lines changed: 51 additions & 66 deletions

File tree

‎kernels/optimized/cpu/binary_ops.h‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -223,17 +223,15 @@ std::optional<BroadcastElementwisePlan> plan_broadcast_elementwise(
223223
const Tensor& b,
224224
Tensor& out,
225225
const ElementwiseOptimizedPath selected_optimized_path);
226-
} // namespace internal
227226

228227
template <typename CTYPE, typename Op>
229-
Tensor& handle_broadcast_elementwise(
228+
Tensor& handle_broadcast_elementwise_impl(
230229
KernelRuntimeContext& ctx,
231230
const Op& vec_fun,
232231
const Tensor& a,
233232
const Tensor& b,
234233
Tensor& out,
235-
const ElementwiseOptimizedPath selected_optimized_path,
236-
const std::optional<Scalar>& alpha = {}) {
234+
const ElementwiseOptimizedPath selected_optimized_path) {
237235
if ((selected_optimized_path ==
238236
ElementwiseOptimizedPath::kBroadcastLastDim) ||
239237
(selected_optimized_path ==
@@ -257,5 +255,35 @@ Tensor& handle_broadcast_elementwise(
257255
opt_plan->inner_size);
258256
return out;
259257
}
258+
} // namespace internal
259+
260+
// vec_fun always receives (a, b). The reverse paths hand the broadcasting maps
261+
// (b, a), so vec_fun's arguments are swapped back on those paths. Pass
262+
// kCommutative = true when the order doesn't matter to skip compiling the
263+
// swapped copy.
264+
template <typename CTYPE, bool kCommutative = false, typename Op>
265+
Tensor& handle_broadcast_elementwise(
266+
KernelRuntimeContext& ctx,
267+
const Op& vec_fun,
268+
const Tensor& a,
269+
const Tensor& b,
270+
Tensor& out,
271+
const ElementwiseOptimizedPath selected_optimized_path,
272+
const std::optional<Scalar>& alpha = {}) {
273+
if constexpr (!kCommutative) {
274+
if (selected_optimized_path ==
275+
ElementwiseOptimizedPath::kBroadcast2dBy1dReverseArguments ||
276+
selected_optimized_path ==
277+
ElementwiseOptimizedPath::kBroadcastLastDimReverseArguments ||
278+
selected_optimized_path ==
279+
ElementwiseOptimizedPath::kBroadcastNdByNdReverseArguments) {
280+
auto swapped_fun = [&vec_fun](auto x, auto y) { return vec_fun(y, x); };
281+
return internal::handle_broadcast_elementwise_impl<CTYPE>(
282+
ctx, swapped_fun, a, b, out, selected_optimized_path);
283+
}
284+
}
285+
return internal::handle_broadcast_elementwise_impl<CTYPE>(
286+
ctx, vec_fun, a, b, out, selected_optimized_path);
287+
}
260288
} // namespace executor
261289
} // namespace torch

‎kernels/optimized/cpu/op_add_sub_impl.h‎

Lines changed: 10 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -95,51 +95,17 @@ Tensor& opt_add_sub_out_impl(
9595
using Vec = at::vec::Vectorized<CTYPE>;
9696
Vec alpha_val_vec(alpha_val);
9797
if constexpr (is_sub) {
98-
if (selected_optimized_path ==
99-
ElementwiseOptimizedPath::kBroadcast2dBy1dReverseArguments ||
100-
selected_optimized_path ==
101-
ElementwiseOptimizedPath::kBroadcastLastDimReverseArguments ||
102-
selected_optimized_path ==
103-
ElementwiseOptimizedPath::kBroadcastNdByNdReverseArguments) {
104-
auto add_lambda = [&alpha_val_vec](auto x, auto y) {
105-
return y - alpha_val_vec * x;
106-
};
107-
torch::executor::handle_broadcast_elementwise<CTYPE>(
108-
ctx, add_lambda, a, b, out, selected_optimized_path, alpha);
109-
} else {
110-
auto add_lambda = [&alpha_val_vec](auto x, auto y) {
111-
return x - alpha_val_vec * y;
112-
};
113-
torch::executor::handle_broadcast_elementwise<CTYPE>(
114-
ctx, add_lambda, a, b, out, selected_optimized_path, alpha);
115-
}
98+
auto add_lambda = [&alpha_val_vec](auto x, auto y) {
99+
return x - alpha_val_vec * y;
100+
};
101+
torch::executor::handle_broadcast_elementwise<CTYPE>(
102+
ctx, add_lambda, a, b, out, selected_optimized_path, alpha);
116103
} else {
117-
if (selected_optimized_path ==
118-
ElementwiseOptimizedPath::kBroadcast2dBy1dReverseArguments ||
119-
selected_optimized_path ==
120-
ElementwiseOptimizedPath::kBroadcastLastDimReverseArguments ||
121-
selected_optimized_path ==
122-
ElementwiseOptimizedPath::kBroadcastNdByNdReverseArguments) {
123-
// Reason we swap out args here is because
124-
// handle_broadcast_elementwise handles this selected_optimized_path
125-
// option a bit differently. This should really be resolved in
126-
// handle_broadcast_elementwise. However, the current blocker is that
127-
// handle_broadcast_elementwise tries to be agnostic of op. This
128-
// should be fixed, likely by moving lambda creation to
129-
// handle_broadcast_elementwise and it be aware of which op is being
130-
// executed.
131-
auto add_lambda = [&alpha_val_vec](auto x, auto y) {
132-
return y + alpha_val_vec * x;
133-
};
134-
torch::executor::handle_broadcast_elementwise<CTYPE>(
135-
ctx, add_lambda, a, b, out, selected_optimized_path, alpha);
136-
} else {
137-
auto add_lambda = [&alpha_val_vec](auto x, auto y) {
138-
return x + alpha_val_vec * y;
139-
};
140-
torch::executor::handle_broadcast_elementwise<CTYPE>(
141-
ctx, add_lambda, a, b, out, selected_optimized_path, alpha);
142-
}
104+
auto add_lambda = [&alpha_val_vec](auto x, auto y) {
105+
return x + alpha_val_vec * y;
106+
};
107+
torch::executor::handle_broadcast_elementwise<CTYPE>(
108+
ctx, add_lambda, a, b, out, selected_optimized_path, alpha);
143109
}
144110
});
145111
} else {

‎kernels/optimized/cpu/op_div.cpp‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -137,20 +137,9 @@ Tensor& opt_div_out(
137137
// Reason for using alpha is becasuse handle_broadcast_elementwise
138138
// is used for add and sub as well:
139139
ET_SWITCH_REALB_TYPES(out_type, ctx, op_name, CTYPE, [&]() {
140-
if (selected_optimized_path ==
141-
ElementwiseOptimizedPath::kBroadcast2dBy1dReverseArguments ||
142-
selected_optimized_path ==
143-
ElementwiseOptimizedPath::kBroadcastLastDimReverseArguments ||
144-
selected_optimized_path ==
145-
ElementwiseOptimizedPath::kBroadcastNdByNdReverseArguments) {
146-
auto div_lambda = [](auto x, auto y) { return y / x; };
147-
torch::executor::handle_broadcast_elementwise<CTYPE>(
148-
ctx, div_lambda, a, b, out, selected_optimized_path);
149-
} else {
150-
auto div_lambda = [](auto x, auto y) { return x / y; };
151-
torch::executor::handle_broadcast_elementwise<CTYPE>(
152-
ctx, div_lambda, a, b, out, selected_optimized_path);
153-
}
140+
auto div_lambda = [](auto x, auto y) { return x / y; };
141+
torch::executor::handle_broadcast_elementwise<CTYPE>(
142+
ctx, div_lambda, a, b, out, selected_optimized_path);
154143
});
155144
} else {
156145
ScalarType common_type = get_common_type(a.scalar_type(), b.scalar_type());

‎kernels/optimized/cpu/op_mul.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,14 +101,16 @@ Tensor& opt_mul_out(
101101

102102
ET_SWITCH_COMPLEXH_TYPES(out_type, ctx, op_name, CTYPE, [&]() {
103103
auto mul_lambda = [](auto x, auto y) { return x * y; };
104-
torch::executor::handle_broadcast_elementwise<CTYPE>(
105-
ctx, mul_lambda, a, b, out, selected_optimized_path);
104+
torch::executor::
105+
handle_broadcast_elementwise<CTYPE, /*kCommutative=*/true>(
106+
ctx, mul_lambda, a, b, out, selected_optimized_path);
106107
});
107108
} else {
108109
ET_SWITCH_REALB_TYPES(out_type, ctx, op_name, CTYPE, [&]() {
109110
auto mul_lambda = [](auto x, auto y) { return x * y; };
110-
torch::executor::handle_broadcast_elementwise<CTYPE>(
111-
ctx, mul_lambda, a, b, out, selected_optimized_path);
111+
torch::executor::
112+
handle_broadcast_elementwise<CTYPE, /*kCommutative=*/true>(
113+
ctx, mul_lambda, a, b, out, selected_optimized_path);
112114
});
113115
}
114116
} else {

0 commit comments

Comments
 (0)