Skip to content

Add extension subgroup-size-control - #5578

Merged
dneto0 merged 33 commits into
gpuweb:mainfrom
Jiawei-Shao:add-subgroup-size-control
Jun 23, 2026
Merged

Add extension subgroup-size-control#5578
dneto0 merged 33 commits into
gpuweb:mainfrom
Jiawei-Shao:add-subgroup-size-control

Conversation

@Jiawei-Shao

@Jiawei-Shao Jiawei-Shao commented Feb 27, 2026

Copy link
Copy Markdown
Contributor

This patch adds a new WebGPU and WGSL extension subgroup-size-control
based on subgroup-size-control.md.

Fixed: #5545

@Jiawei-Shao
Jiawei-Shao marked this pull request as draft February 27, 2026 07:19
@Jiawei-Shao
Jiawei-Shao marked this pull request as ready for review February 27, 2026 07:58
@github-actions

github-actions Bot commented Feb 27, 2026

Copy link
Copy Markdown
Contributor

Previews, as seen when this build job started (476428f):
WebGPU webgpu.idl | Explainer | Correspondence Reference
WGSL grammar.js | wgsl.lalr.txt

@dneto0
dneto0 requested review from dneto0 February 27, 2026 14:40
Comment thread spec/index.bs Outdated
Comment thread spec/index.bs Outdated
Comment thread spec/index.bs Outdated
Comment thread wgsl/index.bs Outdated
Comment thread wgsl/index.bs Outdated
Comment thread wgsl/index.bs Outdated
@Jiawei-Shao
Jiawei-Shao marked this pull request as draft March 4, 2026 07:59
@Jiawei-Shao
Jiawei-Shao marked this pull request as ready for review March 4, 2026 08:02
@jimblandy

Copy link
Copy Markdown
Contributor

Should this PR also update the correspondence reference?

@jimblandy jimblandy mentioned this pull request Mar 10, 2026
@Jiawei-Shao
Jiawei-Shao marked this pull request as draft March 12, 2026 03:06
@Jiawei-Shao
Jiawei-Shao marked this pull request as ready for review March 12, 2026 03:17
@Jiawei-Shao

Copy link
Copy Markdown
Contributor Author

Should this PR also update the correspondence reference?

Done

@Jiawei-Shao

Copy link
Copy Markdown
Contributor Author

PTAL, thanks!

@jimblandy

Copy link
Copy Markdown
Contributor

I'll take a look at this today.

@jimblandy jimblandy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I found a few editorial issues. Please look through the generated HTML to make sure the formatting is coming through the way it should.

Comment thread wgsl/index.bs
Comment thread wgsl/index.bs Outdated
Comment thread wgsl/index.bs

@jimblandy jimblandy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Editorial issues aside, this looks good to me.

Comment thread correspondence/index.bs Outdated
@Jiawei-Shao

Copy link
Copy Markdown
Contributor Author

API LGTM. The last WGSL changes should still be reviewed by a WGSL person. And we still need tests before landing (that's tracked by the open comment, which will block the PR from landing).

Hi @kainino0x,

I've added all the tests to CTS (in gpuweb/cts#4640).

PTAL, thanks!

@jimblandy

Copy link
Copy Markdown
Contributor

The CTS PR has been merged. @kainino0x, is this ready to land?

@jimblandy

Copy link
Copy Markdown
Contributor

The CTS PR has been merged. @kainino0x, is this ready to land?

I was pushing to get this resolved, but I want to withdraw my pressure here. Based on the discussion above, it looks to me like this question is still very much unresolved. The web will have to deal with whatever we put in the spec forever, so it is very much worthwhile for us to spend a few weeks ensuring that we're doing the best we can do.

Comment thread wgsl/index.bs
@Jiawei-Shao

Copy link
Copy Markdown
Contributor Author

Hi @jimblandy @jrprice @kainino0x ,

I've landed the PR to test all valid subgroup sizes to the CTS. What else should I do to move this PR forward?

@kainino0x

kainino0x commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

The only other thing was to run that test on a reasonable set of different hardware and make sure that our assertion (that it should pass almost everywhere) seems to be true. The Chromium bots probably test enough to cover that, but since the test just landed, it hasn't been run yet.

@jimblandy

Copy link
Copy Markdown
Contributor
minutes from WGSL committee meeting 2026-06-09
  • JB: Kai Ninomiya raised some issues in his review.
  • JB: Between discussion between KN and JP about the fact that fragment shaders on intel have smaller subgroup size then compute shaders can accept and there was discussion about if we should have a separate pair of limits where every size in the range is one that a compute shader can support and then there is the concern however that the decision if a subgroup size is supported can depend on register pressure which is unpredictable to the spec. So, we may need to require content to try different subgroup sizes with shader and see if the compiler will accept it. If that's the case, then no need to change the minimum in order to make sure it doesn't include values that the compute shaders can't handle. So can include the frag shader value of 8 in the range. Proposal has been sitting for a while, do not to land something we don't intend to implement as it is then set in stone. Want to get this right, but we should not drag out longer then is useful.
  • AB: Reason we went with not trying to put the extra limit is was forward looking that it was one class of older hardware, in the future this becomes a non-issue. Didn't want to be stuck with limits that are outdated. Something on one set of hardware that may have a problem compiling. If you can distinguish that from any other dynamic compile error, don't think a user could tell. Not gaining a lot of information. Thinking ahead to the future of what we'd like this to look like, and what's the nicest way to support that's why we decided to not go with a separate limit.
  • JB: So, in all hardware except this family of intel devices, the number of subgroup sizes that are announced in adapter info will fall in the same range for both compute and fragment
  • AB: That's my understanding. This is a bug in a particular series of hardware.
  • JB: Then it is harmless to use 8 as the min-shader size because any real content using subgroup size control must be prepared for compilation to fail already
  • AB: More then subgroup size, in general shaders. We make a claim about available workgroup memory but you could fail before that limit for lots of reasons. Being able to tell the user the reason is beyond our control in a lot of cases. Not something where you can tell if you tried this other thing it would work. Whole reason for uncategorized errors. Things under us we can't control.
  • JB: The way i imagine this working is because WGSL has no recursion we could layout the stack and privates in advance. A device driver could compute the per thread data in advance and will try to move as much as possible into registers and then it will have some leftover that it will put in workgroup memory but at any point workgroup memory is limited so at any point based on how well it fit into registers you can get a situation where the device can't handle it and we get an error. Because that's dependent on how it decided to generate the code and the register architecture it isn't something we can predict.
  • AB: I think that's right. Also compiler chicken and egg, subgroup size decides number of registers your using. Not perfect information based on flow.
  • JB: When we fix subgroup size it constrains the compiler more and could make the decision faster but we haven't fixed it so it can't
  • AB: Think so, but don't have backend knowledge
  • JB: IN that case, we can treat the fact the whole range isn't useable as an inevitability so having the range be larger is fine. So we don't need a separate range.
  • KN: I think that addresses my concerns. Don't entirely see the purpose of the min/max subgroup count in the first place if this is how it works. I thought we'd be able to write a test (a compute shader) that does nothing, with no register pressure, but write a test that says it will pass for any of the subgroup sizes in the range. Really want to do that but if we can't then …
  • JB: If an implementation can say [1, 8k] and that's compliant, then what's the point. I think there is merit to that.
  • KN: Kind of depends. I want to write that test but if it doesn't pass on the hardware we've seen then it doesn't matter
  • AB: I think it's errata. We should write the test and mark failing on that generation of hardware
  • KN: If that's the only place it fails then that's fine. I would like if we just say min was 16 and not 8, but probably we don't want to change what we've shipped already. 8 is fine. My original comment was can we just say subgroupMinSize=16 and use subgroup size control to prevent actually creating fragment pipelines with subgroup size 8. But that's not possible if subgroup size control doesn't work in fragment. Was trying to sand off edges, but looks like there are no better changes we can make to do that.
  • JB: That was the 3rd bullet, but doesn't seem implementable.
  • AB: That's due to d3d? [checked, yes attribute is compute only in d3d]
  • KN: [clarification on last paragraph, clarified in notes]
  • JB: So we're in the 'if not' part of the bullet
  • JB: RESOLUTION: Have a single range as we do now. The CTS will test that a trivial compute shader can be created using each power of 2 in that range. It will be flagged as expected fail on this generation of intel (like an errata) and drivers will always insist on having the privilege of rejecting a shader they don't like and there is nothing we can do about it.
  • AB: Talked about creating the errata list and this would go on it. We could also fix this in Vulkan and just have on D3D
  • JP: I think it already is, the issue is only D3D
  • JB: Does cts follow the resolution?
  • JP: No, CTS changes landed dynamically test for the sizes and then excludes any that fail from the tests. So it dynamically figures out the edge case in CTS itself. Will need CTS changes to treat as an errata
  • JB: Any objections to have CTS expect all to pass in range and treating this device as errata?
  • JP: Think it's fine.
  • JB: Next steps, update CTS. Once that's done, let's try to not make this come through another iteration. We'll put our approvals on PR.

@jimblandy

jimblandy commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

My personal summary of the decisions from the 2026-6-9 meeting:

  • WebGPU should validate @subgroup_size attribute values to fall within the range established by GPUAdapterInfo's subgroupMinSize and subgroupMaxSize attributes.
  • The CTS should test that a pipeline with a trivial compute shader can be created successfully with all powers of two in that range. (If an implementation advertises subgroup sizes that it can never actually support, the CTS should object to that.)
  • For non-trivial compute shaders, the CTS should be prepared for specifying the subgroup size to cause shader compilation or pipeline creation to fail. That is, the CTS should not expect that, simply because a trivial shader can be compiled with a given @subgroup_size attribute value, more complicated shaders can also be compiled in that way.
  • The CTS should treat subgroup sizes reported to fragment shaders that fall outside the subgroup{Min,Max}Size range as known errata on the older Intel GPUs where this occurs.

In the general case, WebGPU cannot promise that all subgroup sizes in the advertised subgroup{Min,Max}Size range are actually going to work for arbitrary shaders, since any shader compilation can fail due to factors outside our control, like register pressure. We can't even guarantee successful compilation of shaders when there are no @subgroup_size attributes in the picture at all. The CTS must content itself with verifying that trivial shaders, at least, can be compiled.

@dneto0
dneto0 merged commit 43fd093 into gpuweb:main Jun 23, 2026
4 checks passed
@jimblandy

Copy link
Copy Markdown
Contributor
minutes from WGSL committee meeting 2026-6-23
  • JB: PR and CTS have been revised. CTS results from Chrome CI should be available by meeting time. Let’s look at those and decide the next step (to merge, hopefully).
  • JP: CTS runs have passed on Chrome CI, modulo an unrelated bug in Chrome. Consider the CTS good.
  • DN: Google says take the PR.
  • JB: Mozilla say take the PR.
  • Resolved: Take the PR.

Kangz pushed a commit to gpuweb/types that referenced this pull request Jun 24, 2026
This patch adds the "subgroup-size-control" feature added to the spec in
gpuweb/gpuweb#5578
Kangz pushed a commit to webgpu-native/webgpu-headers that referenced this pull request Jun 24, 2026
This patch adds the "subgroup-size-control" feature added in
gpuweb/gpuweb#5578.
@kainino0x kainino0x added the wgsl WebGPU Shading Language Issues label Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wgsl WebGPU Shading Language Issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Subgroup Size Control

9 participants