-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
winch: Improve tunables/configuration validation #9490
winch: Improve tunables/configuration validation #9490
Conversation
Subscribe to Label Action
This issue or pull request has been labeled: "wasmtime:api", "wasmtime:config", "winch"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
Label Messager: wasmtime:configIt looks like you are changing Wasmtime's configuration options. Make sure to
To modify this label's message, edit the To add new label messages or remove existing label messages, edit the |
For the fuzz failure I might recommend updating somewhere around here to switch back to Cranelift if epochs (or other Winch-unsupported configs) are enabled during fuzzing |
Subscribe to Label Actioncc @fitzgen
This issue or pull request has been labeled: "fuzzing"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
This commit handles dead code warnings produced by the compiler when the macro is invoked as: #[wasmtime_test(strategies(not(Cranelift))] Which occur because Winch currently only offers support for x86_64. The motivation behind this change comes from bytecodealliance#9490, which is where the warnings surfaced.
This commit handles dead code warnings produced by the compiler when the macro is invoked as: #[wasmtime_test(strategies(not(Cranelift))] Which occur because Winch currently only offers support for x86_64. The motivation behind this change comes from #9490, which is where the warnings surfaced.
This commit introduces proper validation and documentation to handle the engine features not currently supported by Winch (e.g., `consume_fuel`, follow-up to the discussion in bytecodealliance#9472). In this commit, the tunables validation is done at the `set_tunables` method in Winch's `wasmtime_environ::compile::CompilerBuilder` implemetation, which enables removing Winch specific logic from `Config::validate` This change also introduces the question of how to consolidate the compiler specific flags with Winch (e.g, the user-specified `cranelift_*` options) given that not all of them are applicable to Winch (e.g., `cranelift_debug_verifier`, `cranelift_nan_canonicalization`), this change doesn't introduce any functionality on this front, however, it's probably something worth considering/discussing.
This allows a couple of things: * Pre-configuring `Winch` as the strategy * Handling the target architetures supported by Winch
4a1d6df
to
4e280bb
Compare
This commit introduces proper validation and documentation to handle the engine features not currently supported by Winch (e.g.,
consume_fuel
, follow-up to the discussion in#9472).
In this commit, the tunables validation is done at the
set_tunables
method in Winch'swasmtime_environ::compile::CompilerBuilder
implemetation, which enables removing Winch specific logic fromConfig::validate
This change also introduces the question of how to consolidate the compiler specific flags with Winch (e.g, the user-specified
cranelift_*
options) given that not all of them are applicable to Winch (e.g.,cranelift_debug_verifier
,cranelift_nan_canonicalization
), this change doesn't introduce any functionality on this front, however, it's probably something worth considering/discussing.