Skip to content

Commit aa54c1f

Browse files
committed
Swap reverse-broadcast operands once in handle_broadcast_elementwise
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. 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. Authored with AI assistance (Claude Code).
1 parent d5805e4 commit aa54c1f

5 files changed

Lines changed: 54 additions & 80 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_le.cpp‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -57,20 +57,9 @@ Tensor& opt_le_tensor_out(
5757
} else if (selected_optimized_path != ElementwiseOptimizedPath::kNone) {
5858
// Handle optimized broadcast cases
5959
ET_SWITCH_REALB_TYPES(out_type, ctx, op_name, CTYPE, [&]() {
60-
if (selected_optimized_path ==
61-
ElementwiseOptimizedPath::kBroadcast2dBy1dReverseArguments ||
62-
selected_optimized_path ==
63-
ElementwiseOptimizedPath::kBroadcastLastDimReverseArguments ||
64-
selected_optimized_path ==
65-
ElementwiseOptimizedPath::kBroadcastNdByNdReverseArguments) {
66-
auto le_lambda = [](auto x, auto y) { return y.le(x); };
67-
torch::executor::handle_broadcast_elementwise<CTYPE>(
68-
ctx, le_lambda, a, b, out, selected_optimized_path);
69-
} else {
70-
auto le_lambda = [](auto x, auto y) { return x.le(y); };
71-
torch::executor::handle_broadcast_elementwise<CTYPE>(
72-
ctx, le_lambda, a, b, out, selected_optimized_path);
73-
}
60+
auto le_lambda = [](auto x, auto y) { return x.le(y); };
61+
torch::executor::handle_broadcast_elementwise<CTYPE>(
62+
ctx, le_lambda, a, b, out, selected_optimized_path);
7463
});
7564
} else {
7665
internal::comparison_tensor_out<std::less_equal, op_name>(ctx, a, b, out);

‎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)