Skip to content
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.

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

Correct expectation for shift equal to bit width #3868

Merged
merged 2 commits into from
Jul 18, 2024

Conversation

sudonatalie
Copy link
Contributor

@sudonatalie sudonatalie commented Jul 18, 2024

It is a shader/pipeline-creation error if the number of bits to shift is greater than or equal to the bit width of the value to shift. This change corrects the expectations for the equals case, and adds a function reference to the pipeline subcase so it isn't stripped out.

These tests will pass in Dawn following: https://dawn-review.googlesource.com/c/dawn/+/199075

Requirements for PR author:

  • All missing test coverage is tracked with "TODO" or .unimplemented().
  • New helpers are /** documented */ and new helper files are found in helper_index.txt.
  • Test behaves as expected in a WebGPU implementation. (If not passing, explain above.)
  • Test have be tested with compatibility mode validation enabled and behave as expected. (If not passing, explain above.)

Requirements for reviewer sign-off:

  • Tests are properly located in the test tree.
  • Test descriptions allow a reader to "read only the test plans and evaluate coverage completeness", and accurately reflect the test code.
  • Tests provide complete coverage (including validation control cases). Missing coverage MUST be covered by TODOs.
  • Helpers and types promote readability and maintainability.

When landing this PR, be sure to make any necessary issue status updates.

It is a shader/pipeline-creation error if the number of bits to shift is
greater than *or equal to* the bit width of the value to shift.
@sudonatalie sudonatalie requested a review from zoddicus July 18, 2024 15:41
@sudonatalie sudonatalie merged commit c2c2fa9 into gpuweb:main Jul 18, 2024
1 check passed
@sudonatalie sudonatalie deleted the bit-shift-partial-eval branch July 18, 2024 16:26
copybara-service bot pushed a commit to google/dawn that referenced this pull request Jul 18, 2024
This validation already existed when the whole shift expression could be const evaluated, but it also applies when only the rhs is const, and the
lhs is a concrete type. This change moves that validation into the
resolver to catch more cases. Other cases like divide by zero should
also be covered by Validator::BinaryExpression in future.

Related CTS fixes in: gpuweb/cts#3868

Bug: 352768986
Change-Id: I958949dfab2e43670fa358dd11563c5bba40ce20
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/199075
Reviewed-by: James Price <[email protected]>
Commit-Queue: Natalie Chouinard <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants