Skip to content

Commit

Permalink
Change how flags are stored in serialized modules.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
peterhuene committed Apr 2, 2021
1 parent 4ad0099 commit 0ddfe97
Show file tree
Hide file tree
Showing 17 changed files with 193 additions and 185 deletions.
58 changes: 9 additions & 49 deletions cranelift/codegen/meta/src/gen_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Item = &'static str> {");
fmt.doc_comment("Iterates the setting values.");
fmtln!(fmt, "pub fn iter(&self) -> impl Iterator<Item = Value> {");
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, "})");
});
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
13 changes: 3 additions & 10 deletions cranelift/codegen/src/isa/aarch64/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -102,15 +102,8 @@ impl MachBackend for AArch64Backend {
&self.flags
}

fn enabled_isa_flags(&self) -> Vec<String> {
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<shared_settings::Value> {
self.isa_flags.iter().collect()
}

fn hash_all_flags(&self, mut hasher: &mut dyn Hasher) {
Expand Down
2 changes: 1 addition & 1 deletion cranelift/codegen/src/isa/aarch64/settings.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down
8 changes: 2 additions & 6 deletions cranelift/codegen/src/isa/arm32/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -92,14 +92,10 @@ impl MachBackend for Arm32Backend {
&self.flags
}

fn enabled_isa_flags(&self) -> Vec<String> {
fn isa_flags(&self) -> Vec<settings::Value> {
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);
}
Expand Down
9 changes: 3 additions & 6 deletions cranelift/codegen/src/isa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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<String>;

/// 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<settings::Value>;

/// Hashes all flags, both ISA-independent and ISA-dependent, into the specified hasher.
fn hash_all_flags(&self, hasher: &mut dyn Hasher);
Expand Down
18 changes: 3 additions & 15 deletions cranelift/codegen/src/isa/riscv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -74,15 +69,8 @@ impl TargetIsa for Isa {
&self.shared_flags
}

fn enabled_isa_flags(&self) -> Vec<String> {
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<shared_settings::Value> {
self.isa_flags.iter().collect()
}

fn hash_all_flags(&self, mut hasher: &mut dyn Hasher) {
Expand Down
2 changes: 1 addition & 1 deletion cranelift/codegen/src/isa/riscv/settings.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down
13 changes: 3 additions & 10 deletions cranelift/codegen/src/isa/x64/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -85,15 +85,8 @@ impl MachBackend for X64Backend {
&self.flags
}

fn enabled_isa_flags(&self) -> Vec<String> {
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<shared_settings::Value> {
self.x64_flags.iter().collect()
}

fn hash_all_flags(&self, mut hasher: &mut dyn Hasher) {
Expand Down
2 changes: 1 addition & 1 deletion cranelift/codegen/src/isa/x64/settings.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down
18 changes: 3 additions & 15 deletions cranelift/codegen/src/isa/x86/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -83,15 +78,8 @@ impl TargetIsa for Isa {
&self.shared_flags
}

fn enabled_isa_flags(&self) -> Vec<String> {
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<shared_settings::Value> {
self.isa_flags.iter().collect()
}

fn hash_all_flags(&self, mut hasher: &mut dyn Hasher) {
Expand Down
2 changes: 1 addition & 1 deletion cranelift/codegen/src/isa/x86/settings.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down
10 changes: 3 additions & 7 deletions cranelift/codegen/src/machinst/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -58,12 +58,8 @@ impl TargetIsa for TargetIsaAdapter {
self.backend.flags()
}

fn enabled_isa_flags(&self) -> Vec<String> {
self.backend.enabled_isa_flags()
}

fn is_flag_enabled(&self, flag: &str) -> bool {
self.backend.is_flag_enabled(flag)
fn isa_flags(&self) -> Vec<settings::Value> {
self.backend.isa_flags()
}

fn hash_all_flags(&self, hasher: &mut dyn Hasher) {
Expand Down
9 changes: 3 additions & 6 deletions cranelift/codegen/src/machinst/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String>;

/// 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<settings::Value>;

/// Hashes all flags, both ISA-independent and ISA-dependent, into the specified hasher.
fn hash_all_flags(&self, hasher: &mut dyn Hasher);
Expand Down
44 changes: 44 additions & 0 deletions cranelift/codegen/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<u8> {
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<bool> {
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 {
Expand Down
2 changes: 1 addition & 1 deletion crates/environ/src/data_structures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
}

Expand Down
Loading

0 comments on commit 0ddfe97

Please sign in to comment.