[meta] Remove OperandKind::name field and explicitly pass rust_field_name/rust_type; (fixes #1177)

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

@@ -151,12 +151,12 @@ pub(crate) enum Literal {
     /// corresponding to a Rust enum type. An `Enumerator` object is an AST leaf node representing one
     /// of the values.
     Enumerator {
-        rust_type: String,
+        rust_type: &'static str,
         value: &'static str,
     },
 
     /// A bitwise value of an immediate operand, used for bitwise exact floating point constants.
-    Bits { rust_type: String, value: u64 },
+    Bits { rust_type: &'static str, value: u64 },
 
     /// A value of an integer immediate operand.
     Int(i64),
@@ -171,13 +171,13 @@ impl Literal {
             OperandKindFields::ImmEnum(values) => values.get(value).unwrap_or_else(|| {
                 panic!(
                     "nonexistent value '{}' in enumeration '{}'",
-                    value, kind.name
+                    value, kind.rust_type
                 )
             }),
             _ => panic!("enumerator is for enum values"),
         };
         Literal::Enumerator {
-            rust_type: kind.rust_type.clone(),
+            rust_type: kind.rust_type,
             value,
         }
     }
@@ -188,7 +188,7 @@ impl Literal {
             _ => panic!("bits_of is for immediate scalar types"),
         }
         Literal::Bits {
-            rust_type: kind.rust_type.clone(),
+            rust_type: kind.rust_type,
             value: bits,
         }
     }
@@ -475,12 +475,12 @@ impl Apply {
                                 "Nonexistent enum value '{}' passed to field of kind '{}' -- \
                                  did you use the right enum?",
                                 value,
-                                op.kind.name
+                                op.kind.rust_type
                             );
                         } else {
                             panic!(
                                 "Passed non-enum field value {:?} to field of kind {}",
-                                literal, op.kind.name
+                                literal, op.kind.rust_type
                             );
                         }
                     }
@@ -488,14 +488,14 @@ impl Apply {
                         Literal::Enumerator { value, .. } => panic!(
                             "Expected immediate value in immediate field of kind '{}', \
                              obtained enum value '{}'",
-                            op.kind.name, value
+                            op.kind.rust_type, value
                         ),
                         Literal::Bits { .. } | Literal::Int(_) | Literal::EmptyVarArgs => {}
                     },
                     _ => {
                         panic!(
                             "Literal passed to non-literal field of kind {}",
-                            op.kind.name
+                            op.kind.rust_type
                         );
                     }
                 }

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

@@ -93,10 +93,7 @@ impl EncodingBuilder {
                 {
                     let immediate_predicate = InstructionPredicate::new_is_field_equal(
                         &inst.inst.format,
-                        immediate_operand
+                        immediate_operand.kind.rust_field_name,
-                            .kind
-                            .rust_field_name()
-                            .expect("Immediates must always have a field name."),
                         immediate_value.to_string(),
                     );
                     inst_predicate = if let Some(type_predicate) = inst_predicate {

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

@@ -49,7 +49,7 @@ pub(crate) struct InstructionFormat {
 pub(crate) struct FormatStructure {
     pub num_value_operands: usize,
     pub has_value_list: bool,
-    pub imm_field_names: Vec<&'static str>,
+    pub imm_field_names: Vec<(&'static str, &'static str)>,
 }
 
 impl fmt::Display for InstructionFormat {
@@ -57,7 +57,7 @@ impl fmt::Display for InstructionFormat {
         let imm_args = self
             .imm_fields
             .iter()
-            .map(|field| format!("{}: {}", field.member, field.kind.name))
+            .map(|field| format!("{}: {}", field.member, field.kind.rust_type))
             .collect::<Vec<_>>()
             .join(", ");
         fmt.write_fmt(format_args!(
@@ -89,7 +89,7 @@ impl InstructionFormat {
             imm_field_names: self
                 .imm_fields
                 .iter()
-                .map(|field| field.kind.name)
+                .map(|field| (field.kind.rust_field_name, field.kind.rust_type))
                 .collect::<Vec<_>>(),
         }
     }
@@ -127,7 +127,7 @@ impl InstructionFormatBuilder {
     pub fn imm(mut self, operand_kind: &OperandKind) -> Self {
         let field = FormatField {
             kind: operand_kind.clone(),
-            member: operand_kind.rust_field_name().unwrap(),
+            member: operand_kind.rust_field_name,
         };
         self.imm_fields.push(field);
         self

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

@@ -572,10 +572,14 @@ fn verify_format(inst_name: &str, operands_in: &[Operand], format: &InstructionF
         if operand.is_immediate_or_entityref() {
             if let Some(format_field) = format.imm_fields.get(num_immediates) {
                 assert_eq!(
-                    format_field.kind.name, operand.kind.name,
+                    format_field.kind.rust_field_name,
+                    operand.kind.rust_field_name,
                     "{}th operand of {} should be {} (according to format), not {} (according to \
                      inst definition). You may need to use a different format.",
-                    num_immediates, inst_name, format_field.kind.name, operand.kind.name
+                    num_immediates,
+                    inst_name,
+                    format_field.kind.rust_field_name,
+                    operand.kind.rust_field_name
                 );
                 num_immediates += 1;
             }
@@ -1299,7 +1303,8 @@ mod test {
     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());
-        let kind = OperandKindBuilder::new(name, field).build();
+        // Format's name / rust_type don't matter here.
+        let kind = OperandKindBuilder::new(name, name, field).build();
         let operand = Operand::new(name, kind);
         operand
     }

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

@@ -1,6 +1,5 @@
 use std::collections::HashMap;
 
-use crate::cdsl::camel_case;
 use crate::cdsl::typevar::TypeVar;
 
 /// An instruction operand can be an *immediate*, an *SSA value*, or an *entity reference*. The
@@ -18,9 +17,13 @@ use crate::cdsl::typevar::TypeVar;
 ///    function, typically something declared in the function preamble.
 #[derive(Clone, Debug)]
 pub(crate) struct Operand {
+    /// Name of the operand variable, as it appears in function parameters, legalizations, etc.
     pub name: &'static str,
-    doc: Option<&'static str>,
+
+    /// Type of the operand.
     pub kind: OperandKind,
+
+    doc: Option<&'static str>,
 }
 
 impl Operand {
@@ -110,56 +113,19 @@ pub(crate) enum OperandKindFields {
 
 #[derive(Clone, Debug)]
 pub(crate) struct OperandKind {
-    pub name: &'static str,
+    /// String representation of the Rust type mapping to this OperandKind.
-    doc: Option<&'static str>,
+    pub rust_type: &'static str,
-    rust_field_name: Option<&'static str>,
+
-    /// The camel-cased name of an operand kind is also the Rust type used to represent it.
+    /// Name of this OperandKind in the format's member field.
-    pub rust_type: String,
+    pub rust_field_name: &'static str,
+
+    /// Type-specific fields for this OperandKind.
     pub fields: OperandKindFields,
+
+    doc: Option<&'static str>,
 }
 
 impl OperandKind {
-    fn new(
-        name: &'static str,
-        doc: Option<&'static str>,
-        rust_field_name: Option<&'static str>,
-        rust_type: Option<&'static str>,
-        fields: OperandKindFields,
-    ) -> Self {
-        // Compute the default rust_type value, if it wasn't provided.
-        let rust_type = match rust_type {
-            Some(rust_type) => rust_type.to_string(),
-            None => match &fields {
-                OperandKindFields::ImmEnum(_) | OperandKindFields::ImmValue => {
-                    format!("ir::immediates::{}", camel_case(name))
-                }
-                OperandKindFields::VariableArgs => "&[Value]".to_string(),
-                OperandKindFields::TypeVar(_) | OperandKindFields::EntityRef => {
-                    format!("ir::{}", camel_case(name))
-                }
-            },
-        };
-        Self {
-            name,
-            doc,
-            rust_field_name,
-            rust_type,
-            fields,
-        }
-    }
-
-    /// Name of this OperandKind in the format's member field.
-    pub fn rust_field_name(&self) -> Option<&'static str> {
-        if let Some(member) = &self.rust_field_name {
-            return Some(member);
-        }
-        match &self.fields {
-            OperandKindFields::ImmEnum(_) | OperandKindFields::ImmValue => Some("imm"),
-            OperandKindFields::TypeVar(_) | OperandKindFields::EntityRef => Some(self.name),
-            OperandKindFields::VariableArgs => None,
-        }
-    }
-
     fn doc(&self) -> Option<&str> {
         if let Some(doc) = &self.doc {
             return Some(doc);
@@ -176,7 +142,12 @@ impl OperandKind {
 
 impl Into<OperandKind> for &TypeVar {
     fn into(self) -> OperandKind {
-        OperandKindBuilder::new("value", OperandKindFields::TypeVar(self.into())).build()
+        OperandKindBuilder::new(
+            "value",
+            "ir::Value",
+            OperandKindFields::TypeVar(self.into()),
+        )
+        .build()
     }
 }
 impl Into<OperandKind> for &OperandKind {
@@ -186,63 +157,56 @@ impl Into<OperandKind> for &OperandKind {
 }
 
 pub(crate) struct OperandKindBuilder {
-    name: &'static str,
+    rust_field_name: &'static str,
-    doc: Option<&'static str>,
+    rust_type: &'static str,
-    rust_field_name: Option<&'static str>,
-    rust_type: Option<&'static str>,
     fields: OperandKindFields,
+    doc: Option<&'static str>,
 }
 
 impl OperandKindBuilder {
-    pub fn new(name: &'static str, fields: OperandKindFields) -> Self {
+    pub fn new(
+        rust_field_name: &'static str,
+        rust_type: &'static str,
+        fields: OperandKindFields,
+    ) -> Self {
         Self {
-            name,
+            rust_field_name,
-            doc: None,
+            rust_type,
-            rust_field_name: None,
-            rust_type: None,
             fields,
+            doc: None,
         }
     }
-    pub fn new_imm(name: &'static str) -> Self {
+    pub fn new_imm(rust_field_name: &'static str, rust_type: &'static str) -> Self {
         Self {
-            name,
+            rust_field_name,
-            doc: None,
+            rust_type,
-            rust_field_name: None,
-            rust_type: None,
             fields: OperandKindFields::ImmValue,
-        }
-    }
-    pub fn new_enum(name: &'static str, values: EnumValues) -> Self {
-        Self {
-            name,
             doc: None,
-            rust_field_name: None,
-            rust_type: None,
-            fields: OperandKindFields::ImmEnum(values),
         }
     }
-    pub fn doc(mut self, doc: &'static str) -> Self {
+    pub fn new_enum(
+        rust_field_name: &'static str,
+        rust_type: &'static str,
+        values: EnumValues,
+    ) -> Self {
+        Self {
+            rust_field_name,
+            rust_type,
+            fields: OperandKindFields::ImmEnum(values),
+            doc: None,
+        }
+    }
+    pub fn with_doc(mut self, doc: &'static str) -> Self {
         assert!(self.doc.is_none());
         self.doc = Some(doc);
         self
     }
-    pub fn rust_field_name(mut self, rust_field_name: &'static str) -> Self {
-        assert!(self.rust_field_name.is_none());
-        self.rust_field_name = Some(rust_field_name);
-        self
-    }
-    pub fn rust_type(mut self, rust_type: &'static str) -> Self {
-        assert!(self.rust_type.is_none());
-        self.rust_type = Some(rust_type);
-        self
-    }
     pub fn build(self) -> OperandKind {
-        OperandKind::new(
+        OperandKind {
-            self.name,
+            rust_type: self.rust_type,
-            self.doc,
+            fields: self.fields,
-            self.rust_field_name,
+            rust_field_name: self.rust_field_name,
-            self.rust_type,
+            doc: self.doc,
-            self.fields,
+        }
-        )
     }
 }

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

@@ -214,7 +214,7 @@ fn rewrite_expr(
             .inst()
             .operands_in
             .iter()
-            .map(|operand| format!("{}: {}", operand.name, operand.kind.name))
+            .map(|operand| format!("{}: {}", operand.name, operand.kind.rust_type))
             .collect::<Vec<_>>(),
     );
 

cranelift/codegen/meta/src/gen_inst.rs

@@ -914,12 +914,12 @@ fn gen_inst_builder(inst: &Instruction, format: &InstructionFormat, fmt: &mut Fo
     let mut into_args = Vec::new();
     for op in &inst.operands_in {
         let t = if op.is_immediate() {
-            let t = format!("T{}{}", tmpl_types.len() + 1, op.kind.name);
+            let t = format!("T{}", tmpl_types.len() + 1);
             tmpl_types.push(format!("{}: Into<{}>", t, op.kind.rust_type));
             into_args.push(op.name);
             t
         } else {
-            op.kind.rust_type.clone()
+            op.kind.rust_type.to_string()
         };
         args.push(format!("{}: {}", op.name, t));
         args_doc.push(format!(

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

@@ -35,28 +35,29 @@ pub(crate) struct EntityRefs {
 impl EntityRefs {
     pub fn new() -> Self {
         Self {
-            ebb: create("ebb", "An extended basic block in the same function.")
+            ebb: create(
-                .rust_field_name("destination")
+                "destination",
-                .build(),
+                "ir::Ebb",
+                "An extended basic block in the same function.",
+            )
+            .build(),
 
-            stack_slot: create("stack_slot", "A stack slot").build(),
+            stack_slot: create("stack_slot", "ir::StackSlot", "A stack slot").build(),
 
-            global_value: create("global_value", "A global value.").build(),
+            global_value: create("global_value", "ir::GlobalValue", "A global value.").build(),
 
-            sig_ref: create("sig_ref", "A function signature.").build(),
+            sig_ref: create("sig_ref", "ir::SigRef", "A function signature.").build(),
 
-            func_ref: create("func_ref", "An external function.").build(),
+            func_ref: create("func_ref", "ir::FuncRef", "An external function.").build(),
 
-            jump_table: create("jump_table", "A jump table.")
+            jump_table: create("table", "ir::JumpTable", "A jump table.").build(),
-                .rust_field_name("table")
-                .build(),
 
-            heap: create("heap", "A heap.").build(),
+            heap: create("heap", "ir::Heap", "A heap.").build(),
 
-            table: create("table", "A table.").build(),
+            table: create("table", "ir::Table", "A table.").build(),
 
-            varargs: Builder::new("variable_args", OperandKindFields::VariableArgs)
+            varargs: Builder::new("", "&[Value]", OperandKindFields::VariableArgs)
-                .doc(
+                .with_doc(
                     r#"
                         A variable size list of `value` operands.
 
@@ -71,6 +72,6 @@ impl EntityRefs {
 }
 
 /// Small helper to initialize an OperandBuilder with the right kind, for a given name and doc.
-fn create(name: &'static str, doc: &'static str) -> Builder {
+fn create(format_field_name: &'static str, rust_type: &'static str, doc: &'static str) -> Builder {
-    Builder::new(name, OperandKindFields::EntityRef).doc(doc)
+    Builder::new(format_field_name, rust_type, OperandKindFields::EntityRef).with_doc(doc)
 }

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

@@ -76,45 +76,40 @@ pub(crate) struct Immediates {
 impl Immediates {
     pub fn new() -> Self {
         Self {
-            imm64: Builder::new_imm("imm64")
+            imm64: Builder::new_imm("imm", "ir::immediates::Imm64")
-                .doc("A 64-bit immediate integer.")
+                .with_doc("A 64-bit immediate integer.")
                 .build(),
 
-            uimm8: Builder::new_imm("uimm8")
+            uimm8: Builder::new_imm("imm", "ir::immediates::Uimm8")
-                .doc("An 8-bit immediate unsigned integer.")
+                .with_doc("An 8-bit immediate unsigned integer.")
                 .build(),
 
-            uimm32: Builder::new_imm("uimm32")
+            uimm32: Builder::new_imm("imm", "ir::immediates::Uimm32")
-                .doc("A 32-bit immediate unsigned integer.")
+                .with_doc("A 32-bit immediate unsigned integer.")
                 .build(),
 
-            uimm128: Builder::new_imm("uimm128")
+            uimm128: Builder::new_imm("imm", "ir::Immediate")
-                .doc("A 128-bit immediate unsigned integer.")
+                .with_doc("A 128-bit immediate unsigned integer.")
-                .rust_type("ir::Immediate")
                 .build(),
 
-            pool_constant: Builder::new_imm("poolConstant")
+            pool_constant: Builder::new_imm("constant_handle", "ir::Constant")
-                .doc("A constant stored in the constant pool.")
+                .with_doc("A constant stored in the constant pool.")
-                .rust_field_name("constant_handle")
-                .rust_type("ir::Constant")
                 .build(),
 
-            offset32: Builder::new_imm("offset32")
+            offset32: Builder::new_imm("offset", "ir::immediates::Offset32")
-                .doc("A 32-bit immediate signed offset.")
+                .with_doc("A 32-bit immediate signed offset.")
-                .rust_field_name("offset")
                 .build(),
 
-            ieee32: Builder::new_imm("ieee32")
+            ieee32: Builder::new_imm("imm", "ir::immediates::Ieee32")
-                .doc("A 32-bit immediate floating point number.")
+                .with_doc("A 32-bit immediate floating point number.")
                 .build(),
 
-            ieee64: Builder::new_imm("ieee64")
+            ieee64: Builder::new_imm("imm", "ir::immediates::Ieee64")
-                .doc("A 64-bit immediate floating point number.")
+                .with_doc("A 64-bit immediate floating point number.")
                 .build(),
 
-            boolean: Builder::new_imm("boolean")
+            boolean: Builder::new_imm("imm", "bool")
-                .doc("An immediate boolean.")
+                .with_doc("An immediate boolean.")
-                .rust_type("bool")
                 .build(),
 
             intcc: {
@@ -131,10 +126,8 @@ impl Immediates {
                 intcc_values.insert("ult", "UnsignedLessThan");
                 intcc_values.insert("of", "Overflow");
                 intcc_values.insert("nof", "NotOverflow");
-                Builder::new_enum("intcc", intcc_values)
+                Builder::new_enum("cond", "ir::condcodes::IntCC", intcc_values)
-                    .doc("An integer comparison condition code.")
+                    .with_doc("An integer comparison condition code.")
-                    .rust_field_name("cond")
-                    .rust_type("ir::condcodes::IntCC")
                     .build()
             },
 
@@ -154,22 +147,17 @@ impl Immediates {
                 floatcc_values.insert("ule", "UnorderedOrLessThanOrEqual");
                 floatcc_values.insert("ugt", "UnorderedOrGreaterThan");
                 floatcc_values.insert("uge", "UnorderedOrGreaterThanOrEqual");
-                Builder::new_enum("floatcc", floatcc_values)
+                Builder::new_enum("cond", "ir::condcodes::FloatCC", floatcc_values)
-                    .doc("A floating point comparison condition code")
+                    .with_doc("A floating point comparison condition code")
-                    .rust_field_name("cond")
-                    .rust_type("ir::condcodes::FloatCC")
                     .build()
             },
 
-            memflags: Builder::new_imm("memflags")
+            memflags: Builder::new_imm("flags", "ir::MemFlags")
-                .doc("Memory operation flags")
+                .with_doc("Memory operation flags")
-                .rust_field_name("flags")
-                .rust_type("ir::MemFlags")
                 .build(),
 
-            regunit: Builder::new_imm("regunit")
+            regunit: Builder::new_imm("regunit", "isa::RegUnit")
-                .doc("A register unit in the target ISA")
+                .with_doc("A register unit in the target ISA")
-                .rust_type("isa::RegUnit")
                 .build(),
 
             trapcode: {
@@ -178,10 +166,8 @@ impl Immediates {
                 trapcode_values.insert("heap_oob", "HeapOutOfBounds");
                 trapcode_values.insert("int_ovf", "IntegerOverflow");
                 trapcode_values.insert("int_divz", "IntegerDivisionByZero");
-                Builder::new_enum("trapcode", trapcode_values)
+                Builder::new_enum("code", "ir::TrapCode", trapcode_values)
-                    .doc("A trap reason code.")
+                    .with_doc("A trap reason code.")
-                    .rust_field_name("code")
-                    .rust_type("ir::TrapCode")
                     .build()
             },
         }