Add `[T]::as_simd(_mut)`#91479

scottmcm · 2021-12-03T02:15:19Z

SIMD-style optimizations are the most common use for [T]::align_to(_mut), but that's unsafe. So these are safe wrappers around it, now that we have the Simd type available, to make it easier to use.

impl [T] {
    pub fn as_simd<const LANES: usize>(&self) -> (&[T], &[Simd<T, LANES>], &[T]);
    pub fn as_simd_mut<const LANES: usize>(&mut self) -> (&mut [T], &mut [Simd<T, LANES>], &mut [T]);
}

They're cfg'd out for miri because the simd module as a whole is unavailable there.

SIMD-style optimizations are the most common use for `[T]::align_to(_mut)`, but that's `unsafe`. So these are *safe* wrappers around it, now that we have the `Simd` type available, to make it easier to use. ```rust impl [T] { pub fn as_simd<const LANES: usize>(&self) -> (&[T], &[Simd<T, LANES>], &[T]); pub fn as_simd_mut<const LANES: usize>(&mut self) -> (&mut [T], &mut [Simd<T, LANES>], &mut [T]); } ```

rust-highfive · 2021-12-03T02:15:22Z

r? @yaahc

(rust-highfive has picked a reviewer for you, use r? to override)

scottmcm · 2021-12-03T02:20:36Z

cc @workingjubilee in case this is a terrible idea for some reason

workingjubilee

This looks fine, modulo the question it raises and the nits.

workingjubilee · 2021-12-03T23:39:43Z

library/core/src/slice/mod.rs

        }
    }

+    /// Split a slice into a prefix, a middle of aligned simd types, and a suffix.


Suggested change

/// Split a slice into a prefix, a middle of aligned simd types, and a suffix.

/// Split a slice into a prefix, a middle of aligned SIMD types, and a suffix.

workingjubilee · 2021-12-03T23:41:13Z

library/core/src/slice/mod.rs

+    /// This is a safe wrapper around [`slice::align_to`], so has the same weak
+    /// preconditions as that method.  Notably, you must not assume any particular
+    /// split between the three parts: it's legal for the middle slice to be
+    /// empty even if the input slice is longer than `3 * LANES`.


Since this wrapper is safe, this should clarify that this is an invalid assumption for unsafe code, as the function can no longer be relied upon to maintain invariants in the same way. Safe code trying to "rely" on it would be "merely" logically incorrect.

Oh, I wrote "preconditions" here when I meant "postconditions" 🤦

workingjubilee · 2021-12-03T23:44:16Z

library/core/src/slice/mod.rs

+        unsafe { self.align_to() }
+    }
+
+    /// Split a slice into a prefix, a middle of aligned simd types, and a suffix.


Suggested change

/// Split a slice into a prefix, a middle of aligned simd types, and a suffix.

/// Split a slice into a prefix, a middle of aligned SIMD types, and a suffix.

workingjubilee · 2021-12-03T23:49:59Z

library/core/src/slice/mod.rs

+        // SAFETY: The simd types have the same layout as arrays, just with
+        // potentially-higher alignment, so the de-facto transmutes are sound.
+        unsafe { self.align_to() }


Sound, yes, but it would likely exhibit unexpected behavior if LANES is an odd number (like 3), as that would potentially result in bytes no longer being read (because the stride of the type will be as if LANES is 4). This is not supported yet, but we have not yet ruled this possibility out. This function therefore introduces an unanswered question. Should we:

allow people to transmute data into padding

add additional bounds on this if we relax those on LANES

rule out types like Simd<f32, 3> entirely?

Well I guess my assert_eq on the sizes (line 3487) is more useful than I'd thought, as if 3-simd is padded out to 4-simd it'd make the function just panic instead of skipping things.

The other possibility would be to have it just return everything in the prefix for that case, since align_to is allowed to do that anyway (for MIRI), and it'd just be slower than desired if someone uses the non-power-of-two sizes.

I didn't consider that, but it is indeed also an option!
This is definitely a bridge we can cross when we actually come to it, I just wanted to note the bridge is indeed up ahead.

yaahc · 2021-12-10T20:00:21Z

lgtm once all the comments are resolved

scottmcm · 2021-12-14T23:51:15Z

@workingjubilee I've pushed updates; please take a look and see if they satisfactorily address your comments.

And since it's LGTM from yaahc,
@bors delegate=workingjubilee

bors · 2021-12-14T23:51:17Z

✌️ @workingjubilee can now approve this pull request

workingjubilee · 2021-12-15T00:38:34Z

Yeah, this looks fine!
@bors r+ rollup=always

bors · 2021-12-15T00:38:36Z

📌 Commit e4c44c5 has been approved by workingjubilee

…bilee Add `[T]::as_simd(_mut)` SIMD-style optimizations are the most common use for `[T]::align_to(_mut)`, but that's `unsafe`. So these are *safe* wrappers around it, now that we have the `Simd` type available, to make it easier to use. ```rust impl [T] { pub fn as_simd<const LANES: usize>(&self) -> (&[T], &[Simd<T, LANES>], &[T]); pub fn as_simd_mut<const LANES: usize>(&mut self) -> (&mut [T], &mut [Simd<T, LANES>], &mut [T]); } ``` They're `cfg`'d out for miri because the `simd` module as a whole is unavailable there.

…askrgr Rollup of 7 pull requests Successful merges: - rust-lang#90521 (Stabilize `destructuring_assignment`) - rust-lang#91479 (Add `[T]::as_simd(_mut)`) - rust-lang#91584 (Improve code for rustdoc-gui tester) - rust-lang#91886 (core: minor `Option` doc correction) - rust-lang#91888 (Handle unordered const/ty generics for object lifetime defaults) - rust-lang#91905 (Fix source code page sidebar on mobile) - rust-lang#91906 (Removed `in_band_lifetimes` from `library\proc_macro`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

RalfJung · 2022-03-03T17:41:01Z

They're cfg'd out for miri because the simd module as a whole is unavailable there.

The SIMD module is only commented out for doctests, but otherwise available... but I guess that is enough to require these extra cfg here. Which is a shame because I only wanted to disable the tests in Miri, not actually hide any APIs. If there are better ways to disable doctests in a part of a crate, I'd be interested to learn about them. :)

rust-highfive assigned yaahc Dec 3, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 3, 2021

workingjubilee reviewed Dec 3, 2021

View reviewed changes

apiraino added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Dec 9, 2021

dtolnay added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 10, 2021

yaahc added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 10, 2021

Update comments per review feedback

e4c44c5

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 15, 2021

matthiaskrgr mentioned this pull request Dec 15, 2021

Rollup of 7 pull requests #91952

Closed

matthiaskrgr mentioned this pull request Dec 15, 2021

Rollup of 7 pull requests #91959

Merged

bors merged commit efc49c1 into rust-lang:master Dec 15, 2021

rustbot added this to the 1.59.0 milestone Dec 15, 2021

scottmcm deleted the slice-as-simd branch December 15, 2021 18:03

workingjubilee mentioned this pull request Dec 15, 2021

Tracking Issue for RFC 2948: Portable SIMD #86656

Open

9 tasks

ericseppanen mentioned this pull request Jun 8, 2022

Use Python-Markdown 3.3.7 during site deployment rust-lang/this-week-in-rust#3338

Closed

	/// Split a slice into a prefix, a middle of aligned simd types, and a suffix.
	/// Split a slice into a prefix, a middle of aligned SIMD types, and a suffix.

Uh oh!

Conversation

scottmcm commented Dec 3, 2021

Uh oh!

rust-highfive commented Dec 3, 2021

Uh oh!

scottmcm commented Dec 3, 2021

Uh oh!

workingjubilee left a comment

Choose a reason for hiding this comment

Uh oh!

workingjubilee Dec 3, 2021

Choose a reason for hiding this comment

Uh oh!

workingjubilee Dec 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scottmcm Dec 4, 2021

Choose a reason for hiding this comment

Uh oh!

workingjubilee Dec 3, 2021

Choose a reason for hiding this comment

Uh oh!

workingjubilee Dec 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scottmcm Dec 4, 2021

Choose a reason for hiding this comment

Uh oh!

workingjubilee Dec 5, 2021

Choose a reason for hiding this comment

Uh oh!

yaahc commented Dec 10, 2021

Uh oh!

scottmcm commented Dec 14, 2021

Uh oh!

bors commented Dec 14, 2021

Uh oh!

workingjubilee commented Dec 15, 2021

Uh oh!

bors commented Dec 15, 2021

Uh oh!

RalfJung commented Mar 3, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

workingjubilee Dec 3, 2021 •

edited

Loading

workingjubilee Dec 3, 2021 •

edited

Loading