Generic core::ops for Simd<T, _>

[workingjubilee]

In order to maintain type soundness, we need to be sure we only implement an operation for Simd<T, _> where T: SimdElement... and also valid for that operation in general. While we could do this purely parametrically, it is more sound to implement the operators directly for the base scalar type arguments and then use type parameters to extend the operators to the "higher order" operations.

This implements that strategy and cleans up simd::ops into a few submodules:

• assign.rs: core::ops::*Assign
• deref.rs: core::ops impls which "deref" borrowed versions of the arguments
• unary.rs: encloses the logic for unary operators on Simd, as unary ops are much simpler

This is possible since everything need not be nested in a single maze of macros anymore. The result simplifies the logic and allows reasoning about what operators are valid based on the expressed trait bounds, and also reduces the size of the trait implementation output in rustdoc, for a huge win of 4 MB off the size of struct.Simd.html! This addresses a common user complaint, as the original was over 5.5 MB and capable of crashing browsers!

This also carries a fix for a type-inference-related breakage, by removing the autosplatting (vector + scalar binop) impls, as unfortunately the presence of autosplatting was capable of busting type inference. We will likely need to see results from a Crater run before we can understand how to re-land autosplatting.

[workingjubilee]

We discussed this and decided

impl<T, const LANES: usize> BitAnd for Simd<T, LANES>
where
    T: SimdElement + BitAnd<Output = T>,
    LaneCount<LANES>: SupportedLaneCount,
{
    type Output = Self;

    #[inline]
    fn bitand(self, rhs: Self) -> Self::Output {
        unsafe { intrinsics::simd_and(self, rhs) }
    }
}

is a bridge too far, but

impl<T, const LANES: usize> BitAnd<T> for Simd<T, LANES>
where
    T: SimdElement + BitAnd<Output = T>,
    LaneCount<LANES>: SupportedLaneCount,
{
    type Output = Self;

    #[inline]
    fn bitand(self, rhs: T) -> Self::Output {
        unsafe { intrinsics::simd_and(self, Simd::splat(rhs)) }
    }
}

is fine, since if a vector can be added or whatever then we also want the implied promotion for binary ops.
My personal inclination is to also not implicitly promote the LHS in non-commutative ops, but everyone else is strongly in favor of RHS promotion.

[calebzulawski]

I think that second example is still making the same assumptions as the first. I suggested something more like this, since it uses the explicitly defined operation.

impl<T, const LANES: usize> BitAnd<T> for Simd<T, LANES>
where
    T: SimdElement,
    Self: BitAnd,
    LaneCount<LANES>: SupportedLaneCount,
{
    type Output = Self;

    #[inline]
    fn bitand(self, rhs: T) -> Self::Output {
        self & Simd::splat(rhs)
    }
}

This covers the same cases, but doesn't make any assumptions about what T: BitAnd might mean.

[workingjubilee]

Ah, I wasn't actually paying that close attention to the trait bounds because I am adjusting the way all of those are written to fit the new revision anyways.

[workingjubilee]

I pulled back some of the changes I had been experimenting with for later review and consideration. This version still has significant wins in terms of reducing the weight of the docs and in general making our implementation a bit cleaner.

Right now, at the tip of this branch, the LHS is not parametrically autosplatted is autosplatted by repeated scalar implementations on the scalar type argument, but the RHS uses a parametric rule. I don't actually anticipate any functional differences as a result of this, though.

[thomcc]

Hmm, I'm not in favor of the splatting this way — I think having 2.0 * vec not work, and vec * 2.0 work is going to be very surprising, and feels pretty janky. We do it for String, but there's a real performance cost there, and it's still a constant source of confusion for beginners.

[workingjubilee]

They both work, @thomcc. I simply didn't parameterize the LHS splat impls,
so there's no case where the example you cited actually happens, even if you change up the types involved.

That is, the ops are symmetric in actuality.
I intend to revisit how all of the impls work and do additional cleanup if this is merged, but there were questions about the way the LHS was parameterized that I would rather deal with separately, later, rather than having everything bikeshedded over.

[thomcc]

They both work, @thomcc. I simply didn't parameterize the LHS splat impls,

Ah, sorry! My bad, I misread the patch then.

[workingjubilee]

Mmm, that's understandable. I'll adjust the wording on that commit to reflect that.

[programmerjake]

lgtm

[workingjubilee]

All review remarks have been addressed. Specifically, the conversation about splats has been resolved by removing splats entirely, although this is primarily due to autosplatting creating unacceptable levels of type inference related breakage at the moment, per rust-lang/rust#90904. Thus, this now carries a fix for a regression currently on beta.

[calebzulawski]

lgtm, just the remaining stylistic comment which could go either way

[calebzulawski]

also, worth noting that we probably shouldn't update nightly with these changes right away, since there are some other changes on master that are somewhat half-complete