Skip to content

Conversation

@LaurenzV
Copy link
Collaborator

@LaurenzV LaurenzV commented Jan 3, 2026

As per title, by having this, we don't have to store them back using copy_from_slice (which may or may not optimize into vector store instructions) but can use methods that use the corresponding intrinsics under the hood.

@LaurenzV LaurenzV requested review from Ralith and valadaptive January 3, 2026 09:29
@LaurenzV
Copy link
Collaborator Author

LaurenzV commented Jan 3, 2026

(Will add changeling entry upon approval.)

@LaurenzV LaurenzV changed the title Add store methods to use intrinsics for storing vectors back to memory Add methods for storing vectors back to memory Jan 3, 2026
Copy link
Contributor

@valadaptive valadaptive left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! Just a few minor code style nits.

/// `[f32; 4]` for `f32x4<S>`) or a reference to it.
AsArray { kind: RefKind },
/// Takes a vector and a mutable reference to an array, and stores the vector elements into the array.
ToArray,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should be renamed StoreArray?

@LaurenzV
Copy link
Collaborator Author

Thanks! By the way, #171 has also been approved in case you haven't seen it. I could probably take another look at #164 if you want.

@LaurenzV LaurenzV added this pull request to the merge queue Jan 18, 2026
Merged via the queue into linebender:main with commit 4d915a4 Jan 18, 2026
18 checks passed
@LaurenzV LaurenzV deleted the store branch January 18, 2026 14:41
@Shnatsel
Copy link
Contributor

Shnatsel commented Jan 21, 2026

Note that storing to fixed-size slices involves a massive footgun: https://github.com/okaneco/safe_unaligned_simd?tab=readme-ov-file#a-note-on-creating-mutable-array-references-from-slices

This isn't a problem for loads, only for stores. I've been pulling my hair for an hour debugging something that absolutely should work but didn't because of this footgun, and that's how it came to be documented in that repo's README.

The best way to avoid it that I can think of is to make the output slice a &mut [T] as opposed to &mut [T; N] and include an assert! checking the length with >= in each function. All of this is #[inline(always)] so if the length is known to be sufficient the check will be eliminated.

@valadaptive
Copy link
Contributor

Note that storing to fixed-size slices involves a massive footgun

If you use the store_slice method, you should not be subject to this footgun.

The only way to fall victim to this footgun in fearless_simd is if you use the store_array_[whatever] methods directly, like vector.witness().store_array_f32x4(vector, &mut chunk[..4].try_into().unwrap()).

@Shnatsel
Copy link
Contributor

I didn't realize that was an option. If there's already a store_slice, do we even need the footgunny store_array_blah() functions?

I think they should either be removed, or at the very least footgun should be mentioned in their doc comment.

fn from_slice(simd: S, slice: &[Self::Element]) -> Self;
#[doc = r" Store a SIMD vector into a slice."]
#[doc = r""]
#[doc = r" The slice must be the proper width."]

This comment was marked as resolved.

@LaurenzV
Copy link
Collaborator Author

I think at least internally we still need the methods... And making it private would be a bit of a pain I think.

github-merge-queue bot pushed a commit that referenced this pull request Jan 21, 2026
@Shnatsel
Copy link
Contributor

Well, if they can't be made pub(crate) then I think it's worth at least documenting the footgun if we can't prevent it

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.

3 participants