A bunch of cleanups for cranelift-codegen-meta

cranelift/codegen/Cargo.toml

@@ -61,10 +61,6 @@ core = []
 # context, and the `disassemble` method on `MachBufferFinalized`.
 disas = ["anyhow", "capstone"]
 
-# This enables some additional functions useful for writing tests, but which
-# can significantly increase the size of the library.
-testing_hooks = []
-
 # Enables detailed logging which can be somewhat expensive.
 trace-log = []
 

cranelift/codegen/meta/Cargo.toml

@@ -8,9 +8,8 @@ repository = "https://github.com/bytecodealliance/wasmtime"
 readme = "README.md"
 edition.workspace = true
 
-# FIXME(rust-lang/cargo#9300): uncomment once that lands
-# [package.metadata.docs.rs]
-# rustdoc-args = [ "--document-private-items" ]
+[package.metadata.docs.rs]
+rustdoc-args = [ "--document-private-items" ]
 
 [dependencies]
 cranelift-codegen-shared = { path = "../shared", version = "0.99.0" }

cranelift/codegen/meta/src/gen_inst.rs

@@ -1,5 +1,6 @@
 //! Generate instruction data (including opcodes, formats, builders, etc.).
 use std::fmt;
+use std::rc::Rc;
 
 use cranelift_codegen_shared::constant_hash;
 
@@ -17,7 +18,7 @@ use crate::unique_table::{UniqueSeqTable, UniqueTable};
 const TYPESET_LIMIT: usize = 0xff;
 
 /// Generate an instruction format enumeration.
-fn gen_formats(formats: &[&InstructionFormat], fmt: &mut Formatter) {
+fn gen_formats(formats: &[Rc<InstructionFormat>], fmt: &mut Formatter) {
     fmt.doc_comment(
         r#"
         An instruction format
@@ -65,7 +66,7 @@ fn gen_formats(formats: &[&InstructionFormat], fmt: &mut Formatter) {
 /// Every variant must contain an `opcode` field. The size of `InstructionData` should be kept at
 /// 16 bytes on 64-bit architectures. If more space is needed to represent an instruction, use a
 /// `ValueList` to store the additional information out of line.
-fn gen_instruction_data(formats: &[&InstructionFormat], fmt: &mut Formatter) {
+fn gen_instruction_data(formats: &[Rc<InstructionFormat>], fmt: &mut Formatter) {
     fmt.line("#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]");
     fmt.line(r#"#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))]"#);
     fmt.line("#[allow(missing_docs)]");
@@ -104,7 +105,7 @@ fn gen_instruction_data(formats: &[&InstructionFormat], fmt: &mut Formatter) {
     fmt.line("}");
 }
 
-fn gen_arguments_method(formats: &[&InstructionFormat], fmt: &mut Formatter, is_mut: bool) {
+fn gen_arguments_method(formats: &[Rc<InstructionFormat>], fmt: &mut Formatter, is_mut: bool) {
     let (method, mut_, rslice, as_slice) = if is_mut {
         (
             "arguments_mut",
@@ -172,7 +173,7 @@ fn gen_arguments_method(formats: &[&InstructionFormat], fmt: &mut Formatter, is_
 /// - `pub fn arguments_mut(&mut self, &pool) -> &mut [Value]`
 /// - `pub fn eq(&self, &other: Self, &pool) -> bool`
 /// - `pub fn hash<H: Hasher>(&self, state: &mut H, &pool)`
-fn gen_instruction_data_impl(formats: &[&InstructionFormat], fmt: &mut Formatter) {
+fn gen_instruction_data_impl(formats: &[Rc<InstructionFormat>], fmt: &mut Formatter) {
     fmt.line("impl InstructionData {");
     fmt.indent(|fmt| {
         fmt.doc_comment("Get the opcode of this instruction.");
@@ -1221,7 +1222,7 @@ enum IsleTarget {
 }
 
 fn gen_common_isle(
-    formats: &[&InstructionFormat],
+    formats: &[Rc<InstructionFormat>],
     instructions: &AllInstructions,
     fmt: &mut Formatter,
     isle_target: IsleTarget,
@@ -1669,15 +1670,15 @@ fn gen_common_isle(
 }
 
 fn gen_opt_isle(
-    formats: &[&InstructionFormat],
+    formats: &[Rc<InstructionFormat>],
     instructions: &AllInstructions,
     fmt: &mut Formatter,
 ) {
     gen_common_isle(formats, instructions, fmt, IsleTarget::Opt);
 }
 
 fn gen_lower_isle(
-    formats: &[&InstructionFormat],
+    formats: &[Rc<InstructionFormat>],
     instructions: &AllInstructions,
     fmt: &mut Formatter,
 ) {
@@ -1707,7 +1708,7 @@ fn gen_isle_enum(name: &str, mut variants: Vec<&str>, fmt: &mut Formatter) {
 /// Generate a Builder trait with methods for all instructions.
 fn gen_builder(
     instructions: &AllInstructions,
-    formats: &[&InstructionFormat],
+    formats: &[Rc<InstructionFormat>],
     fmt: &mut Formatter,
 ) {
     fmt.doc_comment(
@@ -1744,7 +1745,7 @@ fn gen_builder(
 }
 
 pub(crate) fn generate(
-    formats: Vec<&InstructionFormat>,
+    formats: &[Rc<InstructionFormat>],
     all_inst: &AllInstructions,
     opcode_filename: &str,
     inst_builder_filename: &str,

cranelift/codegen/meta/src/isa/arm64.rs

@@ -1,33 +1,31 @@
 use crate::cdsl::isa::TargetIsa;
-use crate::cdsl::settings::{SettingGroup, SettingGroupBuilder};
+use crate::cdsl::settings::SettingGroupBuilder;
 
-use crate::shared::Definitions as SharedDefinitions;
+pub(crate) fn define() -> TargetIsa {
+    let mut settings = SettingGroupBuilder::new("arm64");
 
-fn define_settings(_shared: &SettingGroup) -> SettingGroup {
-    let mut setting = SettingGroupBuilder::new("arm64");
-
-    setting.add_bool(
+    settings.add_bool(
         "has_lse",
         "Has Large System Extensions (FEAT_LSE) support.",
         "",
         false,
     );
-    setting.add_bool(
+    settings.add_bool(
         "has_pauth",
         "Has Pointer authentication (FEAT_PAuth) support; enables the use of \
          non-HINT instructions, but does not have an effect on code generation \
          by itself.",
         "",
         false,
     );
-    setting.add_bool(
+    settings.add_bool(
         "sign_return_address_all",
         "If function return address signing is enabled, then apply it to all \
         functions; does not have an effect on code generation by itself.",
         "",
         false,
     );
-    setting.add_bool(
+    settings.add_bool(
         "sign_return_address",
         "Use pointer authentication instructions to sign function return \
          addresses; HINT-space instructions using the A key are generated \
@@ -36,26 +34,20 @@ fn define_settings(_shared: &SettingGroup) -> SettingGroup {
         "",
         false,
     );
-    setting.add_bool(
+    settings.add_bool(
         "sign_return_address_with_bkey",
         "Use the B key with pointer authentication instructions instead of \
         the default A key; does not have an effect on code generation by \
         itself. Some platform ABIs may require this, for example.",
         "",
         false,
     );
-    setting.add_bool(
+    settings.add_bool(
         "use_bti",
         "Use Branch Target Identification (FEAT_BTI) instructions.",
         "",
         false,
     );
 
-    setting.build()
-}
-
-pub(crate) fn define(shared_defs: &mut SharedDefinitions) -> TargetIsa {
-    let settings = define_settings(&shared_defs.settings);
-
-    TargetIsa::new("arm64", settings)
+    TargetIsa::new("arm64", settings.build())
 }

cranelift/codegen/meta/src/isa/mod.rs

@@ -1,6 +1,5 @@
 //! Define supported ISAs; includes ISA-specific instructions, encodings, registers, settings, etc.
 use crate::cdsl::isa::TargetIsa;
-use crate::shared::Definitions as SharedDefinitions;
 use std::fmt;
 
 mod arm64;
@@ -55,13 +54,13 @@ impl fmt::Display for Isa {
     }
 }
 
-pub(crate) fn define(isas: &[Isa], shared_defs: &mut SharedDefinitions) -> Vec<TargetIsa> {
+pub(crate) fn define(isas: &[Isa]) -> Vec<TargetIsa> {
     isas.iter()
         .map(|isa| match isa {
-            Isa::X86 => x86::define(shared_defs),
-            Isa::Arm64 => arm64::define(shared_defs),
-            Isa::S390x => s390x::define(shared_defs),
-            Isa::Riscv64 => riscv64::define(shared_defs),
+            Isa::X86 => x86::define(),
+            Isa::Arm64 => arm64::define(),
+            Isa::S390x => s390x::define(),
+            Isa::Riscv64 => riscv64::define(),
         })
         .collect()
 }

cranelift/codegen/meta/src/isa/riscv64.rs

@@ -1,7 +1,5 @@
 use crate::cdsl::isa::TargetIsa;
-use crate::cdsl::settings::{SettingGroup, SettingGroupBuilder};
-
-use crate::shared::Definitions as SharedDefinitions;
+use crate::cdsl::settings::SettingGroupBuilder;
 
 macro_rules! define_zvl_ext {
     (DEF: $settings:expr, $size:expr) => {{
@@ -27,7 +25,7 @@ macro_rules! define_zvl_ext {
     }};
 }
 
-fn define_settings(_shared: &SettingGroup) -> SettingGroup {
+pub(crate) fn define() -> TargetIsa {
     let mut setting = SettingGroupBuilder::new("riscv64");
 
     let _has_m = setting.add_bool("has_m", "has extension M?", "", false);
@@ -99,10 +97,5 @@ fn define_settings(_shared: &SettingGroup) -> SettingGroup {
     let (_, zvl32768b) = define_zvl_ext!(setting, 32768, zvl16384b);
     let (_, _zvl65536b) = define_zvl_ext!(setting, 65536, zvl32768b);
 
-    setting.build()
-}
-
-pub(crate) fn define(shared_defs: &mut SharedDefinitions) -> TargetIsa {
-    let settings = define_settings(&shared_defs.settings);
-    TargetIsa::new("riscv64", settings)
+    TargetIsa::new("riscv64", setting.build())
 }

cranelift/codegen/meta/src/isa/s390x.rs

@@ -1,9 +1,7 @@
 use crate::cdsl::isa::TargetIsa;
-use crate::cdsl::settings::{SettingGroup, SettingGroupBuilder};
+use crate::cdsl::settings::SettingGroupBuilder;
 
-use crate::shared::Definitions as SharedDefinitions;
-
-fn define_settings(_shared: &SettingGroup) -> SettingGroup {
+pub(crate) fn define() -> TargetIsa {
     let mut settings = SettingGroupBuilder::new("s390x");
 
     // The baseline architecture for cranelift is z14 (arch12),
@@ -37,11 +35,5 @@ fn define_settings(_shared: &SettingGroup) -> SettingGroup {
         preset!(has_mie2 && has_vxrs_ext2),
     );
 
-    settings.build()
-}
-
-pub(crate) fn define(shared_defs: &mut SharedDefinitions) -> TargetIsa {
-    let settings = define_settings(&shared_defs.settings);
-
-    TargetIsa::new("s390x", settings)
+    TargetIsa::new("s390x", settings.build())
 }

cranelift/codegen/meta/src/isa/x86.rs

@@ -1,15 +1,7 @@
 use crate::cdsl::isa::TargetIsa;
-use crate::cdsl::settings::{PredicateNode, SettingGroup, SettingGroupBuilder};
+use crate::cdsl::settings::{PredicateNode, SettingGroupBuilder};
 
-use crate::shared::Definitions as SharedDefinitions;
-
-pub(crate) fn define(_shared_defs: &mut SharedDefinitions) -> TargetIsa {
-    let settings = define_settings();
-
-    TargetIsa::new("x86", settings)
-}
-
-fn define_settings() -> SettingGroup {
+pub(crate) fn define() -> TargetIsa {
     let mut settings = SettingGroupBuilder::new("x86");
 
     // CPUID.01H:ECX
@@ -405,5 +397,5 @@ fn define_settings() -> SettingGroup {
         preset!(x86_64_v3 && has_avx512dq && has_avx512vl),
     );
 
-    settings.build()
+    TargetIsa::new("x86", settings.build())
 }

cranelift/codegen/meta/src/lib.rs

@@ -23,9 +23,8 @@ pub fn isa_from_arch(arch: &str) -> Result<isa::Isa, String> {
 
 /// Generates all the Rust source files used in Cranelift from the meta-language.
 pub fn generate(isas: &[isa::Isa], out_dir: &str, isle_dir: &str) -> Result<(), error::Error> {
-    // Create all the definitions:
-    // - common definitions.
-    let mut shared_defs = shared::define();
+    // Common definitions.
+    let shared_defs = shared::define();
 
     gen_settings::generate(
         &shared_defs.settings,
@@ -35,15 +34,8 @@ pub fn generate(isas: &[isa::Isa], out_dir: &str, isle_dir: &str) -> Result<(),
     )?;
     gen_types::generate("types.rs", out_dir)?;
 
-    // - per ISA definitions.
-    let target_isas = isa::define(isas, &mut shared_defs);
-
-    // At this point, all definitions are done.
-    let all_formats = shared_defs.verify_instruction_formats();
-
-    // Generate all the code.
     gen_inst::generate(
-        all_formats,
+        &shared_defs.all_formats,
         &shared_defs.all_instructions,
         "opcodes.rs",
         "inst_builder.rs",
@@ -53,7 +45,8 @@ pub fn generate(isas: &[isa::Isa], out_dir: &str, isle_dir: &str) -> Result<(),
         isle_dir,
     )?;
 
-    for isa in target_isas {
+    // Per ISA definitions.
+    for isa in isa::define(isas) {
         gen_settings::generate(
             &isa.settings,
             gen_settings::ParentGroup::Shared,