Extend a few TensorPrimitive optimizations to nint/nuint #116945
Conversation
|
Tagging subscribers to this area: @dotnet/area-system-numerics-tensors |
There was a problem hiding this comment.
Pull Request Overview
This PR extends existing TensorPrimitive vectorization optimizations to cover additional integer types (e.g., nint/nuint) by replacing direct typeof(T) checks with new helper methods (IsInt32Like, IsUInt32Like, IsInt64Like, IsUInt64Like). Key changes include:
- Updated
Sign,Divide,ConvertToInteger, andConvertToIntegerNativeclasses to useIsXXXLikehelpers in bothVectorizableproperties andInvokeoverloads. - Unified type checks for 32/64-bit signed and unsigned integers to include native-sized integer types.
- Removed some conditional
elsewrappers inSign.csto streamline early returns.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/libraries/System.Numerics.Tensors/src/System/Numerics/Tensors/netcore/TensorPrimitives.Sign.cs | Swapped typeof(T)==typeof(int/uint) for IsInt32Like<T>()/IsUInt32Like<T>() in vector methods. |
| src/libraries/System.Numerics.Tensors/src/System/Numerics/Tensors/netcore/TensorPrimitives.Divide.cs | Changed the integer branch in Vectorizable to IsInt32Like<T>(). |
| src/libraries/System.Numerics.Tensors/src/System/Numerics/Tensors/netcore/TensorPrimitives.ConvertToIntegerNative.cs | Replaced each typeof(TTo) check with corresponding IsIntXxxLike<TTo>() in vector conversions. |
| src/libraries/System.Numerics.Tensors/src/System/Numerics/Tensors/netcore/TensorPrimitives.ConvertToInteger.cs | Mirrored the native conversion changes for non-native paths with IsIntXxxLike<TTo>(). |
Comments suppressed due to low confidence (4)
src/libraries/System.Numerics.Tensors/src/System/Numerics/Tensors/netcore/TensorPrimitives.Divide.cs:73
- Consider adding unit tests for
nintandnuinttypes to cover the new vectorized code path introduced byIsInt32Like<T>()in theVectorizableproperty.
|| (Vector256.IsHardwareAccelerated && IsInt32Like<T>());
src/libraries/System.Numerics.Tensors/src/System/Numerics/Tensors/netcore/TensorPrimitives.ConvertToIntegerNative.cs:35
- Add tests to validate conversion to native integer types including
nintandnuintfor bothInvoke(Vector128<TFrom>)andInvoke(Vector256<TFrom>)paths.
(typeof(TFrom) == typeof(float) && IsInt32Like<TTo>()) ||
src/libraries/System.Numerics.Tensors/src/System/Numerics/Tensors/netcore/TensorPrimitives.ConvertToInteger.cs:35
- Include unit tests for
ConvertToInteger<TFrom, TTo>withnintandnuintto ensure the helper methods cover these types correctly.
(typeof(TFrom) == typeof(float) && IsInt32Like<TTo>()) ||
src/libraries/System.Numerics.Tensors/src/System/Numerics/Tensors/netcore/TensorPrimitives.Sign.cs:119
- The removal of the final closing brace appears to unbalance braces in this file and will cause a compilation error. Please ensure the correct number of closing braces are present.
}
...em.Numerics.Tensors/src/System/Numerics/Tensors/netcore/TensorPrimitives.ConvertToInteger.cs
Outdated
Show resolved
Hide resolved
| if (IsInt32Like<TTo>()) return Vector128.ConvertToInt32(x.AsSingle()).As<int, TTo>(); | ||
| if (IsUInt32Like<TTo>()) return Vector128.ConvertToUInt32(x.AsSingle()).As<uint, TTo>(); |
There was a problem hiding this comment.
Given then Vectorizable check, this could be (with optional asserts):
if (TTo.IsNegative(TTo.MinValue))
{
return Vector128.ConvertToInt32(x.AsSingle()).As<int, TTo>();
}
return Vector128.ConvertToUInt32(x.AsSingle()).As<uint, TTo>();It leads to much smaller IL that the JIT can trivially see is constant, without depending on the inliner
There was a problem hiding this comment.
if (TTo.IsNegative(TTo.MinValue))
The public API only constrains TTo to IBinaryInteger<TTo>, which does not have MinValue.
There was a problem hiding this comment.
You could do Vector128.IsNegative<TTo>(Vector128<T>.AllBitsSet) != Vector128<TTo>.Zero. This works since we know tto is an integer and AllBitsSet will either be -1 or MaxValue.
-- It's even simpler in .NET 10, but general intent is to try and avoid needing a second call to determine if a branch is dead or not, since that will impact inline profitability for these core helpers and we've already seen a few cases where the JIT gives up around that due to the size of the methods they're being inlined into.
There was a problem hiding this comment.
You could do Vector128.IsNegative(Vector128.AllBitsSet) != Vector128.Zero
And this is guaranteed to be constant folded down such that the expression and one or the other branches is eliminated?
...braries/System.Numerics.Tensors/src/System/Numerics/Tensors/netcore/TensorPrimitives.Sign.cs
Outdated
Show resolved
Hide resolved
2 participants
No description provided.