[WebGPU] Pin subgroup size to 32 for subgroup-matrix MatMul/Gemm - #32306
Conversation
The 8x16x16 subgroup-matrix templates hard-code 32 lanes per subgroup and derive sg_index/sg_lane from it. Request the size explicitly with SetSubgroupSize(32).
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Pins WebGPU subgroup-matrix MatMul and Gemm kernels to their required 32-lane subgroup size.
Changes:
- Requests subgroup size 32 during program setup.
- Falls back when subgroup-size control is unavailable.
- Removes resolved TODOs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
subgroup_matrix_matmul.cc |
Pins and gates MatMul subgroup size. |
subgroup_matrix_gemm.cc |
Pins and gates Gemm subgroup size. |
No actionable issues found.
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Review — PR #32306: [WebGPU] Pin subgroup size to 32 for subgroup-matrix MatMul/Gemm Bug — what's actually broken pre-PR The 8x16x16 F16 subgroup-matrix templates in subgroup_matrix_matmul.cc and subgroup_matrix_gemm.cc hard-code 32 lanes per subgroup ( The adapters that report the 8x16x16 F16 config (Intel Xe HPG / Arc-family) expose a 16-32 subgroup size range — i.e., WebGPU is free to pick 16 or 32 depending on shader occupancy heuristics. When the runtime picks 16, the kernel:
Fix: explicitly request 32 lanes via The change — mechanics Symmetric edits to both kernels:
Concise and correct explanation, gives the next reader the specific hardware rationale without hand-waving. Correctness notes
Prerequisite check — Grepping onnxruntime/core/providers/webgpu/program.h in my local snapshot, I see only the three Confirm this by checking: Non-blocking, but worth verifying before merge — a build failure on Test coverage None added. Reasonable — this is a hardware-behavior-dependent fix (Intel adapters that pick 16 vs. 32 subgroups at runtime), and there's no portable unit test that would catch it. Existing MatMul/Gemm numerical tests already cover the F16 output; if they didn't fail on Intel Arc pre-PR, that's either (a) the runtime happened to pick 32 in the tested workloads, or (b) the numerical tolerance was loose enough to mask the corruption. Neither is testable in a portable way. If the WebGPU CI has an Intel-Arc-capable leg, running the standard MatMul suite pre- and post-PR would show the delta; if not, this fix relies on hand-validation by the author. CI status 40/81 checks OK at last look — that's a mid-run snapshot, not a settled state. Hariharan requested a Copilot re-review 30 minutes before I looked; several CI legs are likely still queuing/running. Copilot itself already reviewed and generated no comments on both passes. Wait for CI to settle before merge — the interesting checks are the WebGPU CI legs ( Style / nits
Recommendation Approve — pending:
The fix is minimal, correctly diagnosed, symmetrically applied to both kernels, and the fallback semantics preserve pre-existing behavior for adapters that can't honor the pin. This is precisely the right shape for a "kernel-invariant pinning" bug fix. |
|
Jiajia Qin (@qjia7) PTAL |
725aa0a
into
microsoft:main
The 8x16x16 subgroup-matrix templates hard-code 32 lanes per subgroup and derive sg_index/sg_lane from it. Request the size explicitly with SetSubgroupSize(32).