Move down S.N.Vector APIs and expose new conversion support #27401

tannergooding · 2019-10-23T23:16:17Z

This moves the S.N.Vector2/3/4 and supporting types down to S.P.Corelib. It additionally exposes the new conversion APIs approved in https://github.com/dotnet/corefx/issues/36379 to help reduce the turnaround time between CoreCLR and CoreFX (as S.N.Vector still needs type forwards exposed and the new APIs still need to be listed in the reference assembly after this change flows back).

…tem.Runtime.Intrinsics vectors

…P.Corelib

tannergooding · 2019-10-23T23:17:41Z

src/System.Private.CoreLib/shared/System/Numerics/Matrix4x4.cs

@@ -1761,7 +1759,6 @@ public static Matrix4x4 Transform(Matrix4x4 value, Quaternion rotation)
        /// <returns>The transposed matrix.</returns>
        public static unsafe Matrix4x4 Transpose(Matrix4x4 matrix)
        {
-#if HAS_INTRINSICS


We only build for platforms where the intrinsics are exposed, so the #ifdefs aren't needed.

Noting that they are not necessarily supported everywhere and some platforms, such as mono, will treat IsSupported as false (as with other places in shared corelib code).

tannergooding · 2019-10-23T23:18:00Z

src/System.Private.CoreLib/shared/System/Numerics/Vector2.cs

@@ -11,6 +11,7 @@ namespace System.Numerics
    /// <summary>
    /// A structure encapsulating two single precision floating point values and provides hardware accelerated methods.
    /// </summary>
+    [Intrinsic]


SIMD Types in S.P.Corelib are recognized by the Intrinsic attribute

tannergooding · 2019-10-23T23:18:53Z

src/System.Private.CoreLib/shared/System/Runtime/Intrinsics/Vector128.cs

+        public static Vector128<T> AsVector128<T>(this Vector<T> value)
+            where T : struct
+        {
+            Debug.Assert(Vector<T>.Count >= Vector128<T>.Count);


This currently assumes that Vector<T> won't ever be less than 128-bits. This is true today, but the logic would need to change if we ever supported something less (like on ARM/ARM64 where Vector64<T> is the minimum).

tannergooding · 2019-10-23T23:19:30Z

src/System.Private.CoreLib/shared/System/Runtime/Intrinsics/Vector256.cs

+        public static Vector256<T> AsVector256<T>(this Vector<T> value)
+            where T : struct
+        {
+            Debug.Assert(Vector256<T>.Count >= Vector<T>.Count);


This currently assumes Vector<T> is no bigger than Vector256<T>, the logic would need to change if we supported Vector512 or others...

src/vm/jitinterface.cpp

tannergooding · 2019-10-23T23:20:37Z

CC. @CarolEidt, @jkotas

EgorBo · 2019-10-23T23:21:22Z

Does this change affect CoreLib size? I know there's a linker but anyway.
Also, are the any plans to implement Vector4 using S.R.I?

tannergooding · 2019-10-23T23:24:03Z

Also, are the any plans to implement Vector4 using S.R.I?

There is a long standing issue (https://github.com/dotnet/corefx/issues/31425) tracking investigating this. The new zero-cost conversion APIs being exposed as part of this PR (approved in https://github.com/dotnet/corefx/issues/36379) will allow that to be more readily prototyped.

tannergooding · 2019-10-23T23:25:14Z

Does this change affect CoreLib size

Do you mean for the R2R scenario? These are basically a drop in the bucket compared to things like the System.Runtime.Intrinsics APIs that are being exposed (where we have 1500+ new APIs).

src/tools/crossgen2/Common/Compiler/SimdHelper.cs

tannergooding · 2019-10-24T02:58:13Z

@jkotas, I had to revert 9610d38 for the time being and will need to include it when addressing https://github.com/dotnet/coreclr/issues/27402 instead.

Since the type forwards don't exist in System.Numerics.Vectors.dll yet (and can't until after this change makes its rounds), the consumed types are still those from System.Numerics.Vectors.dll and they need to resolve to the intrinsic types to avoid various asserts such as:

Assert failure(PID 46168 [0x0000b458], Thread: 8848 [0x2290]): Assertion failed 'varDsc->lvType == TYP_SIMD12' in 'System.Runtime.Intrinsics.Vector128:AsVector3(struct):struct' (IL size 13)

    File: C:\Users\tagoo\repos\coreclr\src\jit\compiler.h Line: 3385
    Image: C:\Users\tagoo\repos\coreclr\bin\Product\Windows_NT.x86.Checked\crossgen.exe

tannergooding · 2019-10-24T03:36:13Z

Hmmm, actually it looks like there is something invalid for x86 (but not Windows x64) and a couple other targets around the following (and a couple other attempts with ReadUnaligned, etc):

public static Vector3 AsVector3(this Vector128<float> value)
{
    return Unsafe.As<Vector128<float>, Vector3>(ref value);
}

I've repushed up 9610d38 and changed the above method to throw PNSE while I try to investigate further...

CC. @CarolEidt

tannergooding · 2019-10-24T03:54:26Z

Found a simple repro indpendent of these changes and logged: https://github.com/dotnet/coreclr/issues/27405

tannergooding · 2019-10-24T10:18:39Z

Had to pull out 9610d38 anyways as there are tests that validate how the System.Numerics types were compiled.

https://dev.azure.com/dnceng/public/_build/results?buildId=399960&view=ms.vss-test-web.build-test-results-tab&runId=12482770&resultId=102505&paneView=debug

Return code: 1
Raw output file: /home/helixbot/work/f7112894-88c4-47fb-bb53-deb9a5aa9405/Work/629682d2-9549-4af3-bcbf-c1f477027888/Exec/JIT/SIMD/Reports/JIT.SIMD/VectorMul_r/VectorMul_r.output.txt
Raw output:
BEGIN EXECUTION
/home/helixbot/work/f7112894-88c4-47fb-bb53-deb9a5aa9405/Payload/corerun VectorMul_r.dll ''
Method System.Numerics.Vector4:op_Multiply was compiled but should not have been
Method System.Numerics.Vector3:op_Multiply was compiled but should not have been
Method System.Numerics.Vector2:op_Multiply was compiled but should not have been
Expected: 100
Actual: 255
END EXECUTION - FAILED
Test Harness Exitcode is : 1
To run the test:
> set CORE_ROOT=/home/helixbot/work/f7112894-88c4-47fb-bb53-deb9a5aa9405/Payload\n> /home/helixbot/work/f7112894-88c4-47fb-bb53-deb9a5aa9405/Work/629682d2-9549-4af3-bcbf-c1f477027888/Exec/JIT/SIMD/VectorMul_r/VectorMul_r.sh
Expected: True
Actual: False

tannergooding · 2019-10-24T15:39:14Z

This should be ready for merge after sign-off and then I can checkin the other changes after the type forwards flow from CoreFX

tannergooding added 5 commits October 23, 2019 16:08

Updating S.P.Corelib to start building Vector2/3/4 and supprting types

431704d

Removing HAS_INTRINSICS usages from Matrix4x4

1129395

Marking Vector2, Vector3, and Vector4 as Intrinsic

01baff2

Adding the new methods for converting between System.Numerics and Sys…

d9c6594

…tem.Runtime.Intrinsics vectors

Updating the VM and crossgen2 to recognize the S.N.Vector types in S.…

685b82c

…P.Corelib

tannergooding commented Oct 23, 2019

View reviewed changes

src/vm/jitinterface.cpp Show resolved Hide resolved

jkotas reviewed Oct 24, 2019

View reviewed changes

src/tools/crossgen2/Common/Compiler/SimdHelper.cs Show resolved Hide resolved

Updating Vector128.AsVector3 to throw PNSE

9e8bce5

jkotas approved these changes Oct 24, 2019

View reviewed changes

jkotas merged commit 7233801 into dotnet:master Oct 24, 2019

tannergooding mentioned this pull request Jan 31, 2020

Make isInSIMDModule obsolete dotnet/runtime#13632

Closed

Gnbrkm41 mentioned this pull request Feb 9, 2020

Implement Vector.Ceiling / Vector.Floor dotnet/runtime#31993

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Move down S.N.Vector APIs and expose new conversion support #27401

Move down S.N.Vector APIs and expose new conversion support #27401

Uh oh!

tannergooding commented Oct 23, 2019

Uh oh!

tannergooding Oct 23, 2019

Uh oh!

tannergooding Oct 23, 2019

Uh oh!

tannergooding Oct 23, 2019 •

edited

Loading

Uh oh!

tannergooding Oct 23, 2019

Uh oh!

Uh oh!

tannergooding commented Oct 23, 2019

Uh oh!

EgorBo commented Oct 23, 2019

Uh oh!

tannergooding commented Oct 23, 2019

Uh oh!

tannergooding commented Oct 23, 2019

Uh oh!

Uh oh!

tannergooding commented Oct 24, 2019

Uh oh!

tannergooding commented Oct 24, 2019 •

edited

Loading

Uh oh!

tannergooding commented Oct 24, 2019

Uh oh!

tannergooding commented Oct 24, 2019

Uh oh!

tannergooding commented Oct 24, 2019

Uh oh!

Uh oh!

Move down S.N.Vector APIs and expose new conversion support #27401

Move down S.N.Vector APIs and expose new conversion support #27401

Uh oh!

Conversation

tannergooding commented Oct 23, 2019

Uh oh!

tannergooding Oct 23, 2019

Choose a reason for hiding this comment

Uh oh!

tannergooding Oct 23, 2019

Choose a reason for hiding this comment

Uh oh!

tannergooding Oct 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tannergooding Oct 23, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tannergooding commented Oct 23, 2019

Uh oh!

EgorBo commented Oct 23, 2019

Uh oh!

tannergooding commented Oct 23, 2019

Uh oh!

tannergooding commented Oct 23, 2019

Uh oh!

Uh oh!

tannergooding commented Oct 24, 2019

Uh oh!

tannergooding commented Oct 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tannergooding commented Oct 24, 2019

Uh oh!

tannergooding commented Oct 24, 2019

Uh oh!

tannergooding commented Oct 24, 2019

Uh oh!

Uh oh!

tannergooding Oct 23, 2019 •

edited

Loading

tannergooding commented Oct 24, 2019 •

edited

Loading