From 0ddfe97a09e36bc97caab36cd217b69fb76a1ba8 Mon Sep 17 00:00:00 2001 From: Peter Huene Date: Thu, 1 Apr 2021 19:32:15 -0700 Subject: [PATCH] Change how flags are stored in serialized modules. This commit changes how both the shared flags and ISA flags are stored in the serialized module to detect incompatibilities when a serialized module is instantiated. It improves the error reporting when a compiled module has mismatched shared flags. --- cranelift/codegen/meta/src/gen_settings.rs | 58 +------ cranelift/codegen/src/isa/aarch64/mod.rs | 13 +- cranelift/codegen/src/isa/aarch64/settings.rs | 2 +- cranelift/codegen/src/isa/arm32/mod.rs | 8 +- cranelift/codegen/src/isa/mod.rs | 9 +- cranelift/codegen/src/isa/riscv/mod.rs | 18 +- cranelift/codegen/src/isa/riscv/settings.rs | 2 +- cranelift/codegen/src/isa/x64/mod.rs | 13 +- cranelift/codegen/src/isa/x64/settings.rs | 2 +- cranelift/codegen/src/isa/x86/mod.rs | 18 +- cranelift/codegen/src/isa/x86/settings.rs | 2 +- cranelift/codegen/src/machinst/adapter.rs | 10 +- cranelift/codegen/src/machinst/mod.rs | 9 +- cranelift/codegen/src/settings.rs | 44 +++++ crates/environ/src/data_structures.rs | 2 +- crates/wasmtime/src/module/serialization.rs | 160 ++++++++++++------ src/commands/settings.rs | 10 +- 17 files changed, 194 insertions(+), 186 deletions(-) diff --git a/cranelift/codegen/meta/src/gen_settings.rs b/cranelift/codegen/meta/src/gen_settings.rs index 2808c2a777..d7116cac9c 100644 --- a/cranelift/codegen/meta/src/gen_settings.rs +++ b/cranelift/codegen/meta/src/gen_settings.rs @@ -70,31 +70,25 @@ fn gen_constructor(group: &SettingGroup, parent: ParentGroup, fmt: &mut Formatte fmtln!(fmt, "}"); } -/// Generates the `iter_enabled` function. +/// Generates the `iter` function. fn gen_iterator(group: &SettingGroup, fmt: &mut Formatter) { fmtln!(fmt, "impl Flags {"); fmt.indent(|fmt| { - fmt.doc_comment("Iterates the enabled boolean settings."); - fmtln!(fmt, "pub fn iter_enabled(&self) -> impl Iterator {"); + fmt.doc_comment("Iterates the setting values."); + fmtln!(fmt, "pub fn iter(&self) -> impl Iterator {"); fmt.indent(|fmt| { fmtln!(fmt, "let mut bytes = [0; {}];", group.settings_size); fmtln!(fmt, "bytes.copy_from_slice(&self.bytes[0..{}]);", group.settings_size); fmtln!(fmt, "DESCRIPTORS.iter().filter_map(move |d| {"); fmt.indent(|fmt| { - fmtln!(fmt, "if match d.detail {"); + fmtln!(fmt, "let values = match &d.detail {"); fmt.indent(|fmt| { - fmtln!(fmt, "detail::Detail::Bool { bit } => (bytes[d.offset as usize] & (1 << bit as usize)) != 0,"); - fmtln!(fmt, "_ => false"); + fmtln!(fmt, "detail::Detail::Preset => return None,"); + fmtln!(fmt, "detail::Detail::Enum { last, enumerators } => Some(TEMPLATE.enums(*last, *enumerators)),"); + fmtln!(fmt, "_ => None"); }); - fmtln!(fmt, "} {"); - fmt.indent(|fmt| { - fmtln!(fmt, "Some(d.name)"); - }); - fmtln!(fmt, "} else {"); - fmt.indent(|fmt| { - fmtln!(fmt, "None"); - }); - fmtln!(fmt, "}"); + fmtln!(fmt, "};"); + fmtln!(fmt, "Some(Value{ name: d.name, detail: d.detail, values, value: bytes[d.offset as usize] })"); }); fmtln!(fmt, "})"); }); @@ -103,39 +97,6 @@ fn gen_iterator(group: &SettingGroup, fmt: &mut Formatter) { fmtln!(fmt, "}"); } -/// Generates the `is_enabled` function. -fn gen_is_enabled(fmt: &mut Formatter) { - fmtln!(fmt, "impl Flags {"); - fmt.indent(|fmt| { - fmt.doc_comment("Checks if a boolean setting is enabled by name."); - fmtln!(fmt, "pub fn is_enabled(&self, name: &str) -> bool {"); - fmt.indent(|fmt| { - fmtln!(fmt, "match crate::constant_hash::probe(&TEMPLATE, name, crate::constant_hash::simple_hash(name)) {"); - fmt.indent(|fmt| { - fmtln!(fmt, "Err(_) => false,"); - fmtln!(fmt, "Ok(entry) => {"); - fmt.indent(|fmt| { - fmtln!(fmt, "let d = &TEMPLATE.descriptors[TEMPLATE.hash_table[entry] as usize];"); - fmtln!(fmt, "match &d.detail {"); - fmt.indent(|fmt| { - fmtln!(fmt, "detail::Detail::Bool{ bit } => {"); - fmt.indent(|fmt| { - fmtln!(fmt, "(self.bytes[d.offset as usize] & (1 << bit)) != 0"); - }); - fmtln!(fmt, "},"); - fmtln!(fmt, "_ => false"); - }); - fmtln!(fmt, "}"); - }); - fmtln!(fmt, "}"); - }); - fmtln!(fmt, "}"); - }); - fmtln!(fmt, "}"); - }); - fmtln!(fmt, "}"); -} - /// Emit Display and FromStr implementations for enum settings. fn gen_to_and_from_str(name: &str, values: &[&'static str], fmt: &mut Formatter) { fmtln!(fmt, "impl fmt::Display for {} {{", name); @@ -496,7 +457,6 @@ fn gen_group(group: &SettingGroup, parent: ParentGroup, fmt: &mut Formatter) { gen_constructor(group, parent, fmt); gen_iterator(group, fmt); - gen_is_enabled(fmt); gen_enum_types(group, fmt); gen_getters(group, fmt); gen_descriptors(group, fmt); diff --git a/cranelift/codegen/src/isa/aarch64/mod.rs b/cranelift/codegen/src/isa/aarch64/mod.rs index 9fbf7e60c5..a6892b301d 100644 --- a/cranelift/codegen/src/isa/aarch64/mod.rs +++ b/cranelift/codegen/src/isa/aarch64/mod.rs @@ -7,7 +7,7 @@ use crate::isa::Builder as IsaBuilder; use crate::machinst::{compile, MachBackend, MachCompileResult, TargetIsaAdapter, VCode}; use crate::result::CodegenResult; use crate::settings as shared_settings; -use alloc::{borrow::ToOwned, boxed::Box, string::String, vec::Vec}; +use alloc::{boxed::Box, vec::Vec}; use core::hash::{Hash, Hasher}; use regalloc::{PrettyPrint, RealRegUniverse}; use target_lexicon::{Aarch64Architecture, Architecture, Triple}; @@ -102,15 +102,8 @@ impl MachBackend for AArch64Backend { &self.flags } - fn enabled_isa_flags(&self) -> Vec { - self.isa_flags - .iter_enabled() - .map(ToOwned::to_owned) - .collect() - } - - fn is_flag_enabled(&self, flag: &str) -> bool { - self.isa_flags.is_enabled(flag) + fn isa_flags(&self) -> Vec { + self.isa_flags.iter().collect() } fn hash_all_flags(&self, mut hasher: &mut dyn Hasher) { diff --git a/cranelift/codegen/src/isa/aarch64/settings.rs b/cranelift/codegen/src/isa/aarch64/settings.rs index a9849c121b..9d3898e7b5 100644 --- a/cranelift/codegen/src/isa/aarch64/settings.rs +++ b/cranelift/codegen/src/isa/aarch64/settings.rs @@ -1,6 +1,6 @@ //! AArch64 Settings. -use crate::settings::{self, detail, Builder}; +use crate::settings::{self, detail, Builder, Value}; use core::fmt; // Include code generated by `cranelift-codegen/meta/src/gen_settings.rs:`. This file contains a diff --git a/cranelift/codegen/src/isa/arm32/mod.rs b/cranelift/codegen/src/isa/arm32/mod.rs index 50e6f6f59c..832fc46f47 100644 --- a/cranelift/codegen/src/isa/arm32/mod.rs +++ b/cranelift/codegen/src/isa/arm32/mod.rs @@ -7,7 +7,7 @@ use crate::machinst::{compile, MachBackend, MachCompileResult, TargetIsaAdapter, use crate::result::CodegenResult; use crate::settings; -use alloc::{boxed::Box, string::String, vec::Vec}; +use alloc::{boxed::Box, vec::Vec}; use core::hash::{Hash, Hasher}; use regalloc::{PrettyPrint, RealRegUniverse}; use target_lexicon::{Architecture, ArmArchitecture, Triple}; @@ -92,14 +92,10 @@ impl MachBackend for Arm32Backend { &self.flags } - fn enabled_isa_flags(&self) -> Vec { + fn isa_flags(&self) -> Vec { Vec::new() } - fn is_flag_enabled(&self, _flag: &str) -> bool { - false - } - fn hash_all_flags(&self, mut hasher: &mut dyn Hasher) { self.flags.hash(&mut hasher); } diff --git a/cranelift/codegen/src/isa/mod.rs b/cranelift/codegen/src/isa/mod.rs index 519f061f91..079a39fa8a 100644 --- a/cranelift/codegen/src/isa/mod.rs +++ b/cranelift/codegen/src/isa/mod.rs @@ -63,7 +63,7 @@ use crate::result::CodegenResult; use crate::settings; use crate::settings::SetResult; use crate::timing; -use alloc::{borrow::Cow, boxed::Box, string::String, vec::Vec}; +use alloc::{borrow::Cow, boxed::Box, vec::Vec}; use core::any::Any; use core::fmt; use core::fmt::{Debug, Formatter}; @@ -274,11 +274,8 @@ pub trait TargetIsa: fmt::Display + Send + Sync { /// Get the ISA-independent flags that were used to make this trait object. fn flags(&self) -> &settings::Flags; - /// Get the enabled ISA-dependent flags that were used to make this trait object. - fn enabled_isa_flags(&self) -> Vec; - - /// Determines if the given ISA-dependent flag is enabled. - fn is_flag_enabled(&self, flag: &str) -> bool; + /// Get the ISA-dependent flag values that were used to make this trait object. + fn isa_flags(&self) -> Vec; /// Hashes all flags, both ISA-independent and ISA-dependent, into the specified hasher. fn hash_all_flags(&self, hasher: &mut dyn Hasher); diff --git a/cranelift/codegen/src/isa/riscv/mod.rs b/cranelift/codegen/src/isa/riscv/mod.rs index 9db8b60ffa..2c1ebf1c85 100644 --- a/cranelift/codegen/src/isa/riscv/mod.rs +++ b/cranelift/codegen/src/isa/riscv/mod.rs @@ -15,12 +15,7 @@ use crate::isa::enc_tables::{self as shared_enc_tables, lookup_enclist, Encoding use crate::isa::Builder as IsaBuilder; use crate::isa::{EncInfo, RegClass, RegInfo, TargetIsa}; use crate::regalloc; -use alloc::{ - borrow::{Cow, ToOwned}, - boxed::Box, - string::String, - vec::Vec, -}; +use alloc::{borrow::Cow, boxed::Box, vec::Vec}; use core::any::Any; use core::fmt; use core::hash::{Hash, Hasher}; @@ -74,15 +69,8 @@ impl TargetIsa for Isa { &self.shared_flags } - fn enabled_isa_flags(&self) -> Vec { - self.isa_flags - .iter_enabled() - .map(ToOwned::to_owned) - .collect() - } - - fn is_flag_enabled(&self, flag: &str) -> bool { - self.isa_flags.is_enabled(flag) + fn isa_flags(&self) -> Vec { + self.isa_flags.iter().collect() } fn hash_all_flags(&self, mut hasher: &mut dyn Hasher) { diff --git a/cranelift/codegen/src/isa/riscv/settings.rs b/cranelift/codegen/src/isa/riscv/settings.rs index 40aa3bed2b..3da9f491fd 100644 --- a/cranelift/codegen/src/isa/riscv/settings.rs +++ b/cranelift/codegen/src/isa/riscv/settings.rs @@ -1,6 +1,6 @@ //! RISC-V Settings. -use crate::settings::{self, detail, Builder}; +use crate::settings::{self, detail, Builder, Value}; use core::fmt; // Include code generated by `cranelift-codegen/meta/src/gen_settings.rs`. This file contains a diff --git a/cranelift/codegen/src/isa/x64/mod.rs b/cranelift/codegen/src/isa/x64/mod.rs index ae8b889fbd..c150c01f23 100644 --- a/cranelift/codegen/src/isa/x64/mod.rs +++ b/cranelift/codegen/src/isa/x64/mod.rs @@ -9,7 +9,7 @@ use crate::isa::Builder as IsaBuilder; use crate::machinst::{compile, MachBackend, MachCompileResult, TargetIsaAdapter, VCode}; use crate::result::CodegenResult; use crate::settings::{self as shared_settings, Flags}; -use alloc::{borrow::ToOwned, boxed::Box, string::String, vec::Vec}; +use alloc::{boxed::Box, vec::Vec}; use core::hash::{Hash, Hasher}; use regalloc::{PrettyPrint, RealRegUniverse, Reg}; use target_lexicon::Triple; @@ -85,15 +85,8 @@ impl MachBackend for X64Backend { &self.flags } - fn enabled_isa_flags(&self) -> Vec { - self.x64_flags - .iter_enabled() - .map(ToOwned::to_owned) - .collect() - } - - fn is_flag_enabled(&self, flag: &str) -> bool { - self.x64_flags.is_enabled(flag) + fn isa_flags(&self) -> Vec { + self.x64_flags.iter().collect() } fn hash_all_flags(&self, mut hasher: &mut dyn Hasher) { diff --git a/cranelift/codegen/src/isa/x64/settings.rs b/cranelift/codegen/src/isa/x64/settings.rs index c5371bb132..501e153b46 100644 --- a/cranelift/codegen/src/isa/x64/settings.rs +++ b/cranelift/codegen/src/isa/x64/settings.rs @@ -1,6 +1,6 @@ //! x86 Settings. -use crate::settings::{self, detail, Builder}; +use crate::settings::{self, detail, Builder, Value}; use core::fmt; // Include code generated by `cranelift-codegen/meta/src/gen_settings.rs:`. This file contains a diff --git a/cranelift/codegen/src/isa/x86/mod.rs b/cranelift/codegen/src/isa/x86/mod.rs index c01f8acf01..54efe7fcfd 100644 --- a/cranelift/codegen/src/isa/x86/mod.rs +++ b/cranelift/codegen/src/isa/x86/mod.rs @@ -21,12 +21,7 @@ use crate::isa::{EncInfo, RegClass, RegInfo, TargetIsa}; use crate::regalloc; use crate::result::CodegenResult; use crate::timing; -use alloc::{ - borrow::{Cow, ToOwned}, - boxed::Box, - string::String, - vec::Vec, -}; +use alloc::{borrow::Cow, boxed::Box, vec::Vec}; use core::any::Any; use core::fmt; use core::hash::{Hash, Hasher}; @@ -83,15 +78,8 @@ impl TargetIsa for Isa { &self.shared_flags } - fn enabled_isa_flags(&self) -> Vec { - self.isa_flags - .iter_enabled() - .map(ToOwned::to_owned) - .collect() - } - - fn is_flag_enabled(&self, flag: &str) -> bool { - self.isa_flags.is_enabled(flag) + fn isa_flags(&self) -> Vec { + self.isa_flags.iter().collect() } fn hash_all_flags(&self, mut hasher: &mut dyn Hasher) { diff --git a/cranelift/codegen/src/isa/x86/settings.rs b/cranelift/codegen/src/isa/x86/settings.rs index 2d3a3f6698..f13431c1a2 100644 --- a/cranelift/codegen/src/isa/x86/settings.rs +++ b/cranelift/codegen/src/isa/x86/settings.rs @@ -1,6 +1,6 @@ //! x86 Settings. -use crate::settings::{self, detail, Builder}; +use crate::settings::{self, detail, Builder, Value}; use core::fmt; // Include code generated by `cranelift-codegen/meta/src/gen_settings.rs:`. This file contains a diff --git a/cranelift/codegen/src/machinst/adapter.rs b/cranelift/codegen/src/machinst/adapter.rs index a75e7393f6..543084a0b5 100644 --- a/cranelift/codegen/src/machinst/adapter.rs +++ b/cranelift/codegen/src/machinst/adapter.rs @@ -5,7 +5,7 @@ use crate::ir; use crate::isa::{EncInfo, Encoding, Encodings, Legalize, RegClass, RegInfo, TargetIsa}; use crate::machinst::*; use crate::regalloc::RegisterSet; -use crate::settings::Flags; +use crate::settings::{self, Flags}; #[cfg(feature = "testing_hooks")] use crate::regalloc::RegDiversions; @@ -58,12 +58,8 @@ impl TargetIsa for TargetIsaAdapter { self.backend.flags() } - fn enabled_isa_flags(&self) -> Vec { - self.backend.enabled_isa_flags() - } - - fn is_flag_enabled(&self, flag: &str) -> bool { - self.backend.is_flag_enabled(flag) + fn isa_flags(&self) -> Vec { + self.backend.isa_flags() } fn hash_all_flags(&self, hasher: &mut dyn Hasher) { diff --git a/cranelift/codegen/src/machinst/mod.rs b/cranelift/codegen/src/machinst/mod.rs index d1c96640dc..401863cbd8 100644 --- a/cranelift/codegen/src/machinst/mod.rs +++ b/cranelift/codegen/src/machinst/mod.rs @@ -64,7 +64,7 @@ use crate::binemit::{CodeInfo, CodeOffset, StackMap}; use crate::ir::condcodes::IntCC; use crate::ir::{Function, SourceLoc, StackSlot, Type, ValueLabel}; use crate::result::CodegenResult; -use crate::settings::Flags; +use crate::settings::{self, Flags}; use crate::value_label::ValueLabelsRanges; use alloc::boxed::Box; use alloc::vec::Vec; @@ -368,11 +368,8 @@ pub trait MachBackend { /// Return flags for this backend. fn flags(&self) -> &Flags; - /// Get the enabled ISA-dependent flags that were used to make this trait object. - fn enabled_isa_flags(&self) -> Vec; - - /// Determines if the given ISA-dependent flag is enabled. - fn is_flag_enabled(&self, flag: &str) -> bool; + /// Get the ISA-dependent flag values that were used to make this trait object. + fn isa_flags(&self) -> Vec; /// Hashes all flags, both ISA-independent and ISA-dependent, into the specified hasher. fn hash_all_flags(&self, hasher: &mut dyn Hasher); diff --git a/cranelift/codegen/src/settings.rs b/cranelift/codegen/src/settings.rs index ecdb17050b..88a3c62157 100644 --- a/cranelift/codegen/src/settings.rs +++ b/cranelift/codegen/src/settings.rs @@ -72,6 +72,50 @@ pub struct Setting { pub values: Option<&'static [&'static str]>, } +/// Represents a setting value. +/// +/// This is used for iterating values in `Flags`. +pub struct Value { + /// The name of the setting associated with this value. + pub name: &'static str, + pub(crate) detail: detail::Detail, + pub(crate) values: Option<&'static [&'static str]>, + pub(crate) value: u8, +} + +impl Value { + /// Gets the kind of setting. + pub fn kind(&self) -> SettingKind { + match &self.detail { + detail::Detail::Enum { .. } => SettingKind::Enum, + detail::Detail::Num => SettingKind::Num, + detail::Detail::Bool { .. } => SettingKind::Bool, + detail::Detail::Preset => unreachable!(), + } + } + + /// Gets the enum value if the value is from an enum setting. + pub fn as_enum(&self) -> Option<&'static str> { + self.values.map(|v| v[self.value as usize]) + } + + /// Gets the numerical value if the value is from a num setting. + pub fn as_num(&self) -> Option { + match &self.detail { + detail::Detail::Num => Some(self.value), + _ => None, + } + } + + /// Gets the boolean value if the value is from a boolean setting. + pub fn as_bool(&self) -> Option { + match &self.detail { + detail::Detail::Bool { bit } => Some(self.value & (1 << bit) != 0), + _ => None, + } + } +} + /// Collect settings values based on a template. #[derive(Clone, Hash)] pub struct Builder { diff --git a/crates/environ/src/data_structures.rs b/crates/environ/src/data_structures.rs index de13ac0e49..36eec310ec 100644 --- a/crates/environ/src/data_structures.rs +++ b/crates/environ/src/data_structures.rs @@ -11,7 +11,7 @@ pub mod ir { pub mod settings { pub use cranelift_codegen::settings::{ - builder, Builder, Configurable, Flags, OptLevel, SetError, Setting, SettingKind, + builder, Builder, Configurable, Flags, OptLevel, SetError, Setting, SettingKind, Value, }; } diff --git a/crates/wasmtime/src/module/serialization.rs b/crates/wasmtime/src/module/serialization.rs index 27936ffe9d..cc28cdeaf7 100644 --- a/crates/wasmtime/src/module/serialization.rs +++ b/crates/wasmtime/src/module/serialization.rs @@ -5,10 +5,11 @@ use crate::{Engine, Module, OptLevel}; use anyhow::{anyhow, bail, Context, Result}; use bincode::Options; use serde::{Deserialize, Serialize}; -use std::collections::HashMap; -use std::hash::{Hash, Hasher}; +use std::borrow::Cow; +use std::fmt; use std::str::FromStr; use std::sync::Arc; +use std::{collections::HashMap, fmt::Display}; use wasmtime_environ::Tunables; use wasmtime_environ::{isa::TargetIsa, settings}; use wasmtime_jit::{ @@ -29,7 +30,7 @@ fn bincode_options() -> impl Options { } // This exists because `wasmparser::WasmFeatures` isn't serializable -#[derive(Hash, Debug, Copy, Clone, Serialize, Deserialize)] +#[derive(Debug, Copy, Clone, Serialize, Deserialize)] struct WasmFeatures { pub reference_types: bool, pub multi_value: bool, @@ -175,13 +176,39 @@ impl<'a> SerializedModuleData<'a> { } } +#[derive(Serialize, Deserialize, Eq, PartialEq)] +enum FlagValue { + Enum(Cow<'static, str>), + Num(u8), + Bool(bool), +} + +impl From for FlagValue { + fn from(v: settings::Value) -> Self { + match v.kind() { + settings::SettingKind::Enum => Self::Enum(v.as_enum().unwrap().into()), + settings::SettingKind::Num => Self::Num(v.as_num().unwrap()), + settings::SettingKind::Bool => Self::Bool(v.as_bool().unwrap()), + settings::SettingKind::Preset => unreachable!(), + } + } +} + +impl Display for FlagValue { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::Enum(v) => v.fmt(f), + Self::Num(v) => v.fmt(f), + Self::Bool(v) => v.fmt(f), + } + } +} + #[derive(Serialize, Deserialize)] pub struct SerializedModule<'a> { target: String, - flags_hash: u64, - // Record the opt level as it is the most common Cranelift flag users might change - opt_level: OptLevel, - isa_flags: Vec, + shared_flags: HashMap, + isa_flags: HashMap, strategy: CompilationStrategy, tunables: Tunables, features: WasmFeatures, @@ -220,9 +247,16 @@ impl<'a> SerializedModule<'a> { Self { target: isa.triple().to_string(), - opt_level: isa.flags().opt_level().into(), - flags_hash: Self::simple_hash(isa.flags()), - isa_flags: isa.enabled_isa_flags(), + shared_flags: isa + .flags() + .iter() + .map(|v| (v.name.to_owned(), v.into())) + .collect(), + isa_flags: isa + .isa_flags() + .into_iter() + .map(|v| (v.name.to_owned(), v.into())) + .collect(), strategy: compiler.strategy(), tunables: compiler.tunables().clone(), features: compiler.features().into(), @@ -231,19 +265,17 @@ impl<'a> SerializedModule<'a> { } } - pub fn into_module(self, engine: &Engine) -> Result { + pub fn into_module(mut self, engine: &Engine) -> Result { let compiler = engine.compiler(); let isa = compiler.isa(); self.check_triple(isa)?; + self.check_shared_flags(isa)?; self.check_isa_flags(isa)?; self.check_strategy(compiler)?; self.check_tunables(compiler)?; self.check_features(compiler)?; - // Check the flags last as they are the least helpful in terms of diagnostic message - self.check_flags(isa)?; - let types = self .tables .into_iter() @@ -361,26 +393,63 @@ impl<'a> SerializedModule<'a> { Ok(()) } - fn check_flags(&self, isa: &dyn TargetIsa) -> Result<()> { - let host_level = isa.flags().opt_level().into(); - if self.opt_level != host_level { - bail!("Module was compiled with optimization level '{:?}' but '{:?}' is expected for the host", self.opt_level, host_level); + fn check_shared_flags(&mut self, isa: &dyn TargetIsa) -> Result<()> { + let mut shared_flags = std::mem::take(&mut self.shared_flags); + for value in isa.flags().iter() { + let name = value.name; + match shared_flags.remove(name) { + Some(v) => { + let host: FlagValue = value.into(); + if v != host { + bail!("Module was compiled with a different '{}' setting: expected '{}' but host has '{}'", name, v, host); + } + } + None => bail!("Module was compiled without setting '{}'", name), + } } - if self.flags_hash != Self::simple_hash(isa.flags()) { - bail!("Module was compiled with different Cranelift flags than the host"); + for (name, _) in shared_flags { + bail!( + "Module was compiled with setting '{}' but it is not present for the host", + name + ); } Ok(()) } - fn check_isa_flags(&self, isa: &dyn TargetIsa) -> Result<()> { - for flag in &self.isa_flags { - if !isa.is_flag_enabled(flag) { - bail!("Host is missing CPU flag '{}'", flag); + fn check_isa_flags(&mut self, isa: &dyn TargetIsa) -> Result<()> { + let mut isa_flags = std::mem::take(&mut self.isa_flags); + for value in isa.isa_flags().into_iter() { + let name = value.name; + let host: FlagValue = value.into(); + match isa_flags.remove(name) { + Some(v) => match (&v, &host) { + (FlagValue::Bool(v), FlagValue::Bool(host)) => { + // ISA flags represent CPU features; for boolean values, only + // treat it as an error if the module was compiled with the setting enabled + // but the host does not have it enabled. + if *v && !*host { + bail!("Module was compiled with setting '{}' enabled but the host does not support it", name); + } + } + _ => { + if v != host { + bail!("Module was compiled with a different '{}' setting: expected '{}' but host has '{}'", name, v, host); + } + } + }, + None => bail!("Module was compiled without setting '{}'", name), } } + for (name, _) in isa_flags { + bail!( + "Module was compiled with setting '{}' but it is not present for the host", + name + ); + } + Ok(()) } @@ -429,12 +498,6 @@ impl<'a> SerializedModule<'a> { ); } - fn simple_hash(v: T) -> u64 { - let mut hasher = std::collections::hash_map::DefaultHasher::new(); - v.hash(&mut hasher); - hasher.finish() - } - fn check_tunables(&self, compiler: &Compiler) -> Result<()> { let Tunables { static_memory_bound, @@ -595,55 +658,46 @@ mod test { Ok(()) } - #[test] - fn test_opt_level_mismatch() -> Result<()> { - let engine = Engine::default(); - let module = Module::new(&engine, "(module)")?; - - let mut serialized = SerializedModule::new(&module); - serialized.opt_level = OptLevel::None; - - match serialized.into_module(&engine) { - Ok(_) => unreachable!(), - Err(e) => assert_eq!( - e.to_string(), - "Module was compiled with optimization level 'None' but 'Speed' is expected for the host", - ), - } - - Ok(()) - } - #[test] fn test_cranelift_flags_mismatch() -> Result<()> { let engine = Engine::default(); let module = Module::new(&engine, "(module)")?; let mut serialized = SerializedModule::new(&module); - serialized.flags_hash += 1; + serialized.shared_flags.insert( + "opt_level".to_string(), + FlagValue::Enum(Cow::Borrowed("none")), + ); match serialized.into_module(&engine) { Ok(_) => unreachable!(), Err(e) => assert_eq!( e.to_string(), - "Module was compiled with different Cranelift flags than the host", + "Module was compiled with a different 'opt_level' setting: expected 'none' but host has 'speed'" ), } Ok(()) } + #[cfg(target_arch = "x86_64")] #[test] fn test_isa_flags_mismatch() -> Result<()> { let engine = Engine::default(); let module = Module::new(&engine, "(module)")?; let mut serialized = SerializedModule::new(&module); - serialized.isa_flags.push("not_a_flag".to_string()); + + serialized + .isa_flags + .insert("not_a_flag".to_string(), FlagValue::Bool(true)); match serialized.into_module(&engine) { Ok(_) => unreachable!(), - Err(e) => assert_eq!(e.to_string(), "Host is missing CPU flag 'not_a_flag'",), + Err(e) => assert_eq!( + e.to_string(), + "Module was compiled with setting 'not_a_flag' but it is not present for the host", + ), } Ok(()) diff --git a/src/commands/settings.rs b/src/commands/settings.rs index 63a7beff92..f1949e8c27 100644 --- a/src/commands/settings.rs +++ b/src/commands/settings.rs @@ -67,11 +67,13 @@ impl SettingsCommand { println!(); println!("Settings inferred for the current host:"); - let mut enabled = isa.enabled_isa_flags(); - enabled.sort(); + let mut values = isa.isa_flags(); + values.sort_by_key(|k| k.name); - for flag in enabled { - println!(" {}", flag); + for value in values { + if value.as_bool().unwrap_or(false) { + println!(" {}", value.name); + } } }