Skip to content

Commit

Permalink
Remove usage of BTreeMap for compiler flags (#7287)
Browse files Browse the repository at this point in the history
* Remove usage of `BTreeMap` for compiler flags

No need for a datastructure here really, a simple list with static
strings works alright.

* Fix winch compile and a warning

* Fix test compile
  • Loading branch information
alexcrichton authored Oct 18, 2023
1 parent f7004c1 commit 61e11a6
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 29 deletions.
9 changes: 4 additions & 5 deletions crates/cranelift-shared/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use cranelift_codegen::{
ir::{self, ExternalName, UserExternalNameRef},
settings, FinalizedMachReloc, FinalizedRelocTarget, MachTrap,
};
use std::collections::BTreeMap;
use wasmtime_environ::{FlagValue, FuncIndex, Trap, TrapInformation};

pub mod isa_builder;
Expand Down Expand Up @@ -37,16 +36,16 @@ pub enum RelocationTarget {
/// Converts cranelift_codegen settings to the wasmtime_environ equivalent.
pub fn clif_flags_to_wasmtime(
flags: impl IntoIterator<Item = settings::Value>,
) -> BTreeMap<String, FlagValue> {
) -> Vec<(&'static str, FlagValue<'static>)> {
flags
.into_iter()
.map(|val| (val.name.to_string(), to_flag_value(&val)))
.map(|val| (val.name, to_flag_value(&val)))
.collect()
}

fn to_flag_value(v: &settings::Value) -> FlagValue {
fn to_flag_value(v: &settings::Value) -> FlagValue<'static> {
match v.kind() {
settings::SettingKind::Enum => FlagValue::Enum(v.as_enum().unwrap().into()),
settings::SettingKind::Enum => FlagValue::Enum(v.as_enum().unwrap()),
settings::SettingKind::Num => FlagValue::Num(v.as_num().unwrap()),
settings::SettingKind::Bool => FlagValue::Bool(v.as_bool().unwrap()),
settings::SettingKind::Preset => unreachable!(),
Expand Down
5 changes: 2 additions & 3 deletions crates/cranelift/src/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use object::write::{Object, StandardSegment, SymbolId};
use object::{RelocationEncoding, RelocationKind, SectionKind};
use std::any::Any;
use std::cmp;
use std::collections::BTreeMap;
use std::collections::HashMap;
use std::convert::TryFrom;
use std::mem;
Expand Down Expand Up @@ -522,11 +521,11 @@ impl wasmtime_environ::Compiler for Compiler {
self.isa.triple()
}

fn flags(&self) -> BTreeMap<String, FlagValue> {
fn flags(&self) -> Vec<(&'static str, FlagValue<'static>)> {
wasmtime_cranelift_shared::clif_flags_to_wasmtime(self.isa.flags().iter())
}

fn isa_flags(&self) -> BTreeMap<String, FlagValue> {
fn isa_flags(&self) -> Vec<(&'static str, FlagValue<'static>)> {
wasmtime_cranelift_shared::clif_flags_to_wasmtime(self.isa.isa_flags())
}

Expand Down
11 changes: 5 additions & 6 deletions crates/environ/src/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use object::{Architecture, BinaryFormat, FileFlags};
use serde_derive::{Deserialize, Serialize};
use std::any::Any;
use std::borrow::Cow;
use std::collections::BTreeMap;
use std::fmt;
use std::path;
use std::sync::Arc;
Expand Down Expand Up @@ -348,10 +347,10 @@ pub trait Compiler: Send + Sync {
}

/// Returns a list of configured settings for this compiler.
fn flags(&self) -> BTreeMap<String, FlagValue>;
fn flags(&self) -> Vec<(&'static str, FlagValue<'static>)>;

/// Same as [`Compiler::flags`], but ISA-specific (a cranelift-ism)
fn isa_flags(&self) -> BTreeMap<String, FlagValue>;
fn isa_flags(&self) -> Vec<(&'static str, FlagValue<'static>)>;

/// Get a flag indicating whether branch protection is enabled.
fn is_branch_protection_enabled(&self) -> bool;
Expand Down Expand Up @@ -383,16 +382,16 @@ pub trait Compiler: Send + Sync {

/// Value of a configured setting for a [`Compiler`]
#[derive(Serialize, Deserialize, Hash, Eq, PartialEq, Debug)]
pub enum FlagValue {
pub enum FlagValue<'a> {
/// Name of the value that has been configured for this setting.
Enum(Cow<'static, str>),
Enum(&'a str),
/// The numerical value of the configured settings.
Num(u8),
/// Whether the setting is on or off.
Bool(bool),
}

impl fmt::Display for FlagValue {
impl fmt::Display for FlagValue<'_> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Self::Enum(v) => v.fmt(f),
Expand Down
24 changes: 12 additions & 12 deletions crates/wasmtime/src/engine/serialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use anyhow::{anyhow, bail, Context, Result};
use object::write::{Object, StandardSegment};
use object::{File, FileFlags, Object as _, ObjectSection, SectionKind};
use serde_derive::{Deserialize, Serialize};
use std::collections::BTreeMap;
use std::str::FromStr;
use wasmtime_environ::obj;
use wasmtime_environ::{FlagValue, ObjectKind, Tunables};
Expand Down Expand Up @@ -142,7 +141,7 @@ pub fn check_compatible(engine: &Engine, mmap: &MmapVec, expected: ObjectKind) -
}
ModuleVersionStrategy::None => { /* ignore the version info, accept all */ }
}
bincode::deserialize::<Metadata>(data)?.check_compatible(engine)
bincode::deserialize::<Metadata<'_>>(data)?.check_compatible(engine)
}

fn detect_precompiled<'data, R: object::ReadRef<'data>>(
Expand Down Expand Up @@ -174,10 +173,12 @@ pub fn detect_precompiled_file(path: impl AsRef<std::path::Path>) -> Result<Opti
}

#[derive(Serialize, Deserialize)]
struct Metadata {
struct Metadata<'a> {
target: String,
shared_flags: BTreeMap<String, FlagValue>,
isa_flags: BTreeMap<String, FlagValue>,
#[serde(borrow)]
shared_flags: Vec<(&'a str, FlagValue<'a>)>,
#[serde(borrow)]
isa_flags: Vec<(&'a str, FlagValue<'a>)>,
tunables: Tunables,
features: WasmFeatures,
}
Expand All @@ -200,9 +201,9 @@ struct WasmFeatures {
function_references: bool,
}

impl Metadata {
impl Metadata<'_> {
#[cfg(any(feature = "cranelift", feature = "winch"))]
fn new(engine: &Engine) -> Metadata {
fn new(engine: &Engine) -> Metadata<'static> {
let wasmparser::WasmFeatures {
reference_types,
multi_value,
Expand Down Expand Up @@ -533,10 +534,9 @@ mod test {
let engine = Engine::default();
let mut metadata = Metadata::new(&engine);

metadata.shared_flags.insert(
"preserve_frame_pointers".to_string(),
FlagValue::Bool(false),
);
metadata
.shared_flags
.push(("preserve_frame_pointers", FlagValue::Bool(false)));

match metadata.check_compatible(&engine) {
Ok(_) => unreachable!(),
Expand All @@ -559,7 +559,7 @@ Caused by:

metadata
.isa_flags
.insert("not_a_flag".to_string(), FlagValue::Bool(true));
.push(("not_a_flag", FlagValue::Bool(true)));

match metadata.check_compatible(&engine) {
Ok(_) => unreachable!(),
Expand Down
4 changes: 2 additions & 2 deletions crates/winch/src/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,11 @@ impl wasmtime_environ::Compiler for Compiler {
self.isa.triple()
}

fn flags(&self) -> std::collections::BTreeMap<String, wasmtime_environ::FlagValue> {
fn flags(&self) -> Vec<(&'static str, wasmtime_environ::FlagValue<'static>)> {
wasmtime_cranelift_shared::clif_flags_to_wasmtime(self.isa.flags().iter())
}

fn isa_flags(&self) -> std::collections::BTreeMap<String, wasmtime_environ::FlagValue> {
fn isa_flags(&self) -> Vec<(&'static str, wasmtime_environ::FlagValue<'static>)> {
wasmtime_cranelift_shared::clif_flags_to_wasmtime(self.isa.isa_flags())
}

Expand Down
2 changes: 1 addition & 1 deletion src/commands/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ struct Settings {
bools: Vec<SettingData>,
presets: Vec<SettingData>,

inferred: Option<Vec<String>>,
inferred: Option<Vec<&'static str>>,
}

impl Settings {
Expand Down

0 comments on commit 61e11a6

Please sign in to comment.