-
Notifications
You must be signed in to change notification settings - Fork 101
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
Restore simd_extract
and simd_insert
intrinsics
#1942
Restore simd_extract
and simd_insert
intrinsics
#1942
Conversation
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.
I think this is probably good to merge, my one concern might be a "followup PR" kind of thing if it's a problem at all...
let m = unsafe { simd_insert(y, 0, 1_i64) }; | ||
let n = unsafe { simd_insert(y, 1, 5_i64) }; |
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.
Do we know if this limitation (needing that cast) will cause problems in how it gets used in real code?
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.
Hey @tedinski, sorry for the late reply.
Just wanted to make clear that this is just mimicking rustc
's current behavior. Some SIMD intrinsics require additional type-checking done by the backend, as in here. This would prevent us from accepting that, in principle, cannot be compiled with rustc
(this was the case for this test).
Description of changes:
Restores the SIMD "construction" intrinsics
simd_extract
andsimd_insert
.As in #1853, we must perform some type-checks that depend on the back-end. These type-checking errors are now caught and we include a couple of test cases to ensure that they remain that way.
Resolved issues:
Towards #1148
Testing:
How is this change tested? Existing regression. Adds two new tests, converts a "fixme".
Is this a refactor change? Not completely, the main novelty here are the type-checks.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.