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

[naga] change i32 arithmetic operations to use wrapping_ instead of checked_ #6835

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from

Conversation

matthew-wong1
Copy link
Contributor

@matthew-wong1 matthew-wong1 commented Dec 29, 2024

Connections
Addresses #6023

Description
For arithmetic operations between two variables of type i32, if an overflow occurs, it is meant to wrap instead of throw an error according to the WGSL spec ("Expressions on concrete integer types that overflow produce a result that is modulo 2bitwidth")

Testing
Compiled the original program detailed in #6023 (and variations for other arithemtic operations) that threw the overflow error (now overflow error free)

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@matthew-wong1 matthew-wong1 marked this pull request as ready for review December 29, 2024 17:14
@matthew-wong1 matthew-wong1 requested a review from a team as a code owner December 29, 2024 17:14
@matthew-wong1 matthew-wong1 changed the title [naga] change i32 multiplication to use wrapping_mul instead of checked_mul [naga] change i32 arithmetic operations to use wrapping_ instead of checked_ Dec 30, 2024
@matthew-wong1 matthew-wong1 force-pushed the update-i32-multiplication-to-wrap branch from fec4364 to b2dca02 Compare December 30, 2024 07:45
@sagudev
Copy link
Contributor

sagudev commented Dec 30, 2024

I will do CTS run in servo for this, but it will take some time as I need to set baseline expectations first.

sagudev added a commit to sagudev/servo that referenced this pull request Dec 30, 2024
Signed-off-by: sagudev <[email protected]>
sagudev added a commit to sagudev/servo that referenced this pull request Dec 30, 2024
{"fail_fast": false, "matrix": [{"name": "WebGPU CTS", "workflow": "linux", "wpt_layout": "2020", "profile": "production", "unit_tests": false, "bencher": false, "wpt_args": "_webgpu"}]}
@sagudev
Copy link
Contributor

sagudev commented Dec 30, 2024

CTS run looks good: https://github.com/sagudev/servo/actions/runs/12544331341/attempts/1#summary-34979468146

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