Repository navigation
Investigate moving the types in System.Numerics.Vectors to use Hardware Intrinsics #956
Description
Activity
I had done some basic/preliminary investigation some time back and the numbers looked promising: https://github.com/dotnet/corefx/issues/25386#issuecomment-361137982
I am marking this up for grabs in case someone from the community wants to work on this. If you want to take a stab, feel free to say so and I can assign the issue out.
The "Microsoft DirectX Math" library is open-source, MIT-licensed, and has existing HWIntrinsic accelerated code for many of these APIs (but for Native code): https://github.com/microsoft/directxmath. This would likely be a good starting point as the algorithms have been around for a long time, have been in active use in production code, and are known to generally be correct and performant.
- NOTE: In some cases the C# algorithm and the DirectX algorithm differ slightly, we should make note of these cases and determine the best course of action.
FYI. @eerhardt, @danmosemsft, @CarolEidt, @fiigii
Also CC. @benaadams, who I know has looked at vectorizing/improving some of these types previously
@hughbe in case he is interested in a meaty task.
@tannergooding Thanks for opening this issue. I agree that porting
S.N.Vectorsto managed code in HW intrinsic is much better to maintain. But before that, we may need to address some differences betweenVector128/256<T>andVector2/3/4/<T>. For example,- We should have an internal API to convert
Vector2/3/4/<T>from/toVector128/256<T> Vector128/256<T>are immutable types, butVector3/4are mutable (fields ofVector3/4can be assigned), how can we solve this difference? Or remain the implementation of some operations in the runtime?
- We should have an internal API to convert
The "Microsoft DirectX Math" library is open-source,
@tannergooding Would you like to propose a "VectorMath" library for
Vector128/256<T>?Reacted by Jacek BlaszczynskiIMHO we should take the opportunity when rewriting
Vector<T>implementation to:-
Use almost complete implementation of Intel HW Intrinsics up to
AVX2to expand Vector API surface with several missing operations which were requested by community or became possible due to HW intrinsics implementation (this would require API proposal, discussion and approval) -
Provide Arm64 HW intrinsics support where possible
cc @CarolEidt @danmosemsft @eerhardt @fiigii @tannergooding
-
We should have an internal API to convert Vector2/3/4/ from/to Vector128/256
I don't believe this is needed. You should be able to load a
Vector2/3/4into aVector128<T>, perform all operations, and then convert back to the return type trivially and without much (if any) overhead.Users will also have their own vector/numeric types, so we should focus on making sure that getting a
Vector128<T>from a user defined struct is fast/efficient in general.Vector128/256 are immutable types, but Vector3/4 are mutable (fields of Vector3/4 can be assigned), how can we solve this difference? Or remain the implementation of some operations in the runtime?
I'm not sure why you think this is an issue. Could you elaborate?
@tannergooding Would you like to propose a "VectorMath" library for Vector128/256?
There will likely need to be some minimal helper library that operates solely on
Vector128<T>that theVector2/3/4code would call into. This would also be useful for other places in the framework that could take advantage of this.It would need to be internal at first, and discussions on making it public could happen later.
IMHO we should take the opportunity when rewriting Vector implementation to:
Any port should be a two/three step process:
- Do a clean port of the existing code and make sure it is equally performant
- Do a rewrite of the existing APIs to make improvements, where possible
- Add new APIs, as required, and implement accordingly
Doing them together makes it harder to review the code and track the changes.
ARM/ARM64 support should definitely happen, but only after the APIs have been reviewed/approved/implemented.
should be able to load a Vector2/3/4 into a Vector128
That would be great if the runtime can eliminate the round trip. If not, the memory load/store may have considerable overhead when we have
__vectorcallin the future.Users will also have their own vector/numeric types, so we should focus on making sure that getting a Vector128 from a user defined struct is fast/efficient in general.
Agree! Related to https://github.com/dotnet/coreclr/issues/19116
Reacted by Tanner GoodingThat would be great if the runtime can eliminate the round trip.
I would certainly hope that we can.
From my view, anything that would prevent us from moving the
System.Runtime.Vectorproject to use exclusively HWIntrinsics (outside the dynamicVector<T>sizing feature) is a potential perf problem for users wanting to use HWIntrinsics themselves. So we want to get those types of things flagged, investigated, fixed, etc.anything that would prevent us from moving the System.Runtime.Vector project to use exclusively HWIntrinsics (outside the dynamic Vector sizing feature) is a potential perf problem for users wanting to use HWIntrinsics themselves
This seems like a good enough excuse for me to take this on if nobody else is interested 😉
I'm at a bit of a loss how to start, though. Even something as trivial as
operator +onVector2is twisting my brain. As an intrinsic, the JIT knows whether the struct is currently in an xmm register and can emit just aaddps. If it's not in a register, it loads it withmovsdbefore emitting theaddps. Assuming I pinthisand take its address so I can explicitly load it withSse2.LoadScalarVector128(double*), is the JIT containment logic smart enough to know it's already in a xmm register and skip that part? And would it do the same withVector3where the load process ismovsd+movss+insertps?The constructors are even more confusing to me since they may be a setup for further SIMD operations or the fields may be immediately accessed. The JIT seems to know what to do about that now, but I can't imagine how managed code could get that right.
As far as the porting process, would the goal be to replace all
[intrinsic]methods first and then SIMD-ize the things that aren't using intrinsics today?27 remaining items
JIT was still treating the methods as intrinsic.
Ah, could you try to remove those ones from
simdintrinsiclist.h?Yes, that works. However, it requires modifying CoreCLR every iteration (longer inner loop) and impacts more than just the singular method a person may be working on (for example, the single
Equalsentry insimdintrinsiclist.himpacts theEqualsmethod onVector2,Vector3,Vector4,Vector, andVector<T>). It also leaves theSystem.Numerics.Vectorsassembly in an odd state where you can't determine what can or cannot be an intrinsic by looking at the source.I'd rather get the types actually respecting the
Intrinsicattribute first and then continue looking at swapping things out a method and type at a time afterwards.Reacted by Clinton IngramI'd rather get the types actually respecting the Intrinsic attribute first and then continue looking at swapping things out a method and type at a time afterwards.
Agree.
I had done an initial investigation of this in dotnet/coreclr#27483, but it was closed due to generating more complex trees and therefore missing some optimizations. This is not something that could be readily handled today.
@CarolEidt, @jkotas, @AndyAyersMS.
Unless you have any objections, I plan on closing this as unactionable until the blocking issues are addressed. Do we have issues currently tracking the needed improvements?
As an aside, with us looking at introducing more AOT/R2R options, the ISA specific paths codified in the JIT are becoming more problematic and we are needing to block more methods due to this (e.g. #33090). This also means consumers can't see the benefits of the ISA specific perf improvements when they are available. On the other hand, ISA specific paths codified in managed code don't have this problem and we are able to support both paths relatively trivially. We also have better codegen support in general (especially around containment). and so addressing some of the currently blocking issues may become more necessary as we move forward.
I plan on closing this as unactionable until the blocking issues are addressed
I think that this can be kept open to keep track of the progress. It is something we want to do, it just cannot be done today.
Agree with the rest.
Reacted by Tanner Gooding and Carol EidtGoing to close this as most off the SIMD intrinsics have already been ported to use HWIntrinsics internally during importation.
- ghost locked as resolved and limited conversation to collaborators
on Feb 14, 2021
Today, hardware acceleration of certain types in the
System.Numerics.Vectorsproject is achieved via[Intrinsic]attributes and corresponding runtime support. There are a few downsides to this approach:In
netcoreapp30, the new Hardware Intrinsics feature is supposed to ship. This feature also allows hardware acceleration but at a much more fine-grained level (APIs typically have a 1-to-1 mapping with underlying instructions).We should investigate moving the types in
System.Numerics.Vectorsto use hardware intrinsics as this has multiple potential benefits:Matrix4x4)