Merge pull request #3497 from bjorn3/misc_meta_simplifications

Misc meta crate cleanups

cranelift/codegen/meta/src/cdsl/formats.rs

@@ -84,32 +84,26 @@ impl InstructionFormat {
     }
 }
 
-pub(crate) struct InstructionFormatBuilder {
+pub(crate) struct InstructionFormatBuilder(InstructionFormat);
-    name: &'static str,
-    num_value_operands: usize,
-    has_value_list: bool,
-    imm_fields: Vec<FormatField>,
-    typevar_operand: Option<usize>,
-}
 
 impl InstructionFormatBuilder {
     pub fn new(name: &'static str) -> Self {
-        Self {
+        Self(InstructionFormat {
             name,
             num_value_operands: 0,
             has_value_list: false,
             imm_fields: Vec::new(),
             typevar_operand: None,
-        }
+        })
     }
 
     pub fn value(mut self) -> Self {
-        self.num_value_operands += 1;
+        self.0.num_value_operands += 1;
         self
     }
 
     pub fn varargs(mut self) -> Self {
-        self.has_value_list = true;
+        self.0.has_value_list = true;
         self
     }
 
@@ -118,42 +112,23 @@ impl InstructionFormatBuilder {
             kind: operand_kind.clone(),
             member: operand_kind.rust_field_name,
         };
-        self.imm_fields.push(field);
+        self.0.imm_fields.push(field);
-        self
-    }
-
-    pub fn imm_with_name(mut self, member: &'static str, operand_kind: &OperandKind) -> Self {
-        let field = FormatField {
-            kind: operand_kind.clone(),
-            member,
-        };
-        self.imm_fields.push(field);
         self
     }
 
     pub fn typevar_operand(mut self, operand_index: usize) -> Self {
-        assert!(self.typevar_operand.is_none());
+        assert!(self.0.typevar_operand.is_none());
-        assert!(self.has_value_list || operand_index < self.num_value_operands);
+        assert!(operand_index < self.0.num_value_operands);
-        self.typevar_operand = Some(operand_index);
+        self.0.typevar_operand = Some(operand_index);
         self
     }
 
-    pub fn build(self) -> Rc<InstructionFormat> {
+    pub fn build(mut self) -> Rc<InstructionFormat> {
-        let typevar_operand = if self.typevar_operand.is_some() {
+        if self.0.typevar_operand.is_none() && self.0.num_value_operands > 0 {
-            self.typevar_operand
-        } else if self.has_value_list || self.num_value_operands > 0 {
             // Default to the first value operand, if there's one.
-            Some(0)
+            self.0.typevar_operand = Some(0);
-        } else {
-            None
         };
 
-        Rc::new(InstructionFormat {
+        Rc::new(self.0)
-            name: self.name,
-            num_value_operands: self.num_value_operands,
-            has_value_list: self.has_value_list,
-            imm_fields: self.imm_fields,
-            typevar_operand,
-        })
     }
 }

cranelift/codegen/meta/src/cdsl/instructions.rs

@@ -43,7 +43,7 @@ pub(crate) struct InstructionContent {
     /// Output operands. The output operands must be SSA values or `variable_args`.
     pub operands_out: Vec<Operand>,
 
-    /// Instruction format, automatically derived from the input operands.
+    /// Instruction format.
     pub format: Rc<InstructionFormat>,
 
     /// One of the input or output operands is a free type variable. None if the instruction is not

cranelift/codegen/meta/src/cdsl/operands.rs

@@ -39,12 +39,12 @@ impl Operand {
         self
     }
 
-    pub fn doc(&self) -> Option<&str> {
+    pub fn doc(&self) -> &str {
         if let Some(doc) = &self.doc {
-            return Some(doc);
+            return doc;
         }
         match &self.kind.fields {
-            OperandKindFields::TypeVar(tvar) => Some(&tvar.doc),
+            OperandKindFields::TypeVar(tvar) => &tvar.doc,
             _ => self.kind.doc(),
         }
     }
@@ -130,40 +130,39 @@ impl OperandKind {
         rust_field_name: &'static str,
         rust_type: &'static str,
         fields: OperandKindFields,
+        doc: &'static str,
     ) -> Self {
         Self {
             rust_field_name,
             rust_type,
             fields,
-            doc: None,
+            doc: Some(doc),
         }
     }
-    pub fn with_doc(mut self, doc: &'static str) -> Self {
+    fn doc(&self) -> &str {
-        assert!(self.doc.is_none());
-        self.doc = Some(doc);
-        self
-    }
-    fn doc(&self) -> Option<&str> {
         if let Some(doc) = &self.doc {
-            return Some(doc);
+            return doc;
         }
         match &self.fields {
-            OperandKindFields::TypeVar(type_var) => Some(&type_var.doc),
+            OperandKindFields::TypeVar(type_var) => &type_var.doc,
+            // The only method to create an OperandKind with `doc` set to None is using a TypeVar,
+            // so all other options are unreachable here.
             OperandKindFields::ImmEnum(_)
             | OperandKindFields::ImmValue
             | OperandKindFields::EntityRef
-            | OperandKindFields::VariableArgs => None,
+            | OperandKindFields::VariableArgs => unreachable!(),
         }
     }
 }
 
 impl Into<OperandKind> for &TypeVar {
     fn into(self) -> OperandKind {
-        OperandKind::new(
+        OperandKind {
-            "value",
+            rust_field_name: "value",
-            "ir::Value",
+            rust_type: "ir::Value",
-            OperandKindFields::TypeVar(self.into()),
+            fields: OperandKindFields::TypeVar(self.into()),
-        )
+            doc: None,
+        }
     }
 }
 impl Into<OperandKind> for &OperandKind {

cranelift/codegen/meta/src/cdsl/types.rs

@@ -6,7 +6,7 @@ use crate::shared::types as shared_types;
 use cranelift_codegen_shared::constants;
 
 // Rust name prefix used for the `rust_name` method.
-static _RUST_NAME_PREFIX: &str = "ir::types::";
+static RUST_NAME_PREFIX: &str = "ir::types::";
 
 // ValueType variants (i8, i32, ...) are provided in `shared::types.rs`.
 
@@ -82,14 +82,7 @@ impl ValueType {
 
     /// Return the name of this type for generated Rust source files.
     pub fn rust_name(&self) -> String {
-        format!("{}{}", _RUST_NAME_PREFIX, self.to_string().to_uppercase())
+        format!("{}{}", RUST_NAME_PREFIX, self.to_string().to_uppercase())
-    }
-
-    /// Return true iff:
-    ///     1. self and other have equal number of lanes
-    ///     2. each lane in self has at least as many bits as a lane in other
-    pub fn _wider_or_equal(&self, rhs: &ValueType) -> bool {
-        (self.lane_count() == rhs.lane_count()) && (self.lane_bits() >= rhs.lane_bits())
     }
 
     /// Return the total number of bits of an instance of this type.

cranelift/codegen/meta/src/gen_inst.rs

@@ -75,14 +75,12 @@ fn gen_instruction_data(formats: &[&InstructionFormat], fmt: &mut Formatter) {
             fmtln!(fmt, "{} {{", format.name);
             fmt.indent(|fmt| {
                 fmt.line("opcode: Opcode,");
-                if format.typevar_operand.is_some() {
+                if format.has_value_list {
-                    if format.has_value_list {
+                    fmt.line("args: ValueList,");
-                        fmt.line("args: ValueList,");
+                } else if format.num_value_operands == 1 {
-                    } else if format.num_value_operands == 1 {
+                    fmt.line("arg: Value,");
-                        fmt.line("arg: Value,");
+                } else if format.num_value_operands > 0 {
-                    } else {
+                    fmtln!(fmt, "args: [Value; {}],", format.num_value_operands);
-                        fmtln!(fmt, "args: [Value; {}],", format.num_value_operands);
-                    }
                 }
                 for field in &format.imm_fields {
                     fmtln!(fmt, "{}: {},", field.member, field.kind.rust_type);
@@ -287,17 +285,17 @@ fn gen_instruction_data_impl(formats: &[&InstructionFormat], fmt: &mut Formatter
                     let name = format!("&Self::{}", format.name);
                     let mut members = vec!["opcode"];
 
-                    let args_eq = if format.typevar_operand.is_none() {
+                    let args_eq = if format.has_value_list {
-                        None
-                    } else if format.has_value_list {
                         members.push("args");
                         Some("args1.as_slice(pool) == args2.as_slice(pool)")
                     } else if format.num_value_operands == 1 {
                         members.push("arg");
                         Some("arg1 == arg2")
-                    } else {
+                    } else if format.num_value_operands > 0 {
                         members.push("args");
                         Some("args1 == args2")
+                    } else {
+                        None
                     };
 
                     for field in &format.imm_fields {
@@ -339,17 +337,17 @@ fn gen_instruction_data_impl(formats: &[&InstructionFormat], fmt: &mut Formatter
                     let name = format!("Self::{}", format.name);
                     let mut members = vec!["opcode"];
 
-                    let args = if format.typevar_operand.is_none() {
+                    let args = if format.has_value_list {
-                        "&()"
-                    } else if format.has_value_list {
                         members.push("ref args");
                         "args.as_slice(pool)"
                     } else if format.num_value_operands == 1 {
                         members.push("ref arg");
                         "arg"
-                    } else {
+                    } else if format.num_value_operands > 0{
                         members.push("ref args");
                         "args"
+                    } else {
+                        "&()"
                     };
 
                     for field in &format.imm_fields {
@@ -948,21 +946,11 @@ fn gen_inst_builder(inst: &Instruction, format: &InstructionFormat, fmt: &mut Fo
             op.kind.rust_type.to_string()
         };
         args.push(format!("{}: {}", op.name, t));
-        args_doc.push(format!(
+        args_doc.push(format!("- {}: {}", op.name, op.doc()));
-            "- {}: {}",
-            op.name,
-            op.doc()
-                .expect("every instruction's input operand must be documented")
-        ));
     }
 
     for op in &inst.operands_out {
-        rets_doc.push(format!(
+        rets_doc.push(format!("- {}: {}", op.name, op.doc()));
-            "- {}: {}",
-            op.name,
-            op.doc()
-                .expect("every instruction's output operand must be documented")
-        ));
     }
 
     let rtype = match inst.value_results.len() {

cranelift/codegen/meta/src/shared/entities.rs

@@ -2,7 +2,12 @@ use crate::cdsl::operands::{OperandKind, OperandKindFields};
 
 /// Small helper to initialize an OperandBuilder with the right kind, for a given name and doc.
 fn new(format_field_name: &'static str, rust_type: &'static str, doc: &'static str) -> OperandKind {
-    OperandKind::new(format_field_name, rust_type, OperandKindFields::EntityRef).with_doc(doc)
+    OperandKind::new(
+        format_field_name,
+        rust_type,
+        OperandKindFields::EntityRef,
+        doc,
+    )
 }
 
 pub(crate) struct EntityRefs {
@@ -59,7 +64,10 @@ impl EntityRefs {
 
             table: new("table", "ir::Table", "A table."),
 
-            varargs: OperandKind::new("", "&[Value]", OperandKindFields::VariableArgs).with_doc(
+            varargs: OperandKind::new(
+                "",
+                "&[Value]",
+                OperandKindFields::VariableArgs,
                 r#"
                         A variable size list of `value` operands.
 

cranelift/codegen/meta/src/shared/formats.rs

@@ -101,7 +101,7 @@ impl Formats {
             shuffle: Builder::new("Shuffle")
                 .value()
                 .value()
-                .imm_with_name("mask", &imm.uimm128)
+                .imm(&imm.uimm128)
                 .build(),
 
             int_compare: Builder::new("IntCompare")

cranelift/codegen/meta/src/shared/immediates.rs

@@ -73,40 +73,76 @@ pub(crate) struct Immediates {
     pub atomic_rmw_op: OperandKind,
 }
 
-fn new_imm(format_field_name: &'static str, rust_type: &'static str) -> OperandKind {
+fn new_imm(
-    OperandKind::new(format_field_name, rust_type, OperandKindFields::ImmValue)
+    format_field_name: &'static str,
+    rust_type: &'static str,
+    doc: &'static str,
+) -> OperandKind {
+    OperandKind::new(
+        format_field_name,
+        rust_type,
+        OperandKindFields::ImmValue,
+        doc,
+    )
 }
 fn new_enum(
     format_field_name: &'static str,
     rust_type: &'static str,
     values: EnumValues,
+    doc: &'static str,
 ) -> OperandKind {
     OperandKind::new(
         format_field_name,
         rust_type,
         OperandKindFields::ImmEnum(values),
+        doc,
     )
 }
 
 impl Immediates {
     pub fn new() -> Self {
         Self {
-            imm64: new_imm("imm", "ir::immediates::Imm64").with_doc("A 64-bit immediate integer."),
+            imm64: new_imm(
-            uimm8: new_imm("imm", "ir::immediates::Uimm8")
+                "imm",
-                .with_doc("An 8-bit immediate unsigned integer."),
+                "ir::immediates::Imm64",
-            uimm32: new_imm("imm", "ir::immediates::Uimm32")
+                "A 64-bit immediate integer.",
-                .with_doc("A 32-bit immediate unsigned integer."),
+            ),
-            uimm128: new_imm("imm", "ir::Immediate")
+            uimm8: new_imm(
-                .with_doc("A 128-bit immediate unsigned integer."),
+                "imm",
-            pool_constant: new_imm("constant_handle", "ir::Constant")
+                "ir::immediates::Uimm8",
-                .with_doc("A constant stored in the constant pool."),
+                "An 8-bit immediate unsigned integer.",
-            offset32: new_imm("offset", "ir::immediates::Offset32")
+            ),
-                .with_doc("A 32-bit immediate signed offset."),
+            uimm32: new_imm(
-            ieee32: new_imm("imm", "ir::immediates::Ieee32")
+                "imm",
-                .with_doc("A 32-bit immediate floating point number."),
+                "ir::immediates::Uimm32",
-            ieee64: new_imm("imm", "ir::immediates::Ieee64")
+                "A 32-bit immediate unsigned integer.",
-                .with_doc("A 64-bit immediate floating point number."),
+            ),
-            boolean: new_imm("imm", "bool").with_doc("An immediate boolean."),
+            uimm128: new_imm(
+                "imm",
+                "ir::Immediate",
+                "A 128-bit immediate unsigned integer.",
+            ),
+            pool_constant: new_imm(
+                "constant_handle",
+                "ir::Constant",
+                "A constant stored in the constant pool.",
+            ),
+            offset32: new_imm(
+                "offset",
+                "ir::immediates::Offset32",
+                "A 32-bit immediate signed offset.",
+            ),
+            ieee32: new_imm(
+                "imm",
+                "ir::immediates::Ieee32",
+                "A 32-bit immediate floating point number.",
+            ),
+            ieee64: new_imm(
+                "imm",
+                "ir::immediates::Ieee64",
+                "A 64-bit immediate floating point number.",
+            ),
+            boolean: new_imm("imm", "bool", "An immediate boolean."),
             intcc: {
                 let mut intcc_values = HashMap::new();
                 intcc_values.insert("eq", "Equal");
@@ -121,8 +157,12 @@ impl Immediates {
                 intcc_values.insert("ult", "UnsignedLessThan");
                 intcc_values.insert("of", "Overflow");
                 intcc_values.insert("nof", "NotOverflow");
-                new_enum("cond", "ir::condcodes::IntCC", intcc_values)
+                new_enum(
-                    .with_doc("An integer comparison condition code.")
+                    "cond",
+                    "ir::condcodes::IntCC",
+                    intcc_values,
+                    "An integer comparison condition code.",
+                )
             },
 
             floatcc: {
@@ -141,18 +181,27 @@ impl Immediates {
                 floatcc_values.insert("ule", "UnorderedOrLessThanOrEqual");
                 floatcc_values.insert("ugt", "UnorderedOrGreaterThan");
                 floatcc_values.insert("uge", "UnorderedOrGreaterThanOrEqual");
-                new_enum("cond", "ir::condcodes::FloatCC", floatcc_values)
+                new_enum(
-                    .with_doc("A floating point comparison condition code")
+                    "cond",
+                    "ir::condcodes::FloatCC",
+                    floatcc_values,
+                    "A floating point comparison condition code",
+                )
             },
 
-            memflags: new_imm("flags", "ir::MemFlags").with_doc("Memory operation flags"),
+            memflags: new_imm("flags", "ir::MemFlags", "Memory operation flags"),
             trapcode: {
                 let mut trapcode_values = HashMap::new();
                 trapcode_values.insert("stk_ovf", "StackOverflow");
                 trapcode_values.insert("heap_oob", "HeapOutOfBounds");
                 trapcode_values.insert("int_ovf", "IntegerOverflow");
                 trapcode_values.insert("int_divz", "IntegerDivisionByZero");
-                new_enum("code", "ir::TrapCode", trapcode_values).with_doc("A trap reason code.")
+                new_enum(
+                    "code",
+                    "ir::TrapCode",
+                    trapcode_values,
+                    "A trap reason code.",
+                )
             },
             atomic_rmw_op: {
                 let mut atomic_rmw_op_values = HashMap::new();
@@ -167,8 +216,12 @@ impl Immediates {
                 atomic_rmw_op_values.insert("umax", "Umax");
                 atomic_rmw_op_values.insert("smin", "Smin");
                 atomic_rmw_op_values.insert("smax", "Smax");
-                new_enum("op", "ir::AtomicRmwOp", atomic_rmw_op_values)
+                new_enum(
-                    .with_doc("Atomic Read-Modify-Write Ops")
+                    "op",
+                    "ir::AtomicRmwOp",
+                    atomic_rmw_op_values,
+                    "Atomic Read-Modify-Write Ops",
+                )
             },
         }
     }

cranelift/codegen/src/ir/instructions.rs

@@ -319,7 +319,7 @@ impl InstructionData {
             // included in the `InstructionData` for memory-size reasons. This case, returning
             // `None`, is left here to alert users of this method that they should retrieve the
             // value using the `DataFlowGraph`.
-            &InstructionData::Shuffle { mask: _, .. } => None,
+            &InstructionData::Shuffle { imm: _, .. } => None,
             _ => None,
         }
     }

cranelift/codegen/src/machinst/lower.rs

@@ -1264,8 +1264,8 @@ impl<'func, I: VCodeInst> LowerCtx for Lower<'func, I> {
     fn get_immediate(&self, ir_inst: Inst) -> Option<DataValue> {
         let inst_data = self.data(ir_inst);
         match inst_data {
-            InstructionData::Shuffle { mask, .. } => {
+            InstructionData::Shuffle { imm, .. } => {
-                let buffer = self.f.dfg.immediates.get(mask.clone()).unwrap().as_slice();
+                let buffer = self.f.dfg.immediates.get(imm.clone()).unwrap().as_slice();
                 let value = DataValue::V128(buffer.try_into().expect("a 16-byte data buffer"));
                 Some(value)
             }

cranelift/codegen/src/write.rs

@@ -395,8 +395,8 @@ pub fn write_operands(w: &mut dyn Write, dfg: &DataFlowGraph, inst: Inst) -> fmt
         }
         NullAry { .. } => write!(w, " "),
         TernaryImm8 { imm, args, .. } => write!(w, " {}, {}, {}", args[0], args[1], imm),
-        Shuffle { mask, args, .. } => {
+        Shuffle { imm, args, .. } => {
-            let data = dfg.immediates.get(mask).expect(
+            let data = dfg.immediates.get(imm).expect(
                 "Expected the shuffle mask to already be inserted into the immediates table",
             );
             write!(w, " {}, {}, {}", args[0], args[1], data)

cranelift/interpreter/src/step.rs

@@ -76,12 +76,12 @@ where
                     .as_slice();
                 DataValue::V128(buffer.try_into().expect("a 16-byte data buffer"))
             }
-            InstructionData::Shuffle { mask, .. } => {
+            InstructionData::Shuffle { imm, .. } => {
                 let mask = state
                     .get_current_function()
                     .dfg
                     .immediates
-                    .get(mask)
+                    .get(imm)
                     .unwrap()
                     .as_slice();
                 DataValue::V128(mask.try_into().expect("a 16-byte vector mask"))

cranelift/reader/src/parser.rs

@@ -2694,10 +2694,10 @@ impl<'a> Parser<'a> {
                 let b = self.match_value("expected SSA value second operand")?;
                 self.match_token(Token::Comma, "expected ',' between operands")?;
                 let uimm128 = self.match_uimm128(I8X16)?;
-                let mask = ctx.function.dfg.immediates.push(uimm128);
+                let imm = ctx.function.dfg.immediates.push(uimm128);
                 InstructionData::Shuffle {
                     opcode,
-                    mask,
+                    imm,
                     args: [a, b],
                 }
             }