Stabilize Wasm relaxed SIMD by daxpedda · Pull Request #117468 · rust-lang/rust

daxpedda · 2023-10-31T22:36:50Z

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.

rustbot · 2023-10-31T22:36:57Z

r? @wesleywiser

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2023-10-31T22:36:58Z

⚠️ Warning ⚠️

These commits modify submodules.

daxpedda · 2023-10-31T22:37:11Z

Cc @alexcrichton.

daxpedda · 2023-10-31T22:48:13Z

This won't currently compile until rust-lang/stdarch#1494 is merged and updated, so for now this is waiting for the FCP first.

alexcrichton · 2023-11-01T23:16:47Z

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.

daxpedda · 2023-11-02T09:20:53Z

@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.
It could be argued that this is blocked on #117372, which in turn is blocked by tkaitchuck/aHash#183, but that should probably not block an FCP.

So I would like to request an FCP for stabilizing relaxed SIMD!
Let me know if I misunderstood something.

Amanieu · 2023-11-02T19:13:10Z

This cover the relaxed-simd target feature (lang team) and the related intrinsics in stdarch (libs-api team).

@rfcbot fcp merge

rfcbot · 2023-11-02T19:13:12Z

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.
See this document for info about what commands tagged team members can give me.

RalfJung · 2023-11-02T19:30:19Z

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:

The same instruction at (1) and (2), when given the same inputs, can return two different results.

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.

Some operators are host-dependent, because the set of possible results may depend on properties of the host environment (such as hardware). Technically, each such operator produces a fixed-size list of sets of allowed values. For each execution of the operator in the same environment, only values from the set at the same position in the list are returned, i.e., each environment globally chooses a fixed projection for each operator.

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?

alexcrichton · 2023-11-02T19:41:37Z

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 fpenv idea which was later removed. While I can't speak with complete certainty I'm pretty confident you've discovered a mistake in the overview. This could be confirmed with the rendered specification but that is a broken link at this time. I'm not great at reading the raw spec myself.

RalfJung · 2023-11-04T10:59:09Z

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?

daxpedda · 2023-11-04T11:04:29Z

I don't think that should cause any issues?

No, we do nothing with the output (the part that might not be deterministic).

These are just LLVM intrinsics implemented via certain wasm instructions that, right?

Indeed.

bors · 2023-11-15T01:50:14Z

☔ The latest upstream changes (presumably #117915) made this pull request unmergeable. Please resolve the merge conflicts.

daxpedda · 2024-07-31T13:11:59Z

I also think this should be removed. This test was used to ensure that reduce_add doesn't ub. The current implementation is simple, and the associativity is enforced in the code itself.

Thank you both, I've done just that!

@wesleywiser this PR is ready to be merged again.

alexcrichton

I talked with @daxpedda about this and I believe I can approve PRs now so I'll try to take over this since it's wasm-specific. This looks good to me and while there's bits to sort out in relation to #128221 that seems ok to defer for a bit while that waits to land too. I've got one request here to expand a comment but otherwise I'm happy to send this back to bors

alexcrichton · 2024-07-31T17:42:50Z

compiler/rustc_codegen_llvm/src/llvm_util.rs

+    // `+relaxed-simd` implies `+simd128`
+    if features.iter().any(|f| f == "+relaxed-simd") && !features.iter().any(|f| f == "+simd128") {
+        features.push("+simd128".into());
+    }


Mind expanding this comment a bit? I think it'd be good to mention here:

This code is wasm-specific

Why this code is here and maybe a small snippet about how it relates to per-function features

Maybe a quick mention of why this isn't generalized?

And then do you think it's worth adding a gate around this checking if the LLVM version is <= 19? That could link to the PR which changed this in upstream LLVM as well.

Done! I hope its clear.
I also went ahead and renamed the whole system from "dependant" to "implicit", which is much more appropriate.

And then do you think it's worth adding a gate around this checking if the LLVM version is <= 19? That could link to the PR which changed this in upstream LLVM as well.

I will keep an eye on it and make a PR as soon as it actually lands.

alexcrichton · 2024-07-31T18:02:23Z

@bors: r+

bors · 2024-07-31T18:02:26Z

📌 Commit 61d95e2 has been approved by alexcrichton

It is now in the queue for this repository.

…crichton Stabilize Wasm relaxed SIMD This PR stabilizes [Wasm relaxed SIMD](https://github.com/WebAssembly/relaxed-simd) which has already reached [phase 4](https://github.com/WebAssembly/proposals/tree/04fa8c810e1dc99ab399e41052a6e427ee988180?tab=readme-ov-file#phase-4---standardize-the-feature-wg). Tracking issue: rust-lang#111196 Implementation PR: rust-lang/stdarch#1393 Documentation: rust-lang/reference#1421 Stdarch: rust-lang/stdarch#1494 Closes rust-lang#111196.

Rollup of 7 pull requests Successful merges: - rust-lang#117468 (Stabilize Wasm relaxed SIMD) - rust-lang#123813 (Add `REDUNDANT_IMPORTS` lint for new redundant import detection) - rust-lang#127060 (Migrate `symbol-visibility` `run-make` test to rmake) - rust-lang#127159 (match lowering: Hide `Candidate` from outside the lowering algorithm) - rust-lang#128296 (Update target-spec metadata for loongarch64 targets) - rust-lang#128416 (android: Remove libstd hacks for unsupported Android APIs) - rust-lang#128431 (Add myself as VxWorks target maintainer for reference) r? `@ghost` `@rustbot` modify labels: rollup

tgross35 · 2024-07-31T21:32:04Z

It looks like this may have caused the failure here #128454 (comment). Is it possible to update the stdarch submodule separately from stabilizing the wasm feature? That might be better in its own PR (and at least its own commit, for better history).

@bors r-

daxpedda · 2024-07-31T21:42:13Z

Sure!
Will do that as soon as I figure out what the problem is exactly.

daxpedda · 2024-07-31T21:54:30Z

@Amanieu these errors look like AVX512 work related to the Stdarch update, would you know whats causing this or who to ping?

tgross35 · 2024-07-31T22:00:27Z

It could also just be the RFL job having something wrong with target features, I brought it up on Zulip too.

daxpedda · 2024-08-01T19:38:42Z

Stdarch update is happening in #128466, which this PR will be rebased on after its merged.

tgross35 · 2024-08-04T04:41:57Z

#128466 (comment) merged so this should be ready for a rebase.

alexcrichton · 2024-08-04T13:38:11Z

@bors: r+

bors · 2024-08-04T13:38:14Z

📌 Commit 80b74d3 has been approved by alexcrichton

It is now in the queue for this repository.

bors · 2024-08-05T09:25:54Z

⌛ Testing commit 80b74d3 with merge 9179d9b...

bors · 2024-08-05T12:09:51Z

☀️ Test successful - checks-actions
Approved by: alexcrichton
Pushing 9179d9b to master...

rust-timer · 2024-08-05T13:27:49Z

Finished benchmarking commit (9179d9b): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 7.4%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	7.4%	[7.4%, 7.4%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	7.4%	[7.4%, 7.4%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 757.802s -> 757.898s (0.01%)
Artifact size: 336.77 MiB -> 336.81 MiB (0.01%)

rustbot assigned wesleywiser Oct 31, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Oct 31, 2023

daxpedda mentioned this pull request Oct 31, 2023

Stabilize Wasm relaxed SIMD rust-lang/stdarch#1494

Merged

This comment has been minimized.

Sign in to view

daxpedda force-pushed the wasm-relaxed-simd branch from 68a0b5d to b6847a7 Compare October 31, 2023 22:57

This comment has been minimized.

Sign in to view

This was referenced Oct 31, 2023

Stabilize Wasm target features that are in phase 4 and 5 #117457

Merged

Stabilize Wasm relaxed SIMD rust-lang/reference#1421

Merged

wesleywiser added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Nov 1, 2023

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Nov 2, 2023

daxpedda force-pushed the wasm-relaxed-simd branch from 73fa528 to f06dd33 Compare July 31, 2024 11:44

This comment has been minimized.

Sign in to view

daxpedda force-pushed the wasm-relaxed-simd branch from f06dd33 to d0e9a84 Compare July 31, 2024 11:51

daxpedda force-pushed the wasm-relaxed-simd branch from d0e9a84 to e0c838c Compare July 31, 2024 16:54

alexcrichton reviewed Jul 31, 2024

View reviewed changes

daxpedda force-pushed the wasm-relaxed-simd branch 2 times, most recently from 4d0af88 to 61d95e2 Compare July 31, 2024 17:58

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 31, 2024

nikic mentioned this pull request Jul 31, 2024

PowerPC test failures due to "similarly named associated type" #126260

Closed

tgross35 mentioned this pull request Jul 31, 2024

Rollup of 7 pull requests #128454

Closed

tgross35 mentioned this pull request Aug 1, 2024

Update the stdarch submodule #128466

Merged

daxpedda added 2 commits August 4, 2024 08:44

Stabilize Wasm relaxed SIMD

9052139

Implement a implicit target feature mechanism

80b74d3

yisibl mentioned this pull request Jan 27, 2026

feat: enable SIMD thx/resvg-js#148

Draft

Uh oh!

Conversation

daxpedda commented Oct 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Oct 31, 2023

Uh oh!

rustbot commented Oct 31, 2023

Uh oh!

daxpedda commented Oct 31, 2023

Uh oh!

This comment has been minimized.

daxpedda commented Oct 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

alexcrichton commented Nov 1, 2023

Uh oh!

daxpedda commented Nov 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Amanieu commented Nov 2, 2023

Uh oh!

rfcbot commented Nov 2, 2023 • edited by joshtriplett Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RalfJung commented Nov 2, 2023

Uh oh!

alexcrichton commented Nov 2, 2023

Uh oh!

RalfJung commented Nov 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daxpedda commented Nov 4, 2023

Uh oh!

bors commented Nov 15, 2023

Uh oh!

This comment has been minimized.

daxpedda commented Jul 31, 2024

Uh oh!

alexcrichton left a comment

Choose a reason for hiding this comment

Uh oh!

alexcrichton Jul 31, 2024

Choose a reason for hiding this comment

Uh oh!

daxpedda Jul 31, 2024

Choose a reason for hiding this comment

Uh oh!

alexcrichton commented Jul 31, 2024

Uh oh!

bors commented Jul 31, 2024

Uh oh!

tgross35 commented Jul 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daxpedda commented Jul 31, 2024

Uh oh!

daxpedda commented Jul 31, 2024

Uh oh!

tgross35 commented Jul 31, 2024

Uh oh!

daxpedda commented Aug 1, 2024

Uh oh!

tgross35 commented Aug 4, 2024

Uh oh!

alexcrichton commented Aug 4, 2024

Uh oh!

bors commented Aug 4, 2024

Uh oh!

bors commented Aug 5, 2024

Uh oh!

bors commented Aug 5, 2024

Uh oh!

rust-timer commented Aug 5, 2024

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

daxpedda commented Oct 31, 2023 •

edited

Loading

daxpedda commented Oct 31, 2023 •

edited

Loading

daxpedda commented Nov 2, 2023 •

edited

Loading

rfcbot commented Nov 2, 2023 •

edited by joshtriplett

Loading

RalfJung commented Nov 4, 2023 •

edited

Loading

tgross35 commented Jul 31, 2024 •

edited

Loading