From 551f1c3a14e7568d368b7a1a5b7720ddad56375a Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 29 Sep 2021 19:00:12 +0200 Subject: [PATCH 1/3] Remove BindParameter and Bindable --- .../codegen/meta/src/cdsl/instructions.rs | 214 ------------------ 1 file changed, 214 deletions(-) diff --git a/cranelift/codegen/meta/src/cdsl/instructions.rs b/cranelift/codegen/meta/src/cdsl/instructions.rs index 72979f9350..a2d2f570f7 100644 --- a/cranelift/codegen/meta/src/cdsl/instructions.rs +++ b/cranelift/codegen/meta/src/cdsl/instructions.rs @@ -1,18 +1,14 @@ use cranelift_entity::{entity_impl, PrimaryMap}; use std::fmt; -use std::fmt::{Display, Error, Formatter}; use std::rc::Rc; use crate::cdsl::camel_case; use crate::cdsl::formats::InstructionFormat; use crate::cdsl::operands::Operand; use crate::cdsl::type_inference::Constraint; -use crate::cdsl::types::{LaneType, ReferenceType, ValueType}; use crate::cdsl::typevar::TypeVar; -use crate::shared::types::{Bool, Float, Int, Reference}; - #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] pub(crate) struct OpcodeNumber(u32); entity_impl!(OpcodeNumber); @@ -35,14 +31,6 @@ impl<'all_inst> InstructionGroupBuilder<'all_inst> { } } -/// Instructions can have parameters bound to them to specialize them for more specific encodings -/// (e.g. the encoding for adding two float types may be different than that of adding two -/// integer types) -pub(crate) trait Bindable { - /// Bind a parameter to an instruction - fn bind(&self, parameter: impl Into) -> BoundInstruction; -} - #[derive(Debug)] pub(crate) struct PolymorphicInfo { pub use_typevar_operand: bool, @@ -119,12 +107,6 @@ impl InstructionContent { pub(crate) type Instruction = Rc; -impl Bindable for Instruction { - fn bind(&self, parameter: impl Into) -> BoundInstruction { - BoundInstruction::new(self).bind(parameter) - } -} - impl fmt::Display for InstructionContent { fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> { if !self.operands_out.is_empty() { @@ -336,127 +318,6 @@ impl InstructionBuilder { } } -/// An parameter used for binding instructions to specific types or values -pub(crate) enum BindParameter { - Lane(LaneType), - Reference(ReferenceType), -} - -impl From for BindParameter { - fn from(ty: Int) -> Self { - BindParameter::Lane(ty.into()) - } -} - -impl From for BindParameter { - fn from(ty: Bool) -> Self { - BindParameter::Lane(ty.into()) - } -} - -impl From for BindParameter { - fn from(ty: Float) -> Self { - BindParameter::Lane(ty.into()) - } -} - -impl From for BindParameter { - fn from(ty: LaneType) -> Self { - BindParameter::Lane(ty) - } -} - -impl From for BindParameter { - fn from(ty: Reference) -> Self { - BindParameter::Reference(ty.into()) - } -} - -#[derive(Clone)] -pub(crate) enum Immediate {} - -impl Display for Immediate { - fn fmt(&self, _f: &mut Formatter) -> Result<(), Error> { - match self { - _ => panic!(), - } - } -} - -#[derive(Clone)] -pub(crate) struct BoundInstruction { - pub inst: Instruction, - pub value_types: Vec, - pub immediate_values: Vec, -} - -impl BoundInstruction { - /// Construct a new bound instruction (with nothing bound yet) from an instruction - fn new(inst: &Instruction) -> Self { - BoundInstruction { - inst: inst.clone(), - value_types: vec![], - immediate_values: vec![], - } - } - - /// Verify that the bindings for a BoundInstruction are correct. - fn verify_bindings(&self) -> Result<(), String> { - // Verify that binding types to the instruction does not violate the polymorphic rules. - if !self.value_types.is_empty() { - match &self.inst.polymorphic_info { - Some(poly) => { - if self.value_types.len() > 1 + poly.other_typevars.len() { - return Err(format!( - "trying to bind too many types for {}", - self.inst.name - )); - } - } - None => { - return Err(format!( - "trying to bind a type for {} which is not a polymorphic instruction", - self.inst.name - )); - } - } - } - - // Verify that only the right number of immediates are bound. - let immediate_count = self - .inst - .operands_in - .iter() - .filter(|o| o.is_immediate_or_entityref()) - .count(); - if self.immediate_values.len() > immediate_count { - return Err(format!( - "trying to bind too many immediates ({}) to instruction {} which only expects {} \ - immediates", - self.immediate_values.len(), - self.inst.name, - immediate_count - )); - } - - Ok(()) - } -} - -impl Bindable for BoundInstruction { - fn bind(&self, parameter: impl Into) -> BoundInstruction { - let mut modified = self.clone(); - match parameter.into() { - BindParameter::Lane(lane_type) => modified.value_types.push(lane_type.into()), - BindParameter::Reference(reference_type) => { - modified.value_types.push(reference_type.into()); - } - } - modified.verify_bindings().unwrap(); - modified - } -} - /// Checks that the input operands actually match the given format. fn verify_format(inst_name: &str, operands_in: &[Operand], format: &InstructionFormat) { // A format is defined by: @@ -661,78 +522,3 @@ fn is_ctrl_typevar_candidate( Ok(other_typevars) } - -#[cfg(test)] -mod test { - use super::*; - use crate::cdsl::formats::InstructionFormatBuilder; - use crate::cdsl::operands::{OperandKind, OperandKindFields}; - use crate::cdsl::typevar::TypeSetBuilder; - use crate::shared::types::Int::{I32, I64}; - - fn field_to_operand(index: usize, field: OperandKindFields) -> Operand { - // Pretend the index string is &'static. - let name = Box::leak(index.to_string().into_boxed_str()); - // Format's name / rust_type don't matter here. - let kind = OperandKind::new(name, name, field); - let operand = Operand::new(name, kind); - operand - } - - fn field_to_operands(types: Vec) -> Vec { - types - .iter() - .enumerate() - .map(|(i, f)| field_to_operand(i, f.clone())) - .collect() - } - - fn build_fake_instruction( - inputs: Vec, - outputs: Vec, - ) -> Instruction { - // Setup a format from the input operands. - let mut format = InstructionFormatBuilder::new("fake"); - for (i, f) in inputs.iter().enumerate() { - match f { - OperandKindFields::TypeVar(_) => format = format.value(), - OperandKindFields::ImmValue => { - format = format.imm(&field_to_operand(i, f.clone()).kind) - } - _ => {} - }; - } - let format = format.build(); - - // Create the fake instruction. - InstructionBuilder::new("fake", "A fake instruction for testing.", &format) - .operands_in(field_to_operands(inputs).iter().collect()) - .operands_out(field_to_operands(outputs).iter().collect()) - .build(OpcodeNumber(42)) - } - - #[test] - fn ensure_bound_instructions_can_bind_lane_types() { - let type1 = TypeSetBuilder::new().ints(8..64).build(); - let in1 = OperandKindFields::TypeVar(TypeVar::new("a", "...", type1)); - let inst = build_fake_instruction(vec![in1], vec![]); - inst.bind(LaneType::Int(I32)); - } - - #[test] - #[should_panic] - fn ensure_instructions_fail_to_bind() { - let inst = build_fake_instruction(vec![], vec![]); - inst.bind(BindParameter::Lane(LaneType::Int(I32))); - // Trying to bind to an instruction with no inputs should fail. - } - - #[test] - #[should_panic] - fn ensure_bound_instructions_fail_to_bind_too_many_types() { - let type1 = TypeSetBuilder::new().ints(8..64).build(); - let in1 = OperandKindFields::TypeVar(TypeVar::new("a", "...", type1)); - let inst = build_fake_instruction(vec![in1], vec![]); - inst.bind(LaneType::Int(I32)).bind(LaneType::Int(I64)); - } -} From eb01ba1ed179af1f5b3b0aea06f86b135059630d Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 29 Sep 2021 19:06:12 +0200 Subject: [PATCH 2/3] Flatten directory structure for cranelift_codegen_meta::isa --- .../codegen/meta/src/isa/{arm32/mod.rs => arm32.rs} | 0 .../codegen/meta/src/isa/{arm64/mod.rs => arm64.rs} | 0 .../codegen/meta/src/isa/{s390x/mod.rs => s390x.rs} | 0 .../codegen/meta/src/isa/{x86/settings.rs => x86.rs} | 11 ++++++++++- cranelift/codegen/meta/src/isa/x86/mod.rs | 11 ----------- cranelift/codegen/meta/src/lib.rs | 2 +- 6 files changed, 11 insertions(+), 13 deletions(-) rename cranelift/codegen/meta/src/isa/{arm32/mod.rs => arm32.rs} (100%) rename cranelift/codegen/meta/src/isa/{arm64/mod.rs => arm64.rs} (100%) rename cranelift/codegen/meta/src/isa/{s390x/mod.rs => s390x.rs} (100%) rename cranelift/codegen/meta/src/isa/{x86/settings.rs => x86.rs} (95%) delete mode 100644 cranelift/codegen/meta/src/isa/x86/mod.rs diff --git a/cranelift/codegen/meta/src/isa/arm32/mod.rs b/cranelift/codegen/meta/src/isa/arm32.rs similarity index 100% rename from cranelift/codegen/meta/src/isa/arm32/mod.rs rename to cranelift/codegen/meta/src/isa/arm32.rs diff --git a/cranelift/codegen/meta/src/isa/arm64/mod.rs b/cranelift/codegen/meta/src/isa/arm64.rs similarity index 100% rename from cranelift/codegen/meta/src/isa/arm64/mod.rs rename to cranelift/codegen/meta/src/isa/arm64.rs diff --git a/cranelift/codegen/meta/src/isa/s390x/mod.rs b/cranelift/codegen/meta/src/isa/s390x.rs similarity index 100% rename from cranelift/codegen/meta/src/isa/s390x/mod.rs rename to cranelift/codegen/meta/src/isa/s390x.rs diff --git a/cranelift/codegen/meta/src/isa/x86/settings.rs b/cranelift/codegen/meta/src/isa/x86.rs similarity index 95% rename from cranelift/codegen/meta/src/isa/x86/settings.rs rename to cranelift/codegen/meta/src/isa/x86.rs index 824683bbf6..2a63ef147d 100644 --- a/cranelift/codegen/meta/src/isa/x86/settings.rs +++ b/cranelift/codegen/meta/src/isa/x86.rs @@ -1,6 +1,15 @@ +use crate::cdsl::isa::TargetIsa; use crate::cdsl::settings::{PredicateNode, SettingGroup, SettingGroupBuilder}; -pub(crate) fn define(shared: &SettingGroup) -> SettingGroup { +use crate::shared::Definitions as SharedDefinitions; + +pub(crate) fn define(shared_defs: &mut SharedDefinitions) -> TargetIsa { + let settings = define_settings(&shared_defs.settings); + + TargetIsa::new("x86", settings) +} + +pub(crate) fn define_settings(shared: &SettingGroup) -> SettingGroup { let mut settings = SettingGroupBuilder::new("x86"); // CPUID.01H:ECX diff --git a/cranelift/codegen/meta/src/isa/x86/mod.rs b/cranelift/codegen/meta/src/isa/x86/mod.rs deleted file mode 100644 index b4c670fce7..0000000000 --- a/cranelift/codegen/meta/src/isa/x86/mod.rs +++ /dev/null @@ -1,11 +0,0 @@ -use crate::cdsl::isa::TargetIsa; - -use crate::shared::Definitions as SharedDefinitions; - -pub(crate) mod settings; - -pub(crate) fn define(shared_defs: &mut SharedDefinitions) -> TargetIsa { - let settings = settings::define(&shared_defs.settings); - - TargetIsa::new("x86", settings) -} diff --git a/cranelift/codegen/meta/src/lib.rs b/cranelift/codegen/meta/src/lib.rs index 77fcbc7bf9..d54e490f6c 100644 --- a/cranelift/codegen/meta/src/lib.rs +++ b/cranelift/codegen/meta/src/lib.rs @@ -70,7 +70,7 @@ pub fn generate( continue; } - let settings = crate::isa::x86::settings::define(&shared_defs.settings); + let settings = crate::isa::x86::define_settings(&shared_defs.settings); gen_settings::generate( &settings, gen_settings::ParentGroup::Shared, From d590e6bc1f2dc7f15fa38e7a258f7d8ddb420b55 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 29 Sep 2021 19:12:51 +0200 Subject: [PATCH 3/3] Remove x86 old-backend special case from cranelift-codegen-meta --- cranelift/codegen/build.rs | 26 ++++---------------- cranelift/codegen/meta/src/isa/x86.rs | 2 +- cranelift/codegen/meta/src/lib.rs | 34 ++------------------------- 3 files changed, 7 insertions(+), 55 deletions(-) diff --git a/cranelift/codegen/build.rs b/cranelift/codegen/build.rs index dd6cfc999c..10def102be 100644 --- a/cranelift/codegen/build.rs +++ b/cranelift/codegen/build.rs @@ -27,15 +27,6 @@ fn main() { let out_dir = env::var("OUT_DIR").expect("The OUT_DIR environment variable must be set"); let target_triple = env::var("TARGET").expect("The TARGET environment variable must be set"); - let new_backend_isas = if env::var("CARGO_FEATURE_X64").is_ok() { - // The x64 (new backend for x86_64) is a bit particular: it only requires generating - // the shared meta code; the only ISA-specific code is for settings. - vec![meta::isa::Isa::X86] - } else { - Vec::new() - }; - - // Configure isa targets using the old backend. let isa_targets = meta::isa::Isa::all() .iter() .cloned() @@ -45,7 +36,7 @@ fn main() { }) .collect::>(); - let old_backend_isas = if new_backend_isas.is_empty() && isa_targets.is_empty() { + let isas = if isa_targets.is_empty() { // Try to match native target. let target_name = target_triple.split('-').next().unwrap(); let isa = meta::isa_from_arch(&target_name).expect("error when identifying target"); @@ -65,23 +56,14 @@ fn main() { crate_dir.join("build.rs").to_str().unwrap() ); - if let Err(err) = meta::generate(&old_backend_isas, &new_backend_isas, &out_dir) { + if let Err(err) = meta::generate(&isas, &out_dir) { eprintln!("Error: {}", err); process::exit(1); } if env::var("CRANELIFT_VERBOSE").is_ok() { - for isa in &old_backend_isas { - println!( - "cargo:warning=Includes old-backend support for {} ISA", - isa.to_string() - ); - } - for isa in &new_backend_isas { - println!( - "cargo:warning=Includes new-backend support for {} ISA", - isa.to_string() - ); + for isa in &isas { + println!("cargo:warning=Includes support for {} ISA", isa.to_string()); } println!( "cargo:warning=Build step took {:?}.", diff --git a/cranelift/codegen/meta/src/isa/x86.rs b/cranelift/codegen/meta/src/isa/x86.rs index 2a63ef147d..eec6ac105f 100644 --- a/cranelift/codegen/meta/src/isa/x86.rs +++ b/cranelift/codegen/meta/src/isa/x86.rs @@ -9,7 +9,7 @@ pub(crate) fn define(shared_defs: &mut SharedDefinitions) -> TargetIsa { TargetIsa::new("x86", settings) } -pub(crate) fn define_settings(shared: &SettingGroup) -> SettingGroup { +fn define_settings(shared: &SettingGroup) -> SettingGroup { let mut settings = SettingGroupBuilder::new("x86"); // CPUID.01H:ECX diff --git a/cranelift/codegen/meta/src/lib.rs b/cranelift/codegen/meta/src/lib.rs index d54e490f6c..20815ef8d2 100644 --- a/cranelift/codegen/meta/src/lib.rs +++ b/cranelift/codegen/meta/src/lib.rs @@ -21,11 +21,7 @@ pub fn isa_from_arch(arch: &str) -> Result { } /// Generates all the Rust source files used in Cranelift from the meta-language. -pub fn generate( - old_backend_isas: &[isa::Isa], - new_backend_isas: &[isa::Isa], - out_dir: &str, -) -> Result<(), error::Error> { +pub fn generate(isas: &[isa::Isa], out_dir: &str) -> Result<(), error::Error> { // Create all the definitions: // - common definitions. let mut shared_defs = shared::define(); @@ -39,7 +35,7 @@ pub fn generate( gen_types::generate("types.rs", &out_dir)?; // - per ISA definitions. - let target_isas = isa::define(old_backend_isas, &mut shared_defs); + let target_isas = isa::define(isas, &mut shared_defs); // At this point, all definitions are done. let all_formats = shared_defs.verify_instruction_formats(); @@ -62,31 +58,5 @@ pub fn generate( )?; } - for isa in new_backend_isas { - match isa { - isa::Isa::X86 => { - // If the old backend ISAs contained x86, this file has already been generated. - if old_backend_isas.iter().any(|isa| *isa == isa::Isa::X86) { - continue; - } - - let settings = crate::isa::x86::define_settings(&shared_defs.settings); - gen_settings::generate( - &settings, - gen_settings::ParentGroup::Shared, - "settings-x86.rs", - &out_dir, - )?; - } - isa::Isa::Arm64 => { - // aarch64 doesn't have platform-specific settings. - } - isa::Isa::S390x => { - // s390x doesn't have platform-specific settings. - } - isa::Isa::Arm32 => todo!(), - } - } - Ok(()) }