Validate Limits::max_compute_workgroup_storage_size#9732
Validate Limits::max_compute_workgroup_storage_size#9732ErichDonGubler wants to merge 3 commits into
Limits::max_compute_workgroup_storage_size#9732Conversation
ErichDonGubler
added
type: bug
| let check_workgroup_sizes = |sizes, | ||
| per_dimension_limits, | ||
| per_dimension_limits_desc, | ||
| total_limit, | ||
| total_limit_desc| { | ||
| let workgroup_storage_used = entry_point.workgroup_storage_size.next_multiple_of(16); | ||
| if workgroup_storage_used > self.limits.max_compute_workgroup_storage_size { | ||
| return Err(StageError::WorkgroupStorageSizeLimitExceeded { | ||
| used: workgroup_storage_used, | ||
| limit: self.limits.max_compute_workgroup_storage_size, | ||
| limit_desc: "max_compute_workgroup_storage_size", | ||
| }); | ||
| } | ||
|
|
||
| check_workgroup_sizes( | ||
| sizes, | ||
| per_dimension_limits, | ||
| per_dimension_limits_desc, | ||
| total_limit, | ||
| total_limit_desc, | ||
| ) | ||
| .map_err(Into::into) | ||
| }; | ||
|
|
There was a problem hiding this comment.
note: This particular hunk changes stylistically with merge conflict resolution against #9733. I recommend taking a look at that, too, if you're reviewing this, just to see if you agree with the taste.
ErichDonGubler
force-pushed
the
validate-workgroup_storage_size_limit_exceeded
branch
2 times, most recently
from
c97e9eb to
65e5d81
Compare
|
Hmm…I haven't figured out why validation is incorrect yet, but that's what's tripping up CI. We appear to be failing Weirdly, |
ErichDonGubler
force-pushed
the
validate-workgroup_storage_size_limit_exceeded
branch
from
65e5d81 to
1e017ee
Compare
|
I'd like to take a look at this at some point for mesh/task shader purposes, if I don't take too long can this be blocked on my review? |
If you think it'll take no more than a day or so, that sounds fine. Otherwise, I think it'd be better to just not validate for task and mesh shader stages and iterate in a separate PR, since the impetus here is to become compliant with the WebGPU spec. (and those aren't needed for it). |
inner-daemons
left a comment
inner-daemons
left a comment
There was a problem hiding this comment.
LGTM, at least from the perspective of mesh/task shaders
2 participants
Connections
Implements the last bit of scope for #8946.
Conflicts with #9733. I have a resolution ready in trunk...erichdongubler-mozilla:wgpu:erichdongubler-push-uovkovyvxuzq to handle this.
Testing
Enable CTS tests.
Squash or Rebase? squashplz
Checklist
wgpumayshould be affected behaviorally.CHANGELOG.mdentries for the user-facing effects of this change are present.