Wasmtime GitHub notifications bot (Mar 14 2024 at 03:04): Wasmtime GitHub notifications bot (Mar 14 2024 at 03:04):alexcrichton opened PR #8129 from alexcrichton:test-wasmtime-codegen to bytecodealliance:main:
This commit adds a suite of tests at
tests/asm/*.watwhich is intended to replace the tests incranelift/filetests/filetests/wasm. Tests are configured differently than before using Wasmtime CLI flags rather than a custom TOML-based configuration scheme. Otherwise though the same shape of tests is supported.This commit migrates a small handful of tests as a showcase and bulk migration is left for a follow-up.
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Mar 14 2024 at 03:04):alexcrichton requested fitzgen for a review on PR #8129.
Wasmtime GitHub notifications bot (Mar 14 2024 at 03:04):alexcrichton requested wasmtime-compiler-reviewers for a review on PR #8129.
Wasmtime GitHub notifications bot (Mar 14 2024 at 03:04):alexcrichton requested wasmtime-core-reviewers for a review on PR #8129.
Wasmtime GitHub notifications bot (Mar 14 2024 at 03:04):alexcrichton requested wasmtime-default-reviewers for a review on PR #8129.
Wasmtime GitHub notifications bot (Mar 14 2024 at 03:04):alexcrichton requested jameysharp for a review on PR #8129.
Wasmtime GitHub notifications bot (Mar 14 2024 at 03:05):alexcrichton commented on PR #8129:
This was inspired by https://github.com/bytecodealliance/wasmtime/pull/8125 and the discussion there
Wasmtime GitHub notifications bot (Mar 14 2024 at 03:32):alexcrichton updated PR #8129.
Wasmtime GitHub notifications bot (Mar 14 2024 at 14:02):fitzgen submitted PR review:
Instead of
tests/asmmaybe we should call thistests/disortests/disassince there are CLIF "disassemblies" in there too?
Wasmtime GitHub notifications bot (Mar 14 2024 at 14:02):fitzgen submitted PR review:
Instead of
tests/asmmaybe we should call thistests/disortests/disassince there are CLIF "disassemblies" in there too?
Wasmtime GitHub notifications bot (Mar 14 2024 at 14:02):fitzgen created PR review comment:
Can we avoid manual parsing and do the TOML thing like the other tests? Can still use exact CLI flags by having the toml just contain a string of CLI flags for that particular setting.
But manual parsing is just annoying to extend down the line.
Wasmtime GitHub notifications bot (Mar 14 2024 at 14:21):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Mar 14 2024 at 14:21):alexcrichton created PR review comment:
I was actually hoping we could skip TOML entirely since that felt heavyweight enough to justify with custom heaps/tables but with just a few booleans and such it feels sort of overkill. For example it feels overly verbose to do:
;;! flags = ["-Oopt-level=s", "-Ccranelift-has-avx"]as opposed to:
;;! flags: -O opt-level=s -C cranelift-has-avxI don't necessarily have a great feeling for how extensible we want this to be down the line though. I was sort of also taking inspiration from compiletest-based tests in rust-lang/rust, but I know the parsing there is sort of gnarly nowadays given all the directives it supports.
Wasmtime GitHub notifications bot (Mar 14 2024 at 14:24):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Mar 14 2024 at 14:24):fitzgen created PR review comment:
I was thinking we would have a single string for all CLI flags rather than an array of strings of individual flags, which should make it a little less verbose:
;;! flags: "-O opt-level=s -C cranelift-has-avx"
Wasmtime GitHub notifications bot (Mar 14 2024 at 14:24):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Mar 14 2024 at 14:24):fitzgen created PR review comment:
I was sort of also taking inspiration from compiletest-based tests in rust-lang/rust, but I know the parsing there is sort of gnarly nowadays given all the directives it supports.
Yeah this is the kind of thing I am afraid of
Wasmtime GitHub notifications bot (Mar 14 2024 at 14:25):fitzgen edited PR review comment.
Wasmtime GitHub notifications bot (Mar 14 2024 at 14:43):alexcrichton updated PR #8129.
Wasmtime GitHub notifications bot (Mar 14 2024 at 14:44):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Mar 14 2024 at 14:44):alexcrichton created PR review comment:
Ok, I've switched back to TOML
Last updated: Nov 22 2024 at 17:03 UTC