alexcrichton opened PR #8140 from alexcrichton:migrate-more-tests
to bytecodealliance:main
:
All existing tests are moved and updated so now they're reflecting what Wasmtime is generating. Yay!
alexcrichton requested wasmtime-compiler-reviewers for a review on PR #8140.
alexcrichton requested elliottt for a review on PR #8140.
alexcrichton requested fitzgen for a review on PR #8140.
alexcrichton requested wasmtime-core-reviewers for a review on PR #8140.
jameysharp commented on PR #8140:
I'm trying to diff between old tests and new ones to get a sense of what's really changed. For example,
git diff main:cranelift/filetests/filetests/isa/x64/wasm HEAD:tests/disas/load-store/x64/
It seems odd that on x64,
load_store_dynamic_kind_i32_index_0_guard_no_spectre_i32_access_0_offset.wat
previously used a conditional branch to a trap, but now uses a cmov of a null pointer and lets the memory access trap.Perhaps the script needs to emit settings for
enable_heap_access_spectre_mitigation=false
?
alexcrichton updated PR #8140.
alexcrichton commented on PR #8140:
Oops, indeed! That's a difference between Cranelift's defaults and Wasmtime's defaults I forgot about with the script. Rebased with a fix now
jameysharp commented on PR #8140:
For the
table-*-fixed-size.wat
tests, you've changed them from "optimize" tests to "clif" tests. If you're going to change the type of these tests in this PR, I'd rather go the other direction, with fullopt_level=speed
optimization. On the other hand if this is meant to be a no-op-ish PR, then these should betest = "optimize
withopt_level=none
.
jameysharp commented on PR #8140:
As far as I can tell, this is the series of renames you did, which I needed to reproduce in order to diff the real changes:
mv cranelift/filetests/filetests/wasm/load-store/* tests/disas/load-store/ mv cranelift/filetests/filetests/isa/aarch64/wasm/* tests/disas/load-store/aarch64/ mv cranelift/filetests/filetests/isa/x64/wasm/* tests/disas/load-store/x64/ mv cranelift/filetests/filetests/isa/riscv64/wasm/* tests/disas/load-store/riscv64/ mv cranelift/filetests/filetests/isa/s390x/wasm/* tests/disas/load-store/s390x/ mv cranelift/filetests/filetests/wasm/*.wat tests/disas for f in tests/disas/load-store/riscv64/zb?.wat; do mv $f tests/disas/riscv64-$(basename $f) -i; done
After that,
git diff -R
shows the actual test changes.
alexcrichton updated PR #8140.
alexcrichton updated PR #8140.
alexcrichton commented on PR #8140:
you've changed them from "optimize" tests to "clif" tests
Oops yes I crossed the wires reproducing what was there before, thanks for catching!
this is the series of renames you did
Sorry you had to reconstruct this! You gave be a better idea of how to organize this though, I've now inserted a commit which explicitly moves everything with no changes, which means that while the overall diff is still messy the per-commit view is now easier to see what changed in each test.
alexcrichton updated PR #8140.
jameysharp submitted PR review:
That helped a ton, thank you! Could you apply the same
opt_level=0
change to the non-fixed-size table-get/set tests too? Otherwise I think these all match, if my quick read of the diff didn't miss anything.Very excited to start writing more tests with this new infrastructure. Thank you!
alexcrichton updated PR #8140.
alexcrichton has enabled auto merge for PR #8140.
alexcrichton merged PR #8140.
Last updated: Dec 23 2024 at 12:05 UTC