Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 4 commits into from
Apr 30, 2021
Merged

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

merged 4 commits into from
Apr 30, 2021

Conversation

tannergooding
Copy link
Member

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

@dotnet-issue-labeler dotnet-issue-labeler bot 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

Also CC @echesakovMSFT

@tannergooding
Copy link
Member Author

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

@tannergooding
Copy link
Member Author

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 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
Member

cc @dotnet/jit-contrib

tannergooding
tannergooding commented Apr 29, 2021
View reviewed changes
src/coreclr/vm/class.cpp
@@ -1502,7 +1502,7 @@ int MethodTable::GetVectorSize()
// We need to verify that T (the element or "base" type) is a primitive type.
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

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

PayloadShouldHaveSimilarSizeWhenSplitIntoSegments test failure is tracked by #52031.

@tannergooding
Copy link
Member Author

@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

@AntonLapounov AntonLapounov AntonLapounov approved these changes

@jkotas jkotas jkotas approved these changes

@EgorBo EgorBo EgorBo 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