Code review feedback.

* Removed `Config::cranelift_clear_cpu_flags`.
* Renamed `Config::cranelift_other_flag` to `Config::cranelift::flag_set`.
* Renamed `--cranelift-flag` to `--cranelift-set`.
* Renamed `--cranelift-preset` to `--cranelift-enable`.
This commit is contained in:
Peter Huene
2021-03-31 11:45:07 -07:00
parent 0000aa0646
commit a474524d3b
4 changed files with 36 additions and 44 deletions

View File

@@ -11,8 +11,14 @@
* The `Module::compile` method was added to support AOT compilation of a module. * The `Module::compile` method was added to support AOT compilation of a module.
* Added the `Config::cranelift_flag_enable` to enable setting Cranelift boolean * Added the `Config::target` method to change the compilation target of the
flags or presets in a config. configuration. This can be used in conjunction with `Module::compile` to target
a different host triple than the current one.
* Added the `Config::cranelift_flag_enable` method to enable setting Cranelift
boolean flags or presets in a config.
* Added CLI option `--cranelift-enable` to enable boolean settings and ISA presets.
### Changed ### Changed
@@ -20,7 +26,12 @@
singular `--wasm-features` option. The previous options are still supported, but singular `--wasm-features` option. The previous options are still supported, but
are not displayed in help text. are not displayed in help text.
* Breaking: the CLI option `--cranelift-flags` was changed to `--cranelift-flag`. * Breaking: `Config::cranelift_clear_cpu_flags` was removed. Use `Config::target`
to clear the CPU flags for the host's target.
* Breaking: `Config::cranelift_other_flag` was renamed to `Config::cranelift_flag_set`.
* Breaking: the CLI option `--cranelift-flags` was changed to `--cranelift-set`.
* Breaking: the CLI option `--enable-reference-types=false` has been changed to * Breaking: the CLI option `--enable-reference-types=false` has been changed to
`--wasm-features=-reference-types`. `--wasm-features=-reference-types`.

View File

@@ -441,10 +441,8 @@ impl Config {
/// ///
/// This method can be used to change the target triple. /// This method can be used to change the target triple.
/// ///
/// Note that any no Cranelift flags will be inferred for the given target. /// Cranelift flags will not be inferred for the given target and any
/// /// existing target-specific Cranelift flags will be cleared.
/// [`Config::cranelift_clear_cpu_flags`] will reset the target triple back to
/// the host's target.
/// ///
/// # Errors /// # Errors
/// ///
@@ -898,22 +896,6 @@ impl Config {
self self
} }
/// Clears native CPU flags inferred from the host.
///
/// Note: this method will change the target to that of the host.
///
/// By default Wasmtime will tune generated code for the host that Wasmtime
/// itself is running on. If you're compiling on one host, however, and
/// shipping artifacts to another host then this behavior may not be
/// desired. This function will clear all inferred native CPU features.
///
/// To enable CPU features afterwards it's recommended to use the
/// [`Config::cranelift_other_flag`] method.
pub fn cranelift_clear_cpu_flags(&mut self) -> &mut Self {
self.isa_flags = native::builder_without_flags();
self
}
/// Allows setting a Cranelift boolean flag or preset. This allows /// Allows setting a Cranelift boolean flag or preset. This allows
/// fine-tuning of Cranelift settings. /// fine-tuning of Cranelift settings.
/// ///
@@ -954,7 +936,7 @@ impl Config {
/// ///
/// This method can fail if the flag's name does not exist, or the value is not appropriate for /// This method can fail if the flag's name does not exist, or the value is not appropriate for
/// the flag type. /// the flag type.
pub unsafe fn cranelift_other_flag(&mut self, name: &str, value: &str) -> Result<&mut Self> { pub unsafe fn cranelift_flag_set(&mut self, name: &str, value: &str) -> Result<&mut Self> {
if let Err(err) = self.flags.set(name, value) { if let Err(err) = self.flags.set(name, value) {
match err { match err {
SetError::BadName(_) => { SetError::BadName(_) => {

View File

@@ -213,12 +213,12 @@ struct CommonOptions {
opt_level: Option<wasmtime::OptLevel>, opt_level: Option<wasmtime::OptLevel>,
/// Cranelift common flags to set. /// Cranelift common flags to set.
#[structopt(long = "cranelift-flag", value_name = "NAME=VALUE", parse(try_from_str = parse_cranelift_flag))] #[structopt(long = "cranelift-set", value_name = "NAME=VALUE", parse(try_from_str = parse_cranelift_flag))]
cranelift_flags: Vec<CraneliftFlag>, cranelift_set: Vec<(String, String)>,
/// The Cranelift ISA preset to use. /// The Cranelift boolean setting or preset to enable.
#[structopt(long, value_name = "PRESET")] #[structopt(long, value_name = "SETTING")]
cranelift_preset: Option<String>, cranelift_enable: Vec<String>,
/// Maximum size in bytes of wasm memory before it becomes dynamically /// Maximum size in bytes of wasm memory before it becomes dynamically
/// relocatable instead of up-front-reserved. /// relocatable instead of up-front-reserved.
@@ -273,15 +273,15 @@ impl CommonOptions {
self.enable_wasm_features(&mut config); self.enable_wasm_features(&mut config);
if let Some(preset) = &self.cranelift_preset { for name in &self.cranelift_enable {
unsafe { unsafe {
config.cranelift_flag_enable(preset)?; config.cranelift_flag_enable(name)?;
} }
} }
for CraneliftFlag { name, value } in &self.cranelift_flags { for (name, value) in &self.cranelift_set {
unsafe { unsafe {
config.cranelift_other_flag(name, value)?; config.cranelift_flag_set(name, value)?;
} }
} }
@@ -401,13 +401,7 @@ fn parse_wasm_features(features: &str) -> Result<wasmparser::WasmFeatures> {
memory64: false, memory64: false,
}) })
} }
fn parse_cranelift_flag(name_and_value: &str) -> Result<(String, String)> {
struct CraneliftFlag {
name: String,
value: String,
}
fn parse_cranelift_flag(name_and_value: &str) -> Result<CraneliftFlag> {
let mut split = name_and_value.splitn(2, '='); let mut split = name_and_value.splitn(2, '=');
let name = if let Some(name) = split.next() { let name = if let Some(name) = split.next() {
name.to_string() name.to_string()
@@ -419,7 +413,7 @@ fn parse_cranelift_flag(name_and_value: &str) -> Result<CraneliftFlag> {
} else { } else {
bail!("missing value in cranelift flag"); bail!("missing value in cranelift flag");
}; };
Ok(CraneliftFlag { name, value }) Ok((name, value))
} }
fn parse_target(s: &str) -> Result<Triple> { fn parse_target(s: &str) -> Result<Triple> {

View File

@@ -16,14 +16,14 @@ fn caches_across_engines() {
// differ in shared cranelift flags // differ in shared cranelift flags
let res = Module::deserialize( let res = Module::deserialize(
&Engine::new(&Config::new().cranelift_nan_canonicalization(true)).unwrap(), &Engine::new(Config::new().cranelift_nan_canonicalization(true)).unwrap(),
&bytes, &bytes,
); );
assert!(res.is_err()); assert!(res.is_err());
// differ in cranelift settings // differ in cranelift settings
let res = Module::deserialize( let res = Module::deserialize(
&Engine::new(&Config::new().cranelift_opt_level(OptLevel::None)).unwrap(), &Engine::new(Config::new().cranelift_opt_level(OptLevel::None)).unwrap(),
&bytes, &bytes,
); );
assert!(res.is_err()); assert!(res.is_err());
@@ -31,7 +31,12 @@ fn caches_across_engines() {
// Missing required cpu flags // Missing required cpu flags
if cfg!(target_arch = "x86_64") { if cfg!(target_arch = "x86_64") {
let res = Module::deserialize( let res = Module::deserialize(
&Engine::new(&Config::new().cranelift_clear_cpu_flags()).unwrap(), &Engine::new(
Config::new()
.target(&target_lexicon::Triple::host().to_string())
.unwrap(),
)
.unwrap(),
&bytes, &bytes,
); );
assert!(res.is_err()); assert!(res.is_err());