Improve Vector<uint> and Vector<ushort> multiply

[IJzerbaard]

I read on https://docs.microsoft.com/en-us/dotnet/api/system.numerics.vector-1?view=netcore-3.0 that Vector and Vector multiplications do not map to intrinsics. Experimentation confirmed that.

However, they could simply map to (v)pmulld (when available) and (v)pmullw.

Current workaround:

    static Vector<uint> Multiply(Vector<uint> a, Vector<uint> b)
    {
        return Vector.AsVectorUInt32(Vector.AsVectorInt32(a) * Vector.AsVectorInt32(b));
    }

    static Vector<ushort> Multiply(Vector<ushort> a, Vector<ushort> b)
    {
        return Vector.AsVectorUInt16(Vector.AsVectorInt16(a) * Vector.AsVectorInt16(b));
    }

byte, sbyte, long and ulong could potentially be supported with multi-operation sequences, which is at least better than calling some scalar fallback, especially for the 8bit cases.

[gfoidl]

(when available)

In that case you might also want to have a look at Hardware Intrinsics in .NET Core.

Your issue may also be tracked by https://github.com/dotnet/corefx/issues/31425

[IJzerbaard]

Of course, I prefer System.Runtime.Intrinsics.X86, but that does not take away that multiplication of Vector<ushort> is essentially broken now. I expect the fix would be about as easy as the workaround, it could even do the same thing.

[tannergooding]

@CarolEidt, @echesakovMSFT. Do you see any problems with taking a codegen improvement for these scenarios?

[CarolEidt]

No objection; I don't think it would be a huge amount of work.

[tannergooding]

No objection; I don't think it would be a huge amount of work.

I don't believe so either, so I've marked this up-for-grabs. Happy to help walk anyone through this if they want to pick it up 😄

[newuser]

Could you provide more instructions on what should be done? 1) Vector and other are marked as intrinsic and handled by codegen 2) Numerics should depend on Intrinsic x86 nugget with some architecture condition and call vector extension methods 3) it should use workaround

[tannergooding]

@newuser, the way the intrinsics are supported are currently undergoing a little bit of a rewrite here: #35421. It might be easiest to wait for that to go in before starting on the change.
Once it is in, I'll comment on here again with an explanation of the changes that would be required and will look at providing a doc for people looking at providing support in the future.

[tannergooding]

Existing intrinsics can be extended by updating the relevant table entry in:

• x86/x64: https://github.com/dotnet/runtime/blob/master/src/coreclr/src/jit/simdashwintrinsiclistxarch.h
• Arm64: https://github.com/dotnet/runtime/blob/master/src/coreclr/src/jit/simdashwintrinsiclistarm64.h

When it is implementable by an SSE/SSE2 (x86/x64) or AdvSIMD (Arm64) instruction then you can just update the column corresponding to the T with the corresponding HWIntrinsic name.
For example, in the case of ushort you would update the TYP_USHORT column for SIMD_AS_HWINTRINSIC(VectorT128, op_Multiply, ... to be NI_SSE2_MultiplyLow

• Noting that TYP_BYTE is sbyte and TYP_UBYTE is byte
• Noting that for NI_VectorT256_* the baseline is AVX2 rather than SSE/SSE2

When it requires a higher ISA or if it has a more optimized version that requires a higher ISA, you set the entry to the same as the table entry.
For example, in the case of uint you would update the TYP_UINT column to be NI_VectorT128_op_Multiply

You would then update the impSimdAsHWIntrinsicSpecial method to generate the tree containing the specialized logic.
For example, in the case of NI_VectorT128_op_Multiply we have a path which emits Sse41.MultiplyLow and one that generates a fallback using only Sse2 intrinsics: https://github.com/dotnet/runtime/blob/master/src/coreclr/src/jit/simdashwintrinsic.cpp#L603-L654