Cross Product for Vector2 and Vector4#111265
Merged
tannergooding merged 11 commits intodotnet:mainfrom Jan 28, 2025
Merged
Conversation
…tor4_CrossProduct
Refactor test methods in `Vector2Tests.cs` and `Vector4Tests.cs` for clarity and correctness. Enhance the `Cross` methods in `Vector2.cs` and `Vector4.cs` using SIMD techniques for improved performance.
ghost
added
|
Note regarding the |
1 similar comment
|
Note regarding the |
dotnet-policy-service
bot
added
the
community-contribution
huoyaoyuan
reviewed
Jan 10, 2025
src/libraries/System.Private.CoreLib/src/System/Numerics/Vector2.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Numerics/Vector4.cs
Outdated
Show resolved
Hide resolved
Reverted the Cross method to a simpler implementation by removing the use of Vector64 for vector math. The method now directly computes the cross product using the formula `value1.X * value2.Y - value1.Y * value2.X`, prioritizing clarity and performance on xarch architectures.
…r4.cs Co-authored-by: Huo Yaoyuan <huoyaoyuan@hotmail.com>
EgorBo
reviewed
Jan 10, 2025
src/libraries/System.Private.CoreLib/src/System/Numerics/Vector2.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Numerics/Vector4.cs
Outdated
Show resolved
Hide resolved
Updated the `Cross` method in `Vector2.cs` to utilize SIMD operations with `Vector128`, enhancing performance through hardware acceleration. The `Cross` method in `Vector3.cs` has been modified to use `AsVector128Unsafe()`, aligning with the performance improvements.
Refactor the `Cross` method to enhance performance by utilizing `Vector128.MultiplyAddEstimate`.
Removed unnecessary assignment in the Cross method and streamlined the return statement. This change enhances code clarity and efficiency by eliminating redundancy.
Comment on lines
+371
to
+372
| Vector128<float> v1 = vector1.AsVector128Unsafe(); | ||
| Vector128<float> v2 = vector2.AsVector128Unsafe(); |
Member
There was a problem hiding this comment.
If you're going to do this, you should modify the Shuffle below to use -1, rather than 3, for selecting W
It avoids a potential pessimization on some hardware if W is NaN or similar and is "free".
Comment on lines
341
to
345
| //return value1.X * value2.Y - value1.Y * value2.X; | ||
|
|
||
| Vector128<float> mul = value1.AsVector128Unsafe() * | ||
| Vector128.Shuffle(value2.AsVector128Unsafe(), Vector128.Create(1, 0, 1, 0)); | ||
| return (mul - Vector128.Shuffle(mul, Vector128.Create(1, 0, 1, 0))).ToScalar(); |
Member
There was a problem hiding this comment.
Similar here. It is preferable to use AsVector128 or to guarantee the zeroing of the upper elements by using -1 as the index in the shuffle. It helps avoid pessimization and should be naturally part of the underlying codegen without inserting additional instructions (in some cases at least, you'll need to compare and see which is better in most cases)
Refactored the `Cross` method in `Vector2.cs` to enhance performance by using vector shuffling and subtraction with SIMD operations. Updated the `Cross` method in `Vector3.cs` to correct shuffling indices for accurate component multiplication, also leveraging SIMD for improved efficiency.
tannergooding
approved these changes
Jan 28, 2025
grendello
added a commit
to grendello/runtime
that referenced
this pull request
Jan 28, 2025
* main: (31 commits) Fix linux-x86 build (dotnet#111861) Add FrozenDictionary specialization for integers / enums (dotnet#111886) [SRM] Refactor reading from streams. (dotnet#111323) Sign the DAC and DBI during the build process instead of in separate steps (dotnet#111416) Removing Entry2MethodDesc as it is unnecessary (dotnet#111756) Cross Product for Vector2 and Vector4 (dotnet#111265) Handle unicode in absolute URI path for combine. (dotnet#111710) Drop RequiresProcessIsolation on mcc tests (dotnet#111887) [main] Update dependencies from dotnet/roslyn (dotnet#111691) new trimmer feature System.TimeZoneInfo.Invariant (dotnet#111215) [browser] reduce msbuild memory footprint (dotnet#111751) Add debugging checks for stack overflow tests failure (dotnet#111867) Localized file check-in by OneLocBuild Task: Build definition ID 679: Build ID 2629821 (dotnet#111884) Bump main to preview2 (dotnet#111882) Avoid generic virtual dispatch for frozen collections alternate lookup (dotnet#108732) Bump main versioning to preview1 (dotnet#111880) Switch OneLoc to main (dotnet#111872) Improve docs on building ILVerify (dotnet#111851) Update Debian version to 13 (dotnet#111768) win32: add fallback to environment vars for system folder (dotnet#109673) ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Close #28731