Wasmtime GitHub notifications bot (Jun 15 2020 at 16:26): Wasmtime GitHub notifications bot (Jun 15 2020 at 16:26):bnjbvr opened PR #1880 from x64-testing to master:
- First commit adds capabilities in wasmtime to allow trying the new x64 backing (see commit message for a complete command line example of how to do it). @fitzgen can you have a look, please?
- Second commit makes it possible to run x64 tests in the
test runmode. It uncovered one issue I'm not quite sure how to handle, but that's preexisting and it's not been a problem so far. @abrown, can you have a look, please?This should make it much easier to hack on and try the new work-in-progress x64 backend. Eventually we can delete the Cargo feature for wasmtime once the transition to the new backend is over.
Wasmtime GitHub notifications bot (Jun 15 2020 at 16:26):bnjbvr requested fitzgen and abrown for a review on PR #1880.
Wasmtime GitHub notifications bot (Jun 15 2020 at 16:26):bnjbvr requested fitzgen and abrown for a review on PR #1880.
Wasmtime GitHub notifications bot (Jun 15 2020 at 16:38):fitzgen submitted PR Review.
Wasmtime GitHub notifications bot (Jun 15 2020 at 22:52):abrown submitted PR Review.
Wasmtime GitHub notifications bot (Jun 15 2020 at 22:52):abrown created PR Review Comment:
My understanding of this is that adding
haswellhere means that only hosts compatible with thehaswellflags will run this test (that's what we documented in https://github.com/bytecodealliance/wasmtime/blob/master/cranelift/docs/testing.md#test-run). Is that the intent? Or do we want to try to run onnehalemand above since that has the SSE flags needed? (It's probably hard to find hardware to trigger this error...).
Wasmtime GitHub notifications bot (Jun 15 2020 at 22:54):abrown submitted PR Review.
Wasmtime GitHub notifications bot (Jun 15 2020 at 22:54):abrown created PR Review Comment:
We could fix #1558 while we're at it: we probably want to print something to tell the user we ignored a run test because of hardware issues.
Wasmtime GitHub notifications bot (Jun 16 2020 at 09:17):bnjbvr submitted PR Review.
Wasmtime GitHub notifications bot (Jun 16 2020 at 09:17):bnjbvr created PR Review Comment:
This is actually reflecting a reality that's implicit at the moment: the host machine must match some SSE level to be able to effectively run the tests. I think this was implicit because the native host ISA would parse the CPUID, and all the machines running this would have the SSE SSE prerequisites. But this means that somebody who'd try to run the test on a very old machine might not be able to, and run into illegal instructions exceptions there.
This was uncovered by the change in this patch that uses the target ISA to compile, as defined in the test file, rather than the host's one built and configured from within the Rust code. (To answer your question in another comment, this was required to make it possible to pass the
use_new_backendflag to the test.)I'll identify what the precise SSE flags are, and will try to mitigate the potential SSE flags mismatch issue.
Wasmtime GitHub notifications bot (Jun 16 2020 at 09:24):bnjbvr submitted PR Review.
Wasmtime GitHub notifications bot (Jun 16 2020 at 09:24):bnjbvr created PR Review Comment:
The reason for which it was "commented out" here is that this test doesn't pass yet with the new x64 backend: we need enough IR lowering for creating the trampolines (and iconst/icmp).
Wasmtime GitHub notifications bot (Jun 16 2020 at 13:59):bnjbvr submitted PR Review.
Wasmtime GitHub notifications bot (Jun 16 2020 at 13:59):bnjbvr created PR Review Comment:
It's actually quite complicated, because the target specific flags are not to be queried outside the TargetIsa impl. I tried a lot of different things and lost a lot of time, so I went back to something simpler (this is testing only, after all): ensure the host is Nehalem at least, and by policy ensure that no one will add run tests that require more than that. We'll need to find a more long-term solution that does the right thing, probably with a full refactoring of the flags system.
Wasmtime GitHub notifications bot (Jun 16 2020 at 13:59):bnjbvr requested abrown for a review on PR #1880.
Wasmtime GitHub notifications bot (Jun 16 2020 at 14:01):bnjbvr updated PR #1880 from x64-testing to master:
- First commit adds capabilities in wasmtime to allow trying the new x64 backing (see commit message for a complete command line example of how to do it). @fitzgen can you have a look, please?
- Second commit makes it possible to run x64 tests in the
test runmode. It uncovered one issue I'm not quite sure how to handle, but that's preexisting and it's not been a problem so far. @abrown, can you have a look, please?This should make it much easier to hack on and try the new work-in-progress x64 backend. Eventually we can delete the Cargo feature for wasmtime once the transition to the new backend is over.
Wasmtime GitHub notifications bot (Jun 16 2020 at 15:47):bnjbvr updated PR #1880 from x64-testing to master:
- First commit adds capabilities in wasmtime to allow trying the new x64 backing (see commit message for a complete command line example of how to do it). @fitzgen can you have a look, please?
- Second commit makes it possible to run x64 tests in the
test runmode. It uncovered one issue I'm not quite sure how to handle, but that's preexisting and it's not been a problem so far. @abrown, can you have a look, please?This should make it much easier to hack on and try the new work-in-progress x64 backend. Eventually we can delete the Cargo feature for wasmtime once the transition to the new backend is over.
Wasmtime GitHub notifications bot (Jun 16 2020 at 16:57):bnjbvr updated PR #1880 from x64-testing to master:
- First commit adds capabilities in wasmtime to allow trying the new x64 backing (see commit message for a complete command line example of how to do it). @fitzgen can you have a look, please?
- Second commit makes it possible to run x64 tests in the
test runmode. It uncovered one issue I'm not quite sure how to handle, but that's preexisting and it's not been a problem so far. @abrown, can you have a look, please?This should make it much easier to hack on and try the new work-in-progress x64 backend. Eventually we can delete the Cargo feature for wasmtime once the transition to the new backend is over.
Wasmtime GitHub notifications bot (Jun 17 2020 at 14:19):bnjbvr updated PR #1880 from x64-testing to master:
- First commit adds capabilities in wasmtime to allow trying the new x64 backing (see commit message for a complete command line example of how to do it). @fitzgen can you have a look, please?
- Second commit makes it possible to run x64 tests in the
test runmode. It uncovered one issue I'm not quite sure how to handle, but that's preexisting and it's not been a problem so far. @abrown, can you have a look, please?This should make it much easier to hack on and try the new work-in-progress x64 backend. Eventually we can delete the Cargo feature for wasmtime once the transition to the new backend is over.
Wasmtime GitHub notifications bot (Jun 17 2020 at 14:39):bnjbvr updated PR #1880 from x64-testing to master:
- First commit adds capabilities in wasmtime to allow trying the new x64 backing (see commit message for a complete command line example of how to do it). @fitzgen can you have a look, please?
- Second commit makes it possible to run x64 tests in the
test runmode. It uncovered one issue I'm not quite sure how to handle, but that's preexisting and it's not been a problem so far. @abrown, can you have a look, please?This should make it much easier to hack on and try the new work-in-progress x64 backend. Eventually we can delete the Cargo feature for wasmtime once the transition to the new backend is over.
Wasmtime GitHub notifications bot (Jun 17 2020 at 15:18):bnjbvr merged PR #1880.
Last updated: Nov 22 2024 at 17:03 UTC