-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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.
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
Arm64: Implement LoadVector64x* and LoadVector128x* APIs #92855
Conversation
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsAdd 51
Contributes to #84510
|
@vargaz - any idea why
|
@dotnet/jit-contrib @BruceForstall |
cc: @SamMonoRT @fanyang-mono - Is this the matter of adding an entry in |
I can reproduce. The crash appears unrelated to that specific test, the addition of the new LoadVectorX tests triggers it. |
Also, according to the test output: The new tests are not even supposed to run. |
Wondering why
That's right, it crashed while trying to run
I am assuming this PR cannot be merged until we fix this issue. Are you planning to submit a fix in this PR? |
AdvSimd is only supported on mono when running with llvm, not with the mono JIT.
Tracking this down will take some time. Would be nice to be able to disable this on mono, but I don't see any such functionality in GenerateHWIntrinsicTests_Arm.cs. |
The problem seems to be in the DataTable class in the tests:
If inArray is not 16 byte aligned, then the copy done by Unsafe.CopyBlockUnaligned () can write out of the bounds of the array, causing random crashes. This affects all the LoadVectorxTest.template files. A fix would be to bump the |
I am not sure why this is not problem for other test cases. |
Failures seems to be known: https://github.com/dotnet/runtime/pull/92855/checks?check_run_id=17415219399 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just a request to update the documentation for LoadVector equivalent.
Ah, wrong PR :) |
Add 51
LoadVector*
APIsContributes to #84510