-
Notifications
You must be signed in to change notification settings - Fork 12.1k
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
Stabilize Wasm relaxed SIMD #117468
base: master
Are you sure you want to change the base?
Stabilize Wasm relaxed SIMD #117468
Conversation
r? @wesleywiser (rustbot has picked a reviewer for you, use r? to override) |
|
Cc @alexcrichton. |
This comment has been minimized.
This comment has been minimized.
This won't currently compile until rust-lang/stdarch#1494 is merged and updated, so for now this is waiting for the FCP first. |
68a0b5d
to
b6847a7
Compare
This comment has been minimized.
This comment has been minimized.
Personally I'd say this is good to go (modulo CI of course), and it's something I forgot to do after the last CG meeting, thanks @daxpedda! For others this PR has relevant discussion namely some of the background laid you in this comment. The relaxed-simd proposal is a bit different where there's actual instructions to expose with new intrinsics, but this is following in the footsteps of the simd128 proposal and its intrinsics so AFAIK there's no new precedents set here or anything like that, mostly just following preexisting processes. |
@wesleywiser AFAIU this should probably not be marked as blocked. It's true that merging is blocked on rust-lang/stdarch#1494, but the FCP is required for rust-lang/stdarch#1494 to be merged in the first place. So I would like to request an FCP for stabilizing relaxed SIMD! |
This cover the @rfcbot fcp merge |
Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
Repeating what I posed on the tracking issue: I am confused by the spec here. Quoting from https://github.com/WebAssembly/relaxed-simd/blob/main/proposals/relaxed-simd/Overview.md:
Okay, so that's normal non-determinism then. Wasm already has that for NaNs when there are non-canonical inputs, which is strangely not acknowledged here (has that changed?), but sure, makes sense.
This describes something else! IIUC, it says that the operation, when executed twice in the same environment, must produce the same result. In other words, the non-determinism is fixed once at program startup, and the example givejn just above actually cannot happen within a single program/module execution. This is closer to what we usually call implementation-specific behavior than to full non-determinism. Can someone explain this seeming contradiction? |
To the best of my knowledge I believe that the contradiction is explained by the fact that the proposal has a long history and changed at one point. I believe your first quoted sentence is the original semantics when the proposal was first drafted and the second quote you listed is the desired semantics. For example this was the commit that introduced the first quote and it refers to an |
Okay, I've opened WebAssembly/relaxed-simd#153 to hopefully get clarification from the authors. For Rust, we should probably conservatively treat these as being non-deterministic at each operation. I don't think that should cause any issues? These are just LLVM intrinsics implemented via certain wasm instructions, right? |
No, we do nothing with the output (the part that might not be deterministic).
Indeed. |
☔ The latest upstream changes (presumably #117915) made this pull request unmergeable. Please resolve the merge conflicts. |
@wesleywiser would you mind removing the "S-blocked" label? |
d2f3e34
to
4bd0f3d
Compare
ping @T-lang for checkboxes This only needs lang review for adding a new target feature. |
☔ The latest upstream changes (presumably #121185) made this pull request unmergeable. Please resolve the merge conflicts. |
4bd0f3d
to
f5ab5e8
Compare
☔ The latest upstream changes (presumably #121549) made this pull request unmergeable. Please resolve the merge conflicts. |
f5ab5e8
to
8874ded
Compare
☔ The latest upstream changes (presumably #123065) made this pull request unmergeable. Please resolve the merge conflicts. |
8874ded
to
60f2195
Compare
I removed the dependency on rust-lang/stdarch#1494, AFAIU this is usually done in a follow-up. |
…=wesleywiser Stabilize Wasm target features that are in phase 4 and 5 This stabilizes the Wasm target features that are known to be working and in [phase 4 and 5](https://github.com/WebAssembly/proposals/tree/04fa8c810e1dc99ab399e41052a6e427ee988180). Feature stabilized: - [Non-trapping float-to-int conversions](https://github.com/WebAssembly/nontrapping-float-to-int-conversions) - [Import/Export of Mutable Globals](https://github.com/WebAssembly/mutable-global) - [Sign-extension operators](https://github.com/WebAssembly/sign-extension-ops) - [Bulk memory operations](https://github.com/WebAssembly/bulk-memory-operations) - [Extended Constant Expressions](https://github.com/WebAssembly/extended-const) Features not stabilized: - [Multi-value](https://github.com/WebAssembly/multi-value): requires rebuilding `std` rust-lang#73755. - [Reference Types](https://github.com/WebAssembly/reference-types): no point stabilizing without rust-lang#103516. - [Threads](https://github.com/webassembly/threads): requires rebuilding `std` rust-lang#77839. - [Relaxed SIMD](https://github.com/WebAssembly/relaxed-simd): separate PR rust-lang#117468. - [Multi Memory](https://github.com/WebAssembly/multi-memory): not implemented. See rust-lang#117457 (comment) for more context. Documentation: rust-lang/reference#1420 Tracking issue: rust-lang#44839
…=wesleywiser Stabilize Wasm target features that are in phase 4 and 5 This stabilizes the Wasm target features that are known to be working and in [phase 4 and 5](https://github.com/WebAssembly/proposals/tree/04fa8c810e1dc99ab399e41052a6e427ee988180). Feature stabilized: - [Non-trapping float-to-int conversions](https://github.com/WebAssembly/nontrapping-float-to-int-conversions) - [Import/Export of Mutable Globals](https://github.com/WebAssembly/mutable-global) - [Sign-extension operators](https://github.com/WebAssembly/sign-extension-ops) - [Bulk memory operations](https://github.com/WebAssembly/bulk-memory-operations) - [Extended Constant Expressions](https://github.com/WebAssembly/extended-const) Features not stabilized: - [Multi-value](https://github.com/WebAssembly/multi-value): requires rebuilding `std` rust-lang#73755. - [Reference Types](https://github.com/WebAssembly/reference-types): no point stabilizing without rust-lang#103516. - [Threads](https://github.com/webassembly/threads): requires rebuilding `std` rust-lang#77839. - [Relaxed SIMD](https://github.com/WebAssembly/relaxed-simd): separate PR rust-lang#117468. - [Multi Memory](https://github.com/WebAssembly/multi-memory): not implemented. See rust-lang#117457 (comment) for more context. Documentation: rust-lang/reference#1420 Tracking issue: rust-lang#44839
☔ The latest upstream changes (presumably #117457) made this pull request unmergeable. Please resolve the merge conflicts. |
60f2195
to
5a63c91
Compare
@joshtriplett, @nikomatsakis, @pnkfelix, @tmandry friendly ping. |
@rfcbot reviewed The PR itself is a noop:) I'm taking on faith that this lines up with the stabilized part of the specification. |
@rustbot labels +I-lang-nominated @rust-lang/lang-ops probably worth adding to agenda as a (hopefully) quick hit |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
@rfcbot reviewed |
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. This will be merged soon. |
This PR stabilizes Wasm relaxed SIMD which has already reached phase 4.
Tracking issue: #111196
Implementation PR: rust-lang/stdarch#1393
Documentation: rust-lang/reference#1421
Stdarch: rust-lang/stdarch#1494
Closes #111196.