Skip to content

Introduce semantic conversion traits #31

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Introduce semantic conversion traits #31

wants to merge 1 commit into from

Conversation

Ralith
Copy link
Contributor

@Ralith Ralith commented Jul 10, 2025

This proposes a generic API for conversions between vectors of the same element size and count, usable on generic native-width vectors. If I get positive feedback I'll fill out some more cases before merging.

@Ralith Ralith force-pushed the push-wxrnnqzyxynn branch 3 times, most recently from 1b0786f to 28aba20 Compare July 10, 2025 01:38
@Ralith
Copy link
Contributor Author

Ralith commented Jul 10, 2025 •
edited
Loading

Open question: Should this supplant Simd::cvt_*? If not, it could probably be blanket-implemented based on those methods rather than separately implemented for each backend as in this PR. I do feel like the naming of those methods is a bit confusing.

@Ralith Ralith force-pushed the push-wxrnnqzyxynn branch from 28aba20 to cbafbac Compare July 10, 2025 03:15
@ajakubowicz-canva
Copy link
Collaborator

Discussion about this proposal: https://xi.zulipchat.com/#narrow/channel/514230-simd/topic/Ergonomic.20native-width.20conversions

@Ralith Ralith force-pushed the push-wxrnnqzyxynn branch 3 times, most recently from 3a57607 to 5b83500 Compare July 10, 2025 03:59
@Ralith Ralith changed the title Introduced SimdCast Introduce semantic conversion traits Jul 10, 2025
@Ralith Ralith force-pushed the push-wxrnnqzyxynn branch from 5b83500 to 80b5c5c Compare July 10, 2025 04:00
@Ralith
Copy link
Contributor Author

Ralith commented Jul 10, 2025 •
edited
Loading

Second draft:

  • Multiple, semantically meaningful traits instead of a single overloaded "cast"
  • Helper methods on SimdFloat and SimdInt

I don't like the trait or primitive method names much at all; please bikeshed. Especially "float" as a verb feels bad.

@Ralith Ralith force-pushed the push-wxrnnqzyxynn branch 5 times, most recently from 788569b to 39df826 Compare July 10, 2025 19:53
@Ralith Ralith marked this pull request as ready for review July 10, 2025 19:53
@Ralith
Copy link
Contributor Author

Ralith commented Jul 10, 2025

Tweaked naming, factored out common generation logic, and expanded to a superset of existing conversion operations. I think this is ready now, though still happy to brainstorm on better names.

@Ralith Ralith force-pushed the push-wxrnnqzyxynn branch from 39df826 to f9f9141 Compare July 10, 2025 19:56
Ralith
Ralith commented Jul 10, 2025
View reviewed changes
@Ralith Ralith force-pushed the push-wxrnnqzyxynn branch 6 times, most recently from 1f94ffd to 7a9bd6d Compare July 10, 2025 22:27
@Ralith
Copy link
Contributor Author

Ralith commented Jul 10, 2025

Reworked to allow conversions for e.g. f32x4<S> rather than just S::f32s, by building on top of, rather then replacing, the Simd methods.

Ralith
Ralith commented Jul 10, 2025
View reviewed changes
fearless_simd/src/generated/simd_trait.rs
@@ -667,6 +669,10 @@ pub trait SimdFloat<Element: SimdElement, S: Simd>:
+ core::ops::Div<Output = Self>
+ core::ops::Div<Element, Output = Self>
{
#[inline(always)]
fn to_int<T: SimdCvtTruncate<Self>>(self) -> T {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Naming bikeshed thread. Ideas: to_int_truncate, truncate_to_int, one of those but with trunc...

Ralith
Ralith commented Jul 10, 2025
View reviewed changes
fearless_simd/src/traits.rs

/// Construction of floating point vectors from integers
pub trait SimdCvtFloat<T> {
fn float_from(x: T) -> Self;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Naming bikeshed: from_int, convert_from_int, ...

@Ralith Ralith force-pushed the push-wxrnnqzyxynn branch 2 times, most recently from d26ca59 to eed1d41 Compare July 13, 2025 00:23
@ajakubowicz-canva
Copy link
Collaborator

ajakubowicz-canva commented Jul 18, 2025 •
edited
Loading

Sorry for the lack of communication – I plan to discuss this PR and proposal at the next Renderer office hours. Thank you for your excellent work!

ajakubowicz-canva
ajakubowicz-canva reviewed Jul 18, 2025
View reviewed changes
@Ralith Ralith force-pushed the push-wxrnnqzyxynn branch from eed1d41 to b1b3115 Compare July 18, 2025 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajakubowicz-canva ajakubowicz-canva ajakubowicz-canva left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Ralith @ajakubowicz-canva