HWIntrinsics API Proposal: VPMOVZXBD and friends need pointer-based overloads

[saucecontrol]

Currently, the V?PMOV[SZ]X[BWD][WDQ] HWIntrinsics methods define only Vector128<T> arguments, but most of these instructions consume less than 128 bits.

These instructions also support both [xy]mm,xmm or [xy]mm,mem encoding. In order to use the mem-encoded version of the instruction, we must currently write something like

Avx2.ConvertToVector256UInt32(Sse2.LoadVector128(bytePtr))

And the JIT folds the load, emitting the correct

vpmovzxbd ymm1,mmword ptr [rsi]

However, this gives Sse2.LoadVector128 non-deterministic behavior, as in this case only 64 bits are read.

Additionally, the SSE4.1 and AVX2 versions of these intrinsics are not symmetrical. For example, _mm_cvtepu8_epi32 is mapped to Sse41.ConvertToVector128Int32 while _mm256_cvtepu8_epi32 is mapped to Avx2.ConvertToVector256UInt32. The distinction there being that in Sse41 we return a signed result and in Avx2, we return unsigned.

I propose we do the following:

Change the existing Avx2 methods to return signed types for all widening operations to match the semantics of the Intel intrinsics (e.g. epu8->epi32) and the current Sse41 HWIntrinsics. These will continue to map to the zero-extend versions of the instructions.

namespace System.Runtime.Intrinsics.X86
{
    public abstract class Avx2
    {
-       public static Vector256<ushort> ConvertToVector256UInt16(Vector128<byte> value) => ConvertToVector256UInt16(value);
+       public static Vector256<short> ConvertToVector256Int16(Vector128<byte> value) => ConvertToVector256Int16(value);
-       public static Vector256<uint> ConvertToVector256UInt32(Vector128<byte> value) => ConvertToVector256UInt32(value);
+       public static Vector256<int> ConvertToVector256Int32(Vector128<byte> value) => ConvertToVector256Int32(value);
-       public static Vector256<uint> ConvertToVector256UInt32(Vector128<ushort> value) => ConvertToVector256UInt32(value);
+       public static Vector256<int> ConvertToVector256Int32(Vector128<ushort> value) => ConvertToVector256Int32(value);
-       public static Vector256<ulong> ConvertToVector256UInt64(Vector128<byte> value) => ConvertToVector256UInt64(value);
+       public static Vector256<long> ConvertToVector256Int64(Vector128<byte> value) => ConvertToVector256Int64(value);
-       public static Vector256<ulong> ConvertToVector256UInt64(Vector128<ushort> value) => ConvertToVector256UInt64(value);
+       public static Vector256<long> ConvertToVector256Int64(Vector128<ushort> value) => ConvertToVector256Int64(value);
-       public static Vector256<ulong> ConvertToVector256UInt64(Vector128<uint> value) => ConvertToVector256UInt64(value);
+       public static Vector256<long> ConvertToVector256Int64(Vector128<uint> value) => ConvertToVector256Int64(value);
    }
}

Add pointer overloads to both the SSE4.1 and AVX2 versions of the methods.

namespace System.Runtime.Intrinsics.X86
{
    public abstract class Sse41
    {
        /// <summary>
        /// __m128i _mm_cvtepi8_epi16 (__m128i a)
        ///   PMOVSXBW xmm, xmm/m64
        /// </summary>
        public static unsafe Vector128<short> ConvertToVector128Int16(sbyte* address) => ConvertToVector128Int16(address);
        /// <summary>
        /// __m128i _mm_cvtepu8_epi16 (__m128i a)
        ///   PMOVZXBW xmm, xmm/m64
        /// </summary>
        public static unsafe Vector128<short> ConvertToVector128Int16(byte* address) => ConvertToVector128Int16(address);
        /// <summary>
        /// __m128i _mm_cvtepi8_epi32 (__m128i a)
        ///   PMOVSXBD xmm, xmm/m32
        /// </summary>
        public static unsafe Vector128<int> ConvertToVector128Int32(sbyte* address) => ConvertToVector128Int32(address);
        /// <summary>
        /// __m128i _mm_cvtepu8_epi32 (__m128i a)
        ///   PMOVZXBD xmm, xmm/m32
        /// </summary>
        public static unsafe Vector128<int> ConvertToVector128Int32(byte* address) => ConvertToVector128Int32(address);
        /// <summary>
        /// __m128i _mm_cvtepi16_epi32 (__m128i a)
        ///   PMOVSXWD xmm, xmm/m64
        /// </summary>
        public static unsafe Vector128<int> ConvertToVector128Int32(short* address) => ConvertToVector128Int32(value);
        /// <summary>
        /// __m128i _mm_cvtepu16_epi32 (__m128i a)
        ///   PMOVZXWD xmm, xmm/m64
        /// </summary>
        public static unsafe Vector128<int> ConvertToVector128Int32(ushort* address) => ConvertToVector128Int32(address);
        /// <summary>
        /// __m128i _mm_cvtepi8_epi64 (__m128i a)
        ///   PMOVSXBQ xmm, xmm/m16
        /// </summary>
        public static unsafe Vector128<long> ConvertToVector128Int64(sbyte* address) => ConvertToVector128Int64(address);
        /// <summary>
        /// __m128i _mm_cvtepu8_epi64 (__m128i a)
        ///   PMOVZXBQ xmm, xmm/m16
        /// </summary>
        public static unsafe Vector128<long> ConvertToVector128Int64(byte* address) => ConvertToVector128Int64(address);
        /// <summary>
        /// __m128i _mm_cvtepi16_epi64 (__m128i a)
        ///   PMOVSXWQ xmm, xmm/m32
        /// </summary>
        public static unsafe Vector128<long> ConvertToVector128Int64(short* address) => ConvertToVector128Int64(address);
        /// <summary>
        /// __m128i _mm_cvtepu16_epi64 (__m128i a)
        ///   PMOVZXWQ xmm, xmm/m32
        /// </summary>
        public static unsafe Vector128<long> ConvertToVector128Int64(ushort* address) => ConvertToVector128Int64(address);
        /// <summary>
        /// __m128i _mm_cvtepi32_epi64 (__m128i a)
        ///   PMOVSXDQ xmm, xmm/m64
        /// </summary>
        public static unsafe Vector128<long> ConvertToVector128Int64(int* address) => ConvertToVector128Int64(address);
        /// <summary>
        /// __m128i _mm_cvtepu32_epi64 (__m128i a)
        ///   PMOVZXDQ xmm, xmm/m64
        /// </summary>
        public static unsafe Vector128<long> ConvertToVector128Int64(uint* address) => ConvertToVector128Int64(address);
    }

    public abstract class Avx2
    {
        /// <summary>
        /// __m256i _mm256_cvtepi8_epi16 (__m128i a)
        ///   VPMOVSXBW ymm, xmm/m128
        /// </summary>
        public static unsafe Vector256<short> ConvertToVector256Int16(sbyte* address) => ConvertToVector256Int16(address);
        /// <summary>
        /// __m256i _mm256_cvtepu8_epi16 (__m128i a)
        ///   VPMOVZXBW ymm, xmm/m128
        /// </summary>
        public static unsafe Vector256<short> ConvertToVector256Int16(byte* address) => ConvertToVector256Int16(address);
        /// <summary>
        /// __m256i _mm256_cvtepi8_epi32 (__m128i a)
        ///   VPMOVSXBD ymm, xmm/m64
        /// </summary>
        public static unsafe Vector256<int> ConvertToVector256Int32(sbyte* address) => ConvertToVector256Int32(address);
        /// <summary>
        /// __m256i _mm256_cvtepu8_epi32 (__m128i a)
        ///   VPMOVZXBD ymm, xmm/m64
        /// </summary>
        public static unsafe Vector256<int> ConvertToVector256Int32(byte* address) => ConvertToVector256Int32(address);
        /// <summary>
        /// __m256i _mm256_cvtepi16_epi32 (__m128i a)
        ///   VPMOVSXWD ymm, xmm/m128
        /// </summary>
        public static unsafe Vector256<int> ConvertToVector256Int32(short* address) => ConvertToVector256Int32(address);
        /// <summary>
        /// __m256i _mm256_cvtepu16_epi32 (__m128i a)
        ///   VPMOVZXWD ymm, xmm/m128
        /// </summary>
        public static unsafe Vector256<int> ConvertToVector256Int32(ushort* address) => ConvertToVector256Int32(address);
        /// <summary>
        /// __m256i _mm256_cvtepi8_epi64 (__m128i a)
        ///   VPMOVSXBQ ymm, xmm/m32
        /// </summary>
        public static unsafe Vector256<long> ConvertToVector256Int64(sbyte* address) => ConvertToVector256Int64(address);
        /// <summary>
        /// __m256i _mm256_cvtepu8_epi64 (__m128i a)
        ///   VPMOVZXBQ ymm, xmm/m32
        /// </summary>
        public static unsafe Vector256<long> ConvertToVector256Int64(byte* address) => ConvertToVector256Int64(address);
        /// <summary>
        /// __m256i _mm256_cvtepi16_epi64 (__m128i a)
        ///   VPMOVSXWQ ymm, xmm/m64
        /// </summary>
        public static unsafe Vector256<long> ConvertToVector256Int64(short* address) => ConvertToVector256Int64(address);
        /// <summary>
        /// __m256i _mm256_cvtepu16_epi64 (__m128i a)
        ///   VPMOVZXWQ ymm, xmm/m64
        /// </summary>
        public static unsafe Vector256<long> ConvertToVector256Int64(ushort* address) => ConvertToVector256Int64(address);
        /// <summary>
        /// __m256i _mm256_cvtepi32_epi64 (__m128i a)
        ///   VPMOVSXDQ ymm, xmm/m128
        /// </summary>
        public static unsafe Vector256<long> ConvertToVector256Int64(int* address) => ConvertToVector256Int64(address);
        /// <summary>
        /// __m256i _mm256_cvtepu32_epi64 (__m128i a)
        ///   VPMOVZXDQ ymm, xmm/m128
        /// </summary>
        public static unsafe Vector256<long> ConvertToVector256Int64(uint* address) => ConvertToVector256Int64(address);
    }
}

[saucecontrol]

cc @tannergooding

[tannergooding]

CC. @CarolEidt on the bug. We need to mark these instructions as no-containment for the time being.

[tannergooding]

Change the existing AVX2 methods to return signed types regardless of argument type

@saucecontrol, we should likely instead fix the SSE4.1 methods to return the unsigned type regardless of argument type. The various PMOVZX instructions are all documented as having zero-extend semantics, in which case returning the unsigned type is probably the most logical option.

[tannergooding]

I do agree that we should likely expose overloads that take an address. Given that you have cases like PMOVZXBW which takes a xmm/m64 and we don't have any intrinsics which could represent load 8 scalar bytes, I think this makes sense.

[saucecontrol]

The various PMOVZX instructions are all documented as having zero-extend semantics, in which case returning the unsigned type is probably the most logical option.

Ah, ok, if the convention elsewhere is that zero-extend always returns unsigned, that's fine as long as it's consistent. In my case, I'm taking the result of _mm256_cvtepu8_epi32 and passing it to _mm256_cvtepi32_ps so a signed result would be more convenient. I'll go ahead and flip that around.

we don't have any intrinsics which could represent load 8 scalar bytes

I had a look through the rest of the API to see if there were similar cases. Seems like insert and broadcast are the only other analogues, and those already expose address overloads.

[tannergooding]

so a signed result would be more convenient. I'll go ahead and flip that around.

Definitely. I think it becomes a question of "unexpected" vs "convenience" here. Generally speaking, narrowing just drops the higher bits and and widening either zero-extends (coming from an unsigned type) or sign-extends (coming from a signed type).

For example:

> sbyte s = -127; // 0x81
> (int)(s)        // 0xFFFFFF81
-127
> (uint)(s)       // 0xFFFFFF81
4294967169

vs

> byte b = 129;   // 0x81
> (int)(b)        // 0x00000081
129
> (uint)(s)       // 0x00000081
129

These are all widening operations. The ones that emit PMOVSX are documented as sign-extending and the ones that emit PMOVZX are documented as zero-extending. Given that we can't overload on return type, the ones that take a signed-type should likely return a signed type (and sign-extend) and the ones that tkae a unsigned-type should return an unsigned-type (and zero-extend).

[tannergooding]

Maybe we could have both if they were called ZeroExtendToInt16 vs SignExtendToInt16, but I'm not sure that is worthwhile given you have the zero-cost conversion via As

[saucecontrol]

Yeah, definitely not worth it for that. I've updated the API proposal.

[saucecontrol]

In looking through the Intel intrinsics guide a bit more, I'm back to thinking it makes more sense to have all of these return signed results. There are very few instructions that operate on unsigned int values, so it's very likely an As call would end up being required on every use of these on an unsigned source. And again, the semantics of the Intel intrinsics have them going unsigned->signed (e.g. epu8_epi32), which is of course always safe since these are widening.

Maybe we could have both if they were called ZeroExtendToInt16 vs SignExtendToInt16

This wouldn't be necessary, as the input arg type would determine that behavior

// sign-extend signed input
// _mm256_cvtepi8_epi32 / VPMOVSXBD
Vector256<int> Avx2.ConvertToVector256Int32(sbyte* address)
Vector256<int> Avx2.ConvertToVector256Int32(Vector128<sbyte> value)

// zero-extend unsigned input
// _mm256_cvtepu8_epi32 / VPMOVZXBD
Vector256<int> Avx2.ConvertToVector256Int32(byte* address)
Vector256<int> Avx2.ConvertToVector256Int32(Vector128<byte> value)

[tannergooding]

@saucecontrol, if you want to update the proposal with your suggestion, I think we can mark this as ready-for-review.

[saucecontrol]

Ok, I've flipped it back around. Should be ready for review now. Thanks!

[tannergooding]

Resolved with dotnet/coreclr#23893