Implement traits over scalars with self: Simd<T, N>#493
Conversation
| /// The mask type returned by each comparison. | ||
| type Mask; | ||
|
|
||
| pub trait SimdPartialEq<T, const N: usize> |
There was a problem hiding this comment.
imo N should be an associated #[type_const] but last I knew that's not done being implemented yet.
something like:
pub trait SimdLen {
#[type_const]
const LEN: usize;
}
impl<T: SimdElement, const N: usize> SimdLen for Simd<T, N> {
#[type_const]
const LEN: usize = N;
}
impl<T: MaskElement, const N: usize> SimdLen for Mask<T, N> {
#[type_const]
const LEN: usize = N;
}
pub trait SimdBase: SimdLen {
type Element: SimdElement;
}
impl<T: SimdElement, const N: usize> SimdBase for Simd<T, N> {
type Element = T;
}
pub trait SimdPartialEq: SimdBase {
fn simd_eq(self, other: Self) -> Mask<<Self::Element as SimdElement>::Mask, { Self::LEN }>;
...
}There was a problem hiding this comment.
that would allow your example function's bounds to just be:
fn foo<T: SimdElement>(x: Simd<T, 4>) -> Mask<T::Mask, 4>
where
// output types are completely deduced from the blanket
// SimdBase and SimdLen impls so no additional bounds are needed
Simd<T, 4>: SimdFloat,
{
x.is_sign_negative()
}There was a problem hiding this comment.
sadly, it seems that hits the bug where specifying bounds hides information that could be deduced so the bound ends up having to be:
Simd<T, 4>: SimdFloat<Element = T, LEN = 4>
There was a problem hiding this comment.
I got it to work with just Simd<T, 4>: SimdFloat by removing SimdFloat: SimdBase and instead having where Self: SimdBase on all items in the trait.
There was a problem hiding this comment.
further reduced feature gates: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2024&gist=b57a420a75142e3eda48993dc2f981f1
There was a problem hiding this comment.
if you're trying to be generic over multiple lengths, of course you need a
const N: usizegeneric. if you instead have one or more known lengths that you're not generic over, I intended that you shouldn't need bounds mentioning generic lengths.
imo Simd<T, N> ideally shouldn't need more or less trait bounds for lengths than [T; N], so if you're trying to be generic over length having fn f<const N: usize>(v: Simd<f32, N>) seems good to me. imo you shouldn't need to mention known or generic lengths anywhere other than the N in Simd<T, N> (or Mask), so I don't like having to have Simd<T, N>: SomeTrait<N> + OtherTrait<N> where you have to repeat the N in all the trait bounds.
There was a problem hiding this comment.
if you're trying to be generic over multiple lengths, of course you need a
const N: usizegeneric. if you instead have one or more known lengths that you're not generic over, I intended that you shouldn't need bounds mentioning generic lengths.imo
Simd<T, N>ideally shouldn't need more or less trait bounds for lengths than[T; N], so if you're trying to be generic over length havingfn f<const N: usize>(v: Simd<f32, N>)seems good to me. imo you shouldn't need to mention known or generic lengths anywhere other than theNinSimd<T, N>(orMask), so I don't like having to haveSimd<T, N>: SomeTrait<N> + OtherTrait<N>where you have to repeat theNin all the trait bounds.
Yea but an associated #[type_const] would allow such a function to just be fn f<T: Simd<f32>>(v: T).
--Edit
@programmerjake Also if you need constraint the length you could do something like:
fn f<T: Simd<f32, N = 4>>(v: T) ect..
That number trait I have allows something similar, I guess the main thing I don't like about it is that it makes my arrays opaque to just what trait constraint I put on it. Although the commented out ArrayLike trait allowed me to get around most the issues of that.
There was a problem hiding this comment.
Yea but an associated
#[type_const]would allow such a function to just befn f<T: Simd<f32>>(v: T).
Yes (if you used an actual trait rather than the Simd type). That said, I think using the Simd<f32, N> type is waay better, similar to how it's standard practice to write fn f<const N: usize>(v: [f32; N]) instead of fn f<T: ArrayOf<f32>>(v: T).
-Also if you need constraint the length you could do something like:
fn f<T: Simd<f32, N = 4>>(v: T)ect..
if you already know both the length and element type then there's only one Simd type that matches, using a generic is just extra complexity for no reason, just use Simd<f32, 4> or the f32x4 type alias. this is similar to how you'd write [f32; 4] instead of T: Array<f32, 4>
There was a problem hiding this comment.
I guess that depends on what your doing/preference, I am just saying #[type_const] can allow for less verbose signatures.
There was a problem hiding this comment.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
after thinking some more, I think I found the best API so far: https://rust-lang.zulipchat.com/#narrow/channel/257879-project-portable-simd/topic/Simd.3CT.2C.20N.3E.3A.20Receiver.3CTarget.20.3D.20T.3E/with/593431978 |
self: Simd<T, N>
8f432a0 to
f935cb3
Compare
This comment has been minimized.
This comment has been minimized.
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
There was a problem hiding this comment.
generally looks good to me, though we'll want to wait on rust-lang/rust#156270 before merging.
| fn copysign<const N: usize>(self: Simd<Self, N>, sign: Simd<Self, N>) -> Simd<Self, N> { | ||
| let sign_bit = sign.to_bits() & Simd::splat(-0. as $ty).to_bits(); | ||
| let magnitude = self.to_bits() & !Simd::splat(-0. as $ty).to_bits(); | ||
| <Self as SimdFloat>::simd_from_bits(sign_bit | magnitude) |
There was a problem hiding this comment.
| <Self as SimdFloat>::simd_from_bits(sign_bit | magnitude) | |
| Self::simd_from_bits(sign_bit | magnitude) |
now that it's renamed, the above should work.
This PR has been overhauled a little bit because we decided to try using
self: Simd<T, N>.Original comments:
This improves type inference by removing most associated types. For example, the following function doesn't actually work:
You actually need the constraint
Simd<T, 4>: SimdFloat<Mask = Mask<T::Mask, 4>>.With this PR, the function should look a little more like:
There are still a few instances that need associated types, but we can explore those individually to see if there's a better API.
Also, note that we now end up with a lot of instances of
Mask<<T as SimdElement>::Mask, N>. We can either remove theSimdElement::Maskassociated type with #483 or we can provide an alias to avoid writing that out.