-
Notifications
You must be signed in to change notification settings - Fork 290
core_arch::x86::xsave::tests::test_xsavec{,64}
fails on Windows
#1672
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
Comments
The general problem with the XSAVE{,C,S} tests is that they rely on the fact that there is no state modification between these 2 calls: xsave::_xrstor64(a.ptr(), m);
xsave::_xsavec64(b.ptr(), m); That's something we can be sure of only if the Release mode is used, in which the pair of a XSAVE and XRSTOR instructions would happen one after the other. In debug mode, we might end up with a call to @Amanieu: I would recommend running these tests only in release mode, does it work for you? |
If I understand correctly, we need to ensure that Which instructions are problematic when between these 2? I don't think simple move instructions should be a problem, I guess |
To be honest, I've spent more time looking at the issue and don't see what's wrong. If I reduce the single test into a separate binary (https://github.com/marxin/xsave-test), then it fails both in Dev and Release mode on |
The more I look at these tests, the more I think we should just remove them entirely since the compiler/OS cannot guarantee that the state is preserved between the intrinsic calls. We should just keep a simple test that checks that the intrinsics can be successfully called, but not check the output. We still have |
I support what you've said. I am willing to prepare a PR for that. Will you be interested? |
Sure. |
It happens the test fails on Windows on my Zen PC:
AMD Ryzen 9 7900X 12-Core Processor
:It's probably going to need a similar treatment as I did in #1641. An Intel/AMD CPU expert would be more than welcomed here.
The text was updated successfully, but these errors were encountered: