IndexOutOfRangeException in System.Numerics.Vector<T>(array) Where array Size < 32 Bytes

[zcanann]

System.Numerics.Vector<byte> vec = new System.Numerics.Vector<byte>(new byte[] { 4, 3, 2, 1, 6, 4, 2, 4 });

This call will crash with an IndexOutOfRangeException exception. I'm pulling in this library as a NuGet package. If I pass in >= 32 bytes, this code works.

The same logic applies to other data types if they do not fill enough at least 32 bytes.

Is there a reason for this behavior? My use case involves passing variable length bytes (anywhere from 1 to millions). The constructor does not make this behavior clear, and I'd prefer to not have to pad my byte arrays to use this.

If the library is not supposed to support values less than the size of the XMM/YMM register, then it would be ideal to at least throw a more specific exception.

[benaadams]

System.Numerics.Vector<byte>.Count should tell you the size it supports (which varies based on CPU) it always chooses the largest supported vector size

[zcanann]

Perhaps an exception with more information would be useful then (ie expecting x bytes)

IndexOutOfRangeException is a bit vague for first-time users of this library.

EDIT: Alternatively, address https://github.com/dotnet/corefx/issues/23448 to improve the clarity of what constructor calls do.

[terrajobst]

Normally I'd expect the constructor to throw an ArgumentException if the array's length is less than than Vector<T>.Count. I don't recall whether we explicitly don't do length validation for perf reasons. Also, I don't recall whether the constructor is an intrinsic or not.

@CarolEidt, do you have any thoughts here?

EDIT: @eerhardt just pointed me to the code. Looks intentional, but we do have validation so we could easily change the exception. Changing the exception type is a binary breaking change, not sure it's worth it. We should, however, update the message to explain the error condition.

[eerhardt]

The issue is that the constructor that takes only an array calls into the constructor that takes an array and an index. So that's where the "index" in "IndexOutOfRangeException" comes from -

https://github.com/dotnet/corefx/blob/075df17be2057be1d47f11597f0ee735cb39f60b/src/System.Numerics.Vectors/src/System/Numerics/Vector.tt#L186-L189

Perhaps just adding an appropriate message to this exception would suffice?

[zcanann]

Adding a message seems like a sufficient solution to me

[CarolEidt]

Adding a message seems like a sufficient solution to me

Agreed.

[WinCPP]

May I pick this up for the changes?

To test how it works, I did some change to pass a string "myexception" to the constructor of IndexOutOfRangeException as mentioned in this note by @eerhardt on the thread. However the change doesn't seem to have effect. It still puts out the string "Index was outside the bounds of the array." as exception message; i.e. the default exception message. Seems like my changes to the 'tt' file and the corresponding auto-generated file that has compiled into the assembly aren't being picked up.

I am not sure if I have followed correct step... Help appreciated.

[clarkis117]

@WinCPP You need to build the T4 templates in Visual Studio, so it generates a new .cs file with the changes you made. For the tests, this may involve copying the ...includes.tt file to same directory as the test.tt file. Otherwise, Visual Studio will give you a build error when attempting to build the T4 templates in the solution.

[WinCPP]

@clarkis117 Thanks. I am working on #23688 at this time and would like to take this up next.

@clarkis117 Now I remember, that time I had done the changes to the tt file itself but found that the somehow the generic message in the IndexOutOfRangeException was not getting overridden. It has been sometime that I worked on it. Will redo everything. Thanks!

[jkotas]

Seems like my changes to the 'tt' file and the corresponding auto-generated file that has compiled into the assembly aren't being picked up.

The implementation of [Intrinsics] methods is duplicated in the JIT. The code in .cs files is just a fallback for cases where the JIT intrinsic expansion does not kick in (e.g. when the method is called via reflection, or on platforms where the SIMD intrinsics are not implemented yet). It means that the change of exception message has to be done in the JIT to make it actually kick in.

[WinCPP]

The implementation of [Intrinsics] methods is duplicated in the JIT. The code in .cs files is just a fallback for cases where the JIT intrinsic expansion does not kick in (e.g. when the method is called via reflection, or on platforms where the SIMD intrinsics are not implemented yet). It means that the change of exception message has to be done in the JIT to make it actually kick in.

Is this something that I can do or is it offlimits for me...? I am interested...

[jkotas]

It is not one liner change, but I am sure that you would be able to do it if you dive into it. There is pretty active community around the JIT intrinsics ( @4creators @CarolEidt @eerhardt @fiigii @sdmaclea @tannergooding ) that I am sure will be happy to give you guidance as necessary.

[fiigii]

@WinCPP JIT uses a GenTreeBoundsChk IR (with GT_SIMD_CHK oper) to throw the exception.

https://github.com/dotnet/coreclr/blob/aaafa705f6aa930343061a0753928839b1e75bf2/src/jit/simd.cpp#L2886-L2906

I guess this change may need to add a new value in enum SpecialCodeKind because the current SCK_RNGCHK_FAIL has the fixed error message.

[4creators]

Another piece of code to dive into is here:

https://github.com/dotnet/coreclr/blob/aaafa705f6aa930343061a0753928839b1e75bf2/src/jit/simd.cpp#L2647-L2664

What is interesting this are types of checks as above SCK_RNGCHK_FAIL and SCK_ARG_EXCPN