这是indexloc提供的服务，不要输入任何密码
Skip to content

Add subgroups feature #4963

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 22 commits into from
Jan 16, 2025
Merged

Add subgroups feature #4963

merged 22 commits into from
Jan 16, 2025

Conversation

alan-baker
Copy link
Contributor

@alan-baker alan-baker commented Nov 8, 2024 •
edited by dneto0
Loading

  • API
    • new feature: subgroups
    • new properties for adapter info:
      • minSubgroupSize subgroupMinSize
      • maxSubgroupSize subgroupMaxSize
  • WGSL
    • new built-in values
      • subgroup_invocation_id
      • subgroup_size
    • subgroup and quad built-in functions
    • new uniformity diagnostic subgroup_uniformity

Missing subgroup_branching diagnostic and uniformity updates.

edited to match final naming: 'minSubgroupSize' was changed to 'subgroupMinSize', 'maxSubgroupSize' was changed to 'subgroupMaxSize'

@alan-baker
Add subgroups feature
b90bc5b 
* API
  * new feature: subgroups
  * new properties for adapter info:
    * minSubgroupSize
    * maxSubgroupSize
* WGSL
  * new built-in values
    * subgroup_invocation_id
    * subgroup_size
  * subgroup and quad built-in functions
  * new uniformity diagnostic subgroup_uniformity
@alan-baker
Merge remote-tracking branch 'origin/main' into milestone-1-subgroups
3a5e0cf
@alan-baker alan-baker added this to the Milestone 1 milestone Nov 8, 2024
@alan-baker alan-baker mentioned this pull request Nov 8, 2024
Closed
alan-baker
alan-baker commented Nov 8, 2024
View reviewed changes
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 8, 2024 •
edited
Loading

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

dneto0
dneto0 approved these changes Nov 8, 2024
View reviewed changes
Copy link
Contributor

@dneto0 dneto0 left a comment

Choose a reason for hiding this comment

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

Main question is whether we want a new GPUAdapterProperties object or not

@dneto0 dneto0 self-requested a review November 8, 2024 20:04
Kangz
Kangz reviewed Nov 12, 2024
View reviewed changes
Copy link
Contributor

@Kangz Kangz left a comment

Choose a reason for hiding this comment

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

Looks great! Added a bunch of comments but they are relatively minor.

jrprice
jrprice reviewed Nov 12, 2024
View reviewed changes
@beaufortfrancois beaufortfrancois mentioned this pull request Nov 13, 2024
Closed
kainino0x
kainino0x reviewed Nov 14, 2024
View reviewed changes
Copy link
Contributor

@kainino0x kainino0x left a comment

Choose a reason for hiding this comment

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

No further comments on the API side

@alan-baker
Changes for review
60e26b5 
* move minSubgroupSize and maxSubgroupSize into GPUAdapterInfo
  * provide default values
* Fix link to wgsl spec
* Fix uniformity rules in wgsl related to parameters they are required
  to be uniform
kainino0x
kainino0x reviewed Nov 14, 2024
View reviewed changes
dneto0
dneto0 requested changes Nov 14, 2024
View reviewed changes
Copy link
Contributor

@dneto0 dneto0 left a comment

Choose a reason for hiding this comment

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

WGSL side looking very good. Thanks!

alan-baker and others added 4 commits November 15, 2024 09:41
@alan-baker @kainino0x
Update spec/index.bs
e569df7 
Co-authored-by: Kai Ninomiya <kainino@chromium.org>
@alan-baker
Changes for review
a5ef6bc 
* Mention subgroup_size is uniform in compute shaders in the uniformity
  analysis description of built-in values
* Updates to the subgroups section
* Fix typos and links
@alan-baker
fix bad edit
e538bc7
@alan-baker
improve subgroup builtin value descriptions
f5a2dff
dneto0
dneto0 reviewed Nov 18, 2024
View reviewed changes
dneto0
dneto0 reviewed Nov 18, 2024
View reviewed changes
@kainino0x kainino0x added wgsl WebGPU Shading Language Issues api WebGPU API labels Nov 18, 2024
@alan-baker
Changes for review
938337c 
* add notes about uniform control flow scope
* add prefix to inclusive and exclusive subgroup op descriptions
@alan-baker alan-baker changed the title WIP: Add subgroups feature Add subgroups feature Dec 4, 2024
mwyrzykowski
mwyrzykowski previously approved these changes Dec 6, 2024
View reviewed changes
Copy link

@mwyrzykowski mwyrzykowski left a comment

Choose a reason for hiding this comment

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

Looks good!

@mwyrzykowski mwyrzykowski dismissed their stale review December 6, 2024 18:27

Want to ask a question

mwyrzykowski
mwyrzykowski reviewed Dec 6, 2024
View reviewed changes
jimblandy
jimblandy reviewed Dec 10, 2024
View reviewed changes
jimblandy
jimblandy reviewed Dec 10, 2024
View reviewed changes
jimblandy
jimblandy reviewed Dec 10, 2024
View reviewed changes
jimblandy
jimblandy reviewed Dec 10, 2024
View reviewed changes
@alan-baker
Changes for review
eff1559 
* Add extra spec cross-links for subgroups
jimblandy
jimblandy reviewed Dec 10, 2024
View reviewed changes
@kdashg
Copy link
Contributor

kdashg commented Dec 10, 2024

WGSL 2024-12-10 Minutes
  • (touch-base, status updates)
  • JB: Some review. Some question of properties into their own interface. Min size and max size are not limits; don’t work as limits. Comment on another issue “separate out properties as their own thing when we need them”.
  • JB: Is there a concept of a subgroup index, as a subset of a workgroup.
  • AB: There is but not in every API.
  • AB: There is no reliable mapping between linear invocation Index in a compute shader, and subgroup index.
  • JB: So no reliable chunking of local IDs into subgroups.
  • JB: When someone uses this API they have to write as if an invocation’s subgroup members might be anywhere in the workgroup.
  • AB: There’s nothing you can rely on in all cases. If you have a linear workgroup you do tend to see linear subgroup in an obvious way. Difficulty comes when you have multidimensional workgroups.
  • AB: D3D doesn’t have a subgroup ID. You have to write code to avoid assuming the association.
  • JB: I can’t figure out how to use this. I can use ballot or sum. But what elements am I even operating on. There are no guarantees.
  • AB: A lot of operations are reductions, and you don’t need to know the relative order of things because you reduce all the way down. The CTS tests go out of their way to avoid the assumptions. It often generates its own subgroup ID specific to the execution. E.g. use a broadcast of the invocation index of the first element in the subgroup. So everyone in the subgroup shares some arbitrary value.
  • JB: And you use the index that you place the subgroup results. And you accept the possibility it’s not 100% dense.
  • AB: Most of the time you want to resolve results from the whole workgroup, and so it ends up dense. You can reconstruct where each invocation was placed.
  • JB: CTS is one thing, but doesn’t have to produce a result an ordinary program might want to do.
  • AB: Think of [atomic?] decimated compaction. Use two passes. First pass decides who needs to write, then you use atomics and reductions to calculate the index into which subgroup leads write.
  • JB: I wonder if there are techniques like you describe that are ubiquitous that are so valuable such that we supply them as recommended primitives to give people a foothold.
  • AB: Maybe. It came up in internal discussion to add language features that add polyfills over the enable-extension baseline feature.
  • JB: I put other comments on the PR, don’t need to talk about it here.
  • MW: do we need some kind of barrier functions for subgroups? OR do implementations insert the barriers.
  • AB: There are no new barriers because there no “subgroup memory” memory locations to protect. So you reuse the existing barriers to order memory accesses. In Vulkan the subgroup-execution-scope barriers are non-uniform, which is tricky ot think about. So you end up trying to ensure subgroup reconvergence, then use a workgroup barrier.
  • MW: Sounds good.
  • Reviews and iteration will continue.

@alan-baker alan-baker requested a review from jimblandy December 18, 2024 16:13
This was referenced Jan 8, 2025
Closed
Open
aarongable pushed a commit to chromium/chromium that referenced this pull request Jan 11, 2025
@beaufortfrancois
webgpu: Add experimental subgroupMinSize & subgroupMaxSize adapter info
6045b4d 
This CL adds the subgroupMinSize and subgroupMaxSize properties to
GPUAdapterInfo behind the experimental WebGPUSubgroupsFeatures blink
runtime feature.

Spec PR: gpuweb/gpuweb#4963

Change-Id: I4f9edbd6482a0f7a1f98086bcba1205b5e24ceea
Bug: 354751907
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6162500
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Fr <beaufort.francois@gmail.com>
Cr-Commit-Position: refs/heads/main@{#1405153}
jimblandy
jimblandy approved these changes Jan 16, 2025
View reviewed changes
Copy link
Contributor

@jimblandy jimblandy left a comment

Choose a reason for hiding this comment

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

This looks good to me. I have some editorial comments, but nothing blocking.

wgsl/index.bs
Comment on lines +18942 to +18943
Returns the maximum value of `e` among all [=subgroups/active=] invocations in the [=subgroup=].
</table>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this discuss subnormal and NaN behavior, the way the description of max does?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed this as #5044

alan-baker
alan-baker commented Jan 16, 2025
View reviewed changes
alan-baker and others added 2 commits January 16, 2025 10:12
@alan-baker @jimblandy
Apply suggestions from code review
f950527 
Co-authored-by: Jim Blandy <jimb@red-bean.com>
@alan-baker @jimblandy
Update wgsl/index.bs
476f3c1 
Co-authored-by: Jim Blandy <jimb@red-bean.com>
@alan-baker alan-baker merged commit bb105af into main Jan 16, 2025
4 checks passed
@alan-baker alan-baker mentioned this pull request Jan 16, 2025
Open
@jimblandy jimblandy mentioned this pull request Aug 21, 2024
Open
8 tasks
aarongable pushed a commit to chromium/chromium that referenced this pull request Jan 22, 2025
@beaufortfrancois
WebGPU: Remove experimental WebGPU limits for subgroups
3acdb27 
Spec PR: gpuweb/gpuweb#4963

Bug: 354751907
Change-Id: If5823c0ff95b4de72d0f95e1a5cd1cd92e3b5459
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6163583
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
Commit-Queue: Fr <beaufort.francois@gmail.com>
Cr-Commit-Position: refs/heads/main@{#1409490}
@fairywreath fairywreath mentioned this pull request Jan 29, 2025
Closed
@beaufortfrancois beaufortfrancois mentioned this pull request Mar 3, 2025
Merged
@captainbrosset captainbrosset mentioned this pull request Mar 21, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kangz Kangz Kangz left review comments

@kainino0x kainino0x kainino0x left review comments

@jrprice jrprice jrprice left review comments

@jimblandy jimblandy jimblandy approved these changes

@dneto0 dneto0 dneto0 approved these changes

@mwyrzykowski mwyrzykowski mwyrzykowski approved these changes

@toji toji Awaiting requested review from toji

Assignees
No one assigned
Labels
api WebGPU API wgsl WebGPU Shading Language Issues
Projects
None yet
Milestone
Milestone 1
Development

Successfully merging this pull request may close these issues.
8 participants
@alan-baker @kdashg @Kangz @kainino0x @jimblandy @jrprice @dneto0 @mwyrzykowski