[Perf][Pool] Optimize avg_pool2d kernels#1665
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a fast-path spatial kernel (AvgPool2dSpatialKernel) for common NCHW 2D average pooling workloads, refactoring the existing _avg_pool2d_kernel to simplify its block configuration. It also updates the forward operator to dispatch to this new spatial kernel when specific fast-path conditions are met. Feedback on the changes suggests adding a defensive check in the spatial kernel's forward method to handle cases where the output dimensions are zero, preventing potential CUDA configuration errors or crashes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
superAngGao
left a comment
There was a problem hiding this comment.
Thanks for the optimization. The fast-path split and the flatter fallback look directionally good to me. I have two contract-level comments before approval:
-
Please sync the manifest with the new dispatch surface.
AvgPool2dFwdOp.default_kernel_mapnow contains bothavg_pool2d_kernelandavg_pool2d_spatial_kernel, buttileops/manifest/pool.yamlstill declares onlyavg_pool2d_kernel. The ops design docs saydefault_kernel_mapshould matchsource.kernel_mapverbatim, and the benchmark/reporting path is manifest-driven here, so the new spatial kernel should be listed there as well. It may also be worth exportingAvgPool2dSpatialKernelfrom the top-leveltileops.kernelspackage for consistency with the other pool kernels. -
Please double-check the custom
kernel_mappartial override semantics. Becausedispatch_kernel()fills missing entries fromdefault_kernel_map, a caller that passes only{"avg_pool2d_kernel": MyKernel}will still get the defaultAvgPool2dSpatialKernelinserted. For fast-path parameter sets, dispatch will then choose the default spatial kernel and bypass the user-provided general kernel. I think user-supplied overrides should either explicitly opt into the spatial fast path or fall back to the providedavg_pool2d_kernel; otherwise tests/experiments that override the existing kernel key can silently stop exercising their replacement. A small smoke test for this case, plus one for non-fast-path dispatch, would lock the behavior down.
The core indexing/divisor logic looks reasonable to me, and the output-size guard from the earlier bot comment appears to be addressed.
Ibuki-wind
left a comment
There was a problem hiding this comment.
Clean under a correctness-focused review. I treated manifest/export synchronization and the custom kernel_map override semantics as follow-up contract work rather than blockers for this PR.
The fast-path guard is narrow enough for the fixed-divisor spatial kernel, the general fallback keeps the PyTorch divisor/boundary semantics while flattening the launch grid, and the new output-size guard matches PyTorch error behavior for non-positive outputs. The added avg_pool2d cases cover the changed dispatch paths and the ceil/count/divisor combinations touched by the fallback rewrite.
I did not run local GPU tests or benchmarks; GitHub gpu-smoke/preflight are green and the PR includes H200 benchmark results.
Closes #1654
Summary
AvgPool2dSpatialKernelfast path for the common NCHW case withceil_mode=False,count_include_pad=True, and nodivisor_override.AvgPool2dKernelfallback to use a flatN*C*H_out*W_outlaunch grid and remove theblock_ctuning dimension.AvgPool2dFixturecoverage forceil_mode,count_include_pad, anddivisor_overridecombinations.Test plan
Result:
Result:
Benchmark
Final benchmark on NVIDIA H200:
vision-3x3-s2AvgPool2dSpatialKernel0.05050.00460.009410.98x faster2.04x fastervision-5x5-s2AvgPool2dSpatialKernel0.03720.00450.01028.27x faster2.27x fasterceil-divisor-bf16AvgPool2dKernelflat general0.01490.00380.00803.92x faster2.11x fasterAutotune best configs from the final run:
vision-3x3-s2AvgPool2dSpatialKernel{"block_m": 256, "threads": 256}vision-5x5-s2AvgPool2dSpatialKernel{"block_m": 512, "threads": 256}ceil-divisor-bf16AvgPool2dKernel{"block_m": 256, "threads": 256}Notes on the baseline values:
vision-3x3-s2andvision-5x5-s2"before" values are from the original TileOps kernel measured during the avg_pool2d performance investigation.ceil-divisor-bf16"before" value is the local pre-flat-general fallback re-run used for the review comparison.Profiling summary
The benchmark improvement is consistent with fixed-config
nsysandncudata.For
vision-3x3-s2,ncushows that TileOps executes much less work than torch:5.12 us10.05 us1.481.4873.61%79.44%44.53%67.92%628.30 GB/s321.45 GB/s1.09M4.24MTorch has higher SM busy, but it issues about
3.9xmore instructions. The TileOps fast path avoids the general pooling divisor/control path and reaches about2xhigher effective memory throughput.For
vision-5x5-s2, fixed-confignsysreports:3.808 us9.664 us3.822 us9.672 us392 x 256196 x 10242932For
ceil-divisor-bf16, the new flat general fallback is also faster than torch:3.456 us7.488 us4.48 us8.96 us914 x 256229 x 10240.870.8768.44%73.10%43.02%65.50%1.423.65404.46 GB/s203.60 GB/s875K3.28MTorch again keeps the SMs busier, but it issues about
3.75xmore instructions. The flat general kernel removes the oldblock_csplit, launches overN*C*H_out*W_out, and completes the same workload with a shorter instruction path and higher effective memory throughput.