-
Notifications
You must be signed in to change notification settings - Fork 410
refactor: clean up unused LLVM SIMD #4715
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
refactor: clean up unused LLVM SIMD #4715
Conversation
|
Thank you for contributing to Miri! A reviewer will take a look at your PR, typically within a week or two. |
|
I'm not sure everything has been synchronized all the way, based on the CI log where it complains that there is no implementation for Did the tests pass for you locally? |
|
Yeah, this only makes sense once the stdarch changes are synced to rustc (I don't know if that happened), and then rustc is synced to Miri. @hulxv it's generally a good idea to ask before starting to work on an issue. :) And indeed, please run tests locally. CI exists to catch oversights that happen, but it is not a good replacement for local testing: the feedback cycle is just way too slow for that, and it is too noisy (project members get emails for PR updates). |
This did happen in rust-lang/rust#149118 |
Ah, that's shortly after the most recent sync (yesterday morning). I can kick off a manual sync to unblock this PR. |
|
Uh, no, the fix for failing tests is not to remove the tests. |
Oh, I am really for that. I didn't know that would annoy you. I think the CI will work now. I tested it locally. Can you please explain the current situation? What's this sync problem? |
Aren't these intrinsics removed? how can I fix them then? |
You will have to wait until #4716 lands, and then rebase your branch. Then CI should pass. The failing tests indicate that in the version used for testing, the intrinsics are still needed. The test is important to keep, it ensures that Miri can run all these stdarch functions. |
|
So I need to revert d250793 right? Is there anything else I need to do? |
|
You should remove that commit, and rebase your PR once #4716 lands. |
889a65e to
de6754f
Compare
|
Is it good now? |
|
This looks great, thanks! Please squash the commits. You can squash manually if there are multiple independent commits you want to preserve, or use @rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
refactor: remove `maskload.{d, q, d.256, q.256}` and `maskstore.{d, q, d.256, q.256}`
refactor: remove unsupported AVX2 shift intrinsic implementations
refactor: remove unused functions
de6754f to
eadc1b5
Compare
|
@rustbot ready |
Close #4659