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

workgroupStorageUsed is sum of values, each rounded up to multiple of 16 #3522

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 2 commits into from
Oct 11, 2022
Merged

workgroupStorageUsed is sum of values, each rounded up to multiple of 16 #3522

merged 2 commits into from
Oct 11, 2022

Conversation

dneto0
Copy link
Contributor

@dneto0 dneto0 commented Oct 11, 2022

Fixes: #3485

@dneto0 dneto0 requested review from kainino0x and toji October 11, 2022 21:20
@github-actions
Copy link
Contributor

github-actions bot commented Oct 11, 2022 •
edited
Loading

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

kainino0x
kainino0x approved these changes Oct 11, 2022
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.

LGTM, thanks! Landing now as resolved by the WGSL group.

@kainino0x kainino0x added the needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet label Oct 11, 2022
@kainino0x kainino0x added this to the V1.0 milestone Oct 11, 2022
@kainino0x kainino0x enabled auto-merge (squash) October 11, 2022 21:32
@kainino0x kainino0x added the tacit resolution queue Editors have agreed and intend to land if no feedback is given label Oct 11, 2022
@kainino0x kainino0x merged commit 823a357 into gpuweb:main Oct 11, 2022
@kainino0x kainino0x removed the tacit resolution queue Editors have agreed and intend to land if no feedback is given label Oct 13, 2022
@kdashg
Copy link
Contributor

kdashg commented Oct 14, 2022

GPU Web Meeting 2022-10-12/13 APAC-timed
  • workgroupStorageUsed is sum of values, each rounded up to multiple of 16 #3522
    • Handles the fact that padding between variables in workgroup storage class is implementation-defined. FYI only - already approved by WGSL group.
    • Backends don't define how they pack variables into workgroup memory, so this is a conservative estimate of how much space we'll use.
  • MM: if workgroup variable is a struct, will padding be involved?
  • KN: reason for picking 16 - it's the largest natural alignment you'd need.
  • MM: so struct containing 4 floats - 16 bytes, not 128?
  • KN: yes.
  • KN: this was discussed by folks who understand Vulkan's constraints.

@lokokung
Copy link
Contributor

lokokung commented Dec 3, 2022

Removing 'needs-cts-issue' with filing of CTS re-review draft issue named: val: compute_pipeline:limits,workgroup_storage_size

@lokokung lokokung removed the needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet label Dec 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kainino0x kainino0x kainino0x approved these changes

@toji toji Awaiting requested review from toji

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Milestone 0
Development

Successfully merging this pull request may close these issues.

Validating maxComputeWorkgroupStorageSize
4 participants
@dneto0 @kdashg @lokokung @kainino0x