Skip to content

Updating the JIT to recognize and handle Vector64/128/256<T> for nint/nuint#52016

Merged
tannergooding merged 4 commits intodotnet:mainfrom
tannergooding:fix-arm-hang
Apr 30, 2021
Merged

Updating the JIT to recognize and handle Vector64/128/256<T> for nint/nuint#52016
tannergooding merged 4 commits intodotnet:mainfrom
tannergooding:fix-arm-hang

Conversation

@tannergooding
Copy link
Member

@tannergooding tannergooding commented Apr 28, 2021

This resolves the dotnet/runtime side for dotnet/runtimelab#1035 by recognizing nint and nuint for Vector64/128/256<T> as valid ABI types.

The general-purpose methods (such as As, GetElement, WithElement, etc) are blocked until they go through API review.

CC. @jkotas, @AntonLapounov

@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 28, 2021
@tannergooding
Copy link
Member Author

tannergooding commented Apr 28, 2021

Also CC @echesakovMSFT

@tannergooding
Copy link
Member Author

tannergooding commented Apr 28, 2021

We already have negative tests assertion that the various APIs throw NotSupportedException, such as: https://github.com/dotnet/runtime/blob/main/src/tests/JIT/HardwareIntrinsics/General/NotSupported/Vector128NIntAllBitsSet.cs

AntonLapounov
AntonLapounov reviewed Apr 28, 2021
View reviewed changes
AntonLapounov
AntonLapounov reviewed Apr 28, 2021
View reviewed changes
@tannergooding
Copy link
Member Author

tannergooding commented Apr 28, 2021

Logged #52017 for the API proposal. Most of the surface area is generic and support comes that way.

There is also some surface area that can be exposed for cases like: Sse.ConvertScalarToVector128Single where it has a 64-bit equivalent under Sse.X64.ConvertScalarToVector128Single, but that can be a separate proposal.

AntonLapounov
AntonLapounov reviewed Apr 28, 2021
View reviewed changes
AntonLapounov
AntonLapounov reviewed Apr 28, 2021
View reviewed changes
AntonLapounov
AntonLapounov reviewed Apr 28, 2021
View reviewed changes
AntonLapounov
AntonLapounov approved these changes Apr 28, 2021
View reviewed changes
Copy link
Member

@AntonLapounov AntonLapounov left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Thank you for the quick fix! There is still one outstanding suggestion regarding Vector256NUIntHandle.

@AntonLapounov AntonLapounov mentioned this pull request Apr 28, 2021
Closed
tannergooding and others added 2 commits April 28, 2021 17:39
@tannergooding @AntonLapounov
Update src/coreclr/jit/simd.cpp
a493dd5 
Co-authored-by: Anton Lapounov <anton.lapounov@microsoft.com>
@tannergooding
Applying formatting patch
0fdacc4
@BruceForstall
Copy link
Contributor

BruceForstall commented Apr 29, 2021

cc @dotnet/jit-contrib

tannergooding
tannergooding commented Apr 29, 2021
View reviewed changes
src/coreclr/vm/class.cpp
TypeHandle typeArg = GetInstantiation()[0];
CorElementType corType = typeArg.GetSignatureCorElementType();
if (corType >= ELEMENT_TYPE_I1 && corType <= ELEMENT_TYPE_R8)
if (((corType >= ELEMENT_TYPE_I1) && (corType <= ELEMENT_TYPE_R8)) || (corType == ELEMENT_TYPE_I) || (corType == ELEMENT_TYPE_U))
Copy link
Member Author

@tannergooding tannergooding Apr 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The VM was not correctly handling nint or nuint for the purposes of GetVectorSize() which was impacting downstream decisions about whether it was HFA/HVA and how it should be handled.

@AntonLapounov
Copy link
Member

AntonLapounov commented Apr 30, 2021

PayloadShouldHaveSimilarSizeWhenSplitIntoSegments test failure is tracked by #52031.

@tannergooding
Copy link
Member Author

tannergooding commented Apr 30, 2021

@jkotas (for the small VM side change) and @dotnet/jit-contrib could you give this a quick secondary review. It should be good to merge otherwise.

jkotas
jkotas approved these changes Apr 30, 2021
View reviewed changes
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you!

@tannergooding tannergooding merged commit a4ba77a into dotnet:main Apr 30, 2021
This was referenced May 5, 2021
Open
Closed
@tannergooding tannergooding mentioned this pull request May 7, 2021
Merged
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@JulieLeeMSFT JulieLeeMSFT mentioned this pull request Jun 8, 2021
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
@tannergooding tannergooding deleted the fix-arm-hang branch November 11, 2022 15:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@EgorBo EgorBo EgorBo approved these changes

@jkotas jkotas jkotas approved these changes

@AntonLapounov AntonLapounov AntonLapounov approved these changes

Assignees

No one assigned

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

6 participants

@tannergooding @BruceForstall @AntonLapounov @EgorBo @jkotas @karelz