Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions src/ATen/native/xpu/sycl/Reduce.h
Original file line number Diff line number Diff line change
Expand Up @@ -1352,15 +1352,20 @@ inline void gpu_reduce_kernel(
// XXX: Avoid all WIs in a work group contributes on one output. If so,
// It is inefficient to store output, each work group stores only one
// output. It is not friendly to collapse memory request in an EU.
if (config.values_per_item() >= group_height * 16 ||
config.values_per_item() >= 512) {
// Divide the input across SGs in a work group, if that leaves at least
// 16 elements to be summed by each WI. This will require inter-SG
// reduction using shared memory.
config.input_mult[1] = config.split_input(group_height);
} else {
// Otherwise, each SG handles a separate output.
config.output_mult[1] = config.split_output(group_height);
//
// When input is vectorized, each work-item processes input_vec_size
// elements per load. Scale the threshold proportionally so that each
// work-item gets enough vector loads to amortize the inter-SG reduction
// overhead via shared local memory. For non-vectorized paths the factor
// is 1 and the condition is unchanged.
{
int vf = config.vectorize_input ? config.input_vec_size : 1;
if (config.values_per_item() >= group_height * 16 * vf ||
config.values_per_item() >= 512 * vf) {
config.input_mult[1] = config.split_input(group_height);
} else {
config.output_mult[1] = config.split_output(group_height);
}
Comment on lines +1361 to +1368
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new threshold scaling introduces an extra scope block solely to declare vf. This makes the control flow slightly harder to scan in a hot configuration section. Consider removing the braces and making vf a const local (or auto) declared in the surrounding scope so the split decision reads more directly.

Suggested change
{
int vf = config.vectorize_input ? config.input_vec_size : 1;
if (config.values_per_item() >= group_height * 16 * vf ||
config.values_per_item() >= 512 * vf) {
config.input_mult[1] = config.split_input(group_height);
} else {
config.output_mult[1] = config.split_output(group_height);
}
const auto vf = config.vectorize_input ? config.input_vec_size : 1;
if (config.values_per_item() >= group_height * 16 * vf ||
config.values_per_item() >= 512 * vf) {
config.input_mult[1] = config.split_input(group_height);
} else {
config.output_mult[1] = config.split_output(group_height);

Copilot uses AI. Check for mistakes.
}

// We are finding a general rountine to work out target max WI number on dev
Expand Down
18 changes: 7 additions & 11 deletions src/ATen/native/xpu/sycl/ReduceNormKernel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,33 +18,31 @@

namespace at::native::xpu {

// This reduction accumulates results as the type `acc_t`. By default, when
// `scalar_t` is complex, `acc_t` is the downgraded real number type.
// Otherwise, `acc_t` and `scalar_t` are the same type.
template <
typename scalar_t,
typename acc_t = typename scalar_value_type<scalar_t>::type,
typename out_t = typename scalar_value_type<scalar_t>::type>
void norm_kernel_xpu_impl(TensorIterator& iter, double p) {
constexpr int vt0 = 8;
if (p == static_cast<double>(0)) {
gpu_reduce_kernel<scalar_t, out_t>(
gpu_reduce_kernel<scalar_t, out_t, vt0>(
Comment on lines 25 to +28
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vt0 is hard-coded to 8 for all norm dtypes (including fp32/fp64 and complex via AT_DISPATCH_FLOATING_AND_COMPLEX_TYPES). Since vt0 directly controls unrolling/register usage in Reduce.h (see item_reduce_impl), this can increase register pressure and potentially regress non-16-bit paths. Consider selecting vt0 based on scalar_t (e.g., keep default for fp32/complex and use 8 only for fp16/bf16), or otherwise justify why 8 is safe across all dispatched types.

Copilot uses AI. Check for mistakes.
iter, NormZeroOps<scalar_t, acc_t, out_t>(), 0);
} else if (p == static_cast<double>(1)) {
gpu_reduce_kernel<scalar_t, out_t>(
gpu_reduce_kernel<scalar_t, out_t, vt0>(
iter, NormOneOps<scalar_t, acc_t, out_t>(), 0);
} else if (p == static_cast<double>(2)) {
gpu_reduce_kernel<scalar_t, out_t>(
gpu_reduce_kernel<scalar_t, out_t, vt0>(
iter, NormTwoOps<scalar_t, acc_t, out_t>(), 0);
} else if (p == static_cast<double>(INFINITY)) {
gpu_reduce_kernel<scalar_t, out_t>(
gpu_reduce_kernel<scalar_t, out_t, vt0>(
iter, AbsMaxOps<scalar_t, acc_t, out_t>(), 0);
} else if (p == static_cast<double>(-INFINITY)) {
gpu_reduce_kernel<scalar_t, out_t>(
gpu_reduce_kernel<scalar_t, out_t, vt0>(
iter,
AbsMinOps<scalar_t, acc_t, out_t>(),
std::numeric_limits<acc_t>::infinity());
} else {
gpu_reduce_kernel<scalar_t, out_t>(
gpu_reduce_kernel<scalar_t, out_t, vt0>(
iter, NormOps<scalar_t, acc_t, out_t>{acc_t(p)}, 0);
}
}
Expand All @@ -53,12 +51,10 @@ void norm_launch_kernel(TensorIterator& iter, double ord) {
if (iter.dtype(0) == kHalf) {
return norm_kernel_xpu_impl<at::Half, float>(iter, ord);
} else if (iter.input_dtype() == kHalf && iter.dtype(0) == kFloat) {
// type promotion that does cast and reduction in a single kernel
return norm_kernel_xpu_impl<at::Half, float, float>(iter, ord);
} else if (iter.dtype(0) == kBFloat16) {
return norm_kernel_xpu_impl<at::BFloat16, float>(iter, ord);
} else if (iter.input_dtype() == kBFloat16 && iter.dtype(0) == kFloat) {
// type promotion that does cast and reduction in a single kernel
return norm_kernel_xpu_impl<at::BFloat16, float, float>(iter, ord);
}
AT_DISPATCH_FLOATING_AND_COMPLEX_TYPES(iter.input_dtype(), "norm_xpu", [&] {
Expand Down
Loading