[meta] Avoid unwrapping instructions several times during legalization;

This avoids doing multiple unpacking of the InstructionData for a single
legalization, improving readability and reducing size of the generated
code. For instance, icmp had to unpack the format once per IntCC
condition code.

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

@@ -63,7 +63,7 @@ impl Def {
             format!("({})", results.join(", "))
         };
 
-        format!("{} << {}", results, self.apply.to_comment_string(var_pool))
+        format!("{} := {}", results, self.apply.to_comment_string(var_pool))
     }
 }
 

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

@@ -856,14 +856,14 @@ pub enum TypePredicateNode {
 }
 
 impl TypePredicateNode {
-    fn rust_predicate(&self) -> String {
+    fn rust_predicate(&self, func_str: &str) -> String {
         match self {
             TypePredicateNode::TypeVarCheck(index, value_type_name) => format!(
-                "func.dfg.value_type(args[{}]) == {}",
+                "{}.dfg.value_type(args[{}]) == {}",
-                index, value_type_name
+                func_str, index, value_type_name
             ),
             TypePredicateNode::CtrlTypeVarCheck(value_type_name) => {
-                format!("func.dfg.ctrl_typevar(inst) == {}", value_type_name)
+                format!("{}.dfg.ctrl_typevar(inst) == {}", func_str, value_type_name)
             }
         }
     }
@@ -884,18 +884,18 @@ pub enum InstructionPredicateNode {
 }
 
 impl InstructionPredicateNode {
-    fn rust_predicate(&self) -> String {
+    fn rust_predicate(&self, func_str: &str) -> String {
         match self {
             InstructionPredicateNode::FormatPredicate(node) => node.rust_predicate(),
-            InstructionPredicateNode::TypePredicate(node) => node.rust_predicate(),
+            InstructionPredicateNode::TypePredicate(node) => node.rust_predicate(func_str),
             InstructionPredicateNode::And(nodes) => nodes
                 .iter()
-                .map(|x| x.rust_predicate())
+                .map(|x| x.rust_predicate(func_str))
                 .collect::<Vec<_>>()
                 .join(" && "),
             InstructionPredicateNode::Or(nodes) => nodes
                 .iter()
-                .map(|x| x.rust_predicate())
+                .map(|x| x.rust_predicate(func_str))
                 .collect::<Vec<_>>()
                 .join(" || "),
         }
@@ -1169,9 +1169,9 @@ impl InstructionPredicate {
         self
     }
 
-    pub fn rust_predicate(&self) -> String {
+    pub fn rust_predicate(&self, func_str: &str) -> String {
         match &self.node {
-            Some(root) => root.rust_predicate(),
+            Some(root) => root.rust_predicate(func_str),
             None => "true".into(),
         }
     }

cranelift/codegen/meta/src/gen_encodings.rs

@@ -78,7 +78,7 @@ use crate::unique_table::UniqueSeqTable;
 fn emit_instp(instp: &InstructionPredicate, has_func: bool, fmt: &mut Formatter) {
     if instp.is_type_predicate() {
         fmt.line("let args = inst.arguments(&func.dfg.value_lists);");
-        fmt.line(instp.rust_predicate());
+        fmt.line(instp.rust_predicate("func"));
         return;
     }
 
@@ -127,7 +127,7 @@ fn emit_instp(instp: &InstructionPredicate, has_func: bool, fmt: &mut Formatter)
             // Silence dead argument.
             fmt.line("let _ = func;");
         }
-        fmtln!(fmt, "return {};", instp.rust_predicate());
+        fmtln!(fmt, "return {};", instp.rust_predicate("func"));
     });
     fmtln!(fmt, "}");
 

cranelift/codegen/meta/src/gen_legalizer.rs

@@ -1,6 +1,7 @@
-use crate::cdsl::ast::{Def, DefPool, VarPool};
+use crate::cdsl::ast::{Def, DefPool, Expr, VarPool};
 use crate::cdsl::formats::FormatRegistry;
 use crate::cdsl::isa::TargetIsa;
+use crate::cdsl::operands::Operand;
 use crate::cdsl::type_inference::Constraint;
 use crate::cdsl::typevar::{TypeSet, TypeVar};
 use crate::cdsl::xform::{Transform, TransformGroup, TransformGroups};
@@ -34,7 +35,7 @@ fn unwrap_inst(
     let iform = format_registry.get(inst.format);
 
     fmt.comment(format!(
-        "Unwrap {}",
+        "Unwrap fields from instruction format {}",
         def.to_comment_string(&transform.var_pool)
     ));
 
@@ -42,19 +43,54 @@ fn unwrap_inst(
     let arg_names = apply
         .args
         .iter()
-        .map(|arg| match arg.maybe_var() {
+        .enumerate()
-            Some(var_index) => var_pool.get(var_index).name.as_ref(),
+        .map(|(arg_num, arg)| match &arg {
-            None => "_",
+            Expr::Var(var_index) => var_pool.get(*var_index).name.as_ref(),
+            Expr::Literal(_) => {
+                let n = inst.imm_opnums.iter().position(|&i| i == arg_num).unwrap();
+                iform.imm_fields[n].member
+            }
         })
         .collect::<Vec<_>>()
         .join(", ");
 
+    // May we need "args" in the values consumed by predicates?
+    let emit_args = iform.num_value_operands >= 1 || iform.has_value_list;
+
+    // We need a tuple:
+    // - if there's at least one value operand, then we emit a variable for the value, and the
+    // value list as args.
+    // - otherwise, if there's the count of immediate operands added to the presence of a value list exceeds one.
+    let need_tuple = if iform.num_value_operands >= 1 {
+        true
+    } else {
+        let mut imm_and_varargs = inst
+            .operands_in
+            .iter()
+            .filter(|op| op.is_immediate())
+            .count();
+        if iform.has_value_list {
+            imm_and_varargs += 1;
+        }
+        imm_and_varargs > 1
+    };
+
+    let maybe_args = if emit_args { ", args" } else { "" };
+    let defined_values = format!("{}{}", arg_names, maybe_args);
+
+    let tuple_or_value = if need_tuple {
+        format!("({})", defined_values)
+    } else {
+        defined_values
+    };
+
     fmtln!(
         fmt,
-        "let ({}, predicate) = if let crate::ir::InstructionData::{} {{",
+        "let {} = if let ir::InstructionData::{} {{",
-        arg_names,
+        tuple_or_value,
         iform.name
     );
+
     fmt.indent(|fmt| {
         // Fields are encoded directly.
         for field in &iform.imm_fields {
@@ -69,47 +105,61 @@ fn unwrap_inst(
 
         fmt.line("..");
         fmt.outdented_line("} = pos.func.dfg[inst] {");
-        fmt.line("let func = &pos.func;");
 
         if iform.has_value_list {
-            fmt.line("let args = args.as_slice(&func.dfg.value_lists);");
+            fmt.line("let args = args.as_slice(&pos.func.dfg.value_lists);");
         } else if iform.num_value_operands == 1 {
             fmt.line("let args = [arg];")
         }
 
         // Generate the values for the tuple.
-        fmt.line("(");
+        let emit_one_value =
-        fmt.indent(|fmt| {
+            |fmt: &mut Formatter, needs_comma: bool, op_num: usize, op: &Operand| {
-            for (op_num, op) in inst.operands_in.iter().enumerate() {
+                let comma = if needs_comma { "," } else { "" };
                 if op.is_immediate() {
                     let n = inst.imm_opnums.iter().position(|&i| i == op_num).unwrap();
-                    fmtln!(fmt, "{},", iform.imm_fields[n].member);
+                    fmtln!(fmt, "{}{}", iform.imm_fields[n].member, comma);
                 } else if op.is_value() {
                     let n = inst.value_opnums.iter().position(|&i| i == op_num).unwrap();
-                    fmtln!(fmt, "func.dfg.resolve_aliases(args[{}]),", n);
+                    fmtln!(fmt, "pos.func.dfg.resolve_aliases(args[{}]),", n);
                 } else if op.is_varargs() {
                     let n = inst.imm_opnums.iter().chain(inst.value_opnums.iter()).max().map(|n| n + 1).unwrap_or(0);
                     // We need to create a `Vec` here, as using a slice would result in a borrowck
                     // error later on.
                     fmtln!(fmt, "\
-                        args.iter().skip({}).map(|&arg| func.dfg.resolve_aliases(arg)).collect::<Vec<_>>(),\
+                        args.iter().skip({}).map(|&arg| pos.func.dfg.resolve_aliases(arg)).collect::<Vec<_>>(),\
                     ", n);
+                } else {
+                    // This is a value list argument.
+                    assert!(iform.has_value_list);
                 }
-            }
+            };
 
-            // Evaluate the instruction predicate if any.
+        if need_tuple {
-            fmt.multi_line(
+            fmt.line("(");
-                &apply
+            fmt.indent(|fmt| {
-                    .inst_predicate_with_ctrl_typevar(format_registry, var_pool)
+                for (op_num, op) in inst.operands_in.iter().enumerate() {
-                    .rust_predicate(),
+                    let needs_comma = emit_args || op_num + 1 < inst.operands_in.len();
-            );
+                    emit_one_value(fmt, needs_comma, op_num, op);
-        });
+                }
-        fmt.line(")");
+                if emit_args {
+                    fmt.line("args");
+                }
+            });
+            fmt.line(")");
+        } else {
+            // Only one of these can be true at the same time, otherwise we'd need a tuple.
+            emit_one_value(fmt, false, 0, &inst.operands_in[0]);
+            if emit_args {
+                fmt.line("args");
+            }
+        }
 
         fmt.outdented_line("} else {");
         fmt.line(r#"unreachable!("bad instruction format")"#);
     });
     fmtln!(fmt, "};");
+    fmt.empty_line();
 
     assert_eq!(inst.operands_in.len(), apply.args.len());
     for (i, op) in inst.operands_in.iter().enumerate() {
@@ -385,21 +435,37 @@ fn emit_dst_inst(def: &Def, def_pool: &DefPool, var_pool: &VarPool, fmt: &mut Fo
 /// `inst: Inst` is the variable to be replaced. It is pointed to by `pos: Cursor`.
 /// `dfg: DataFlowGraph` is available and mutable.
 fn gen_transform<'a>(
+    replace_inst: bool,
     transform: &'a Transform,
     format_registry: &FormatRegistry,
     type_sets: &mut UniqueTable<'a, TypeSet>,
     fmt: &mut Formatter,
 ) {
-    // Unwrap the source instruction, create local variables for the input variables.
+    // Evaluate the instruction predicate if any.
-    let replace_inst = unwrap_inst(&transform, format_registry, fmt);
+    let apply = &transform.def_pool.get(transform.src).apply;
 
-    // Emit any runtime checks; these will rebind `predicate` emitted by unwrap_inst().
+    let inst_predicate = apply
+        .inst_predicate_with_ctrl_typevar(format_registry, &transform.var_pool)
+        .rust_predicate("pos.func");
+
+    let has_extra_constraints = !transform.type_env.constraints.is_empty();
+    if has_extra_constraints {
+        // Extra constraints rely on the predicate being a variable that we can rebind as we add
+        // more constraint predicates.
+        fmt.multi_line(&format!("let predicate = {};", inst_predicate));
+    }
+
+    // Emit any runtime checks; these will rebind `predicate` emitted right above.
     for constraint in &transform.type_env.constraints {
         emit_runtime_typecheck(constraint, type_sets, fmt);
     }
 
     // Guard the actual expansion by `predicate`.
-    fmt.line("if predicate {");
+    if has_extra_constraints {
+        fmt.line("if predicate {");
+    } else {
+        fmt.multi_line(&format!("if {} {{", inst_predicate))
+    }
     fmt.indent(|fmt| {
         // Emit any constants that must be created before use.
         for (name, value) in transform.const_pool.iter() {
@@ -525,8 +591,17 @@ fn gen_transform_group<'a>(
                 for camel_name in sorted_inst_names {
                     fmtln!(fmt, "ir::Opcode::{} => {{", camel_name);
                     fmt.indent(|fmt| {
-                        for transform in inst_to_transforms.get(camel_name).unwrap() {
+                        let transforms = inst_to_transforms.get(camel_name).unwrap();
-                            gen_transform(transform, format_registry, type_sets, fmt);
+
+                        // Unwrap the source instruction, create local variables for the input variables.
+                        let replace_inst = unwrap_inst(&transforms[0], format_registry, fmt);
+                        fmt.empty_line();
+
+                        for (i, transform) in transforms.into_iter().enumerate() {
+                            if i > 0 {
+                                fmt.empty_line();
+                            }
+                            gen_transform(replace_inst, transform, format_registry, type_sets, fmt);
                         }
                     });
                     fmtln!(fmt, "}");
@@ -540,7 +615,7 @@ fn gen_transform_group<'a>(
                 for (inst_camel_name, func_name) in sorted_custom_legalizes {
                     fmtln!(fmt, "ir::Opcode::{} => {{", inst_camel_name);
                     fmt.indent(|fmt| {
-                        fmtln!(fmt, "{}(inst, pos.func, cfg, isa);", func_name);
+                        fmtln!(fmt, "{}(inst, func, cfg, isa);", func_name);
                         fmt.line("return true;");
                     });
                     fmtln!(fmt, "}");
@@ -558,7 +633,7 @@ fn gen_transform_group<'a>(
         match &group.chain_with {
             Some(group_id) => fmtln!(
                 fmt,
-                "{}(inst, pos.func, cfg, isa)",
+                "{}(inst, func, cfg, isa)",
                 transform_groups.get(*group_id).rust_name()
             ),
             None => fmt.line("false"),

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

@@ -212,7 +212,9 @@ pub(crate) fn define(shared: &mut SharedDefinitions, x86_instructions: &Instruct
     );
 
     // Population count for baseline x86_64
-    let qv1 = var("qv1");
+    let x = var("x");
+    let r = var("r");
+
     let qv3 = var("qv3");
     let qv4 = var("qv4");
     let qv5 = var("qv5");
@@ -226,7 +228,6 @@ pub(crate) fn define(shared: &mut SharedDefinitions, x86_instructions: &Instruct
     let qv13 = var("qv13");
     let qv14 = var("qv14");
     let qv15 = var("qv15");
-    let qv16 = var("qv16");
     let qc77 = var("qc77");
     #[allow(non_snake_case)]
     let qc0F = var("qc0F");
@@ -235,12 +236,12 @@ pub(crate) fn define(shared: &mut SharedDefinitions, x86_instructions: &Instruct
     let imm64_1 = Literal::constant(&imm.imm64, 1);
     let imm64_4 = Literal::constant(&imm.imm64, 4);
     group.legalize(
-        def!(qv16 = popcnt.I64(qv1)),
+        def!(r = popcnt.I64(x)),
         vec![
-            def!(qv3 = ushr_imm(qv1, imm64_1)),
+            def!(qv3 = ushr_imm(x, imm64_1)),
             def!(qc77 = iconst(Literal::constant(&imm.imm64, 0x7777777777777777))),
             def!(qv4 = band(qv3, qc77)),
-            def!(qv5 = isub(qv1, qv4)),
+            def!(qv5 = isub(x, qv4)),
             def!(qv6 = ushr_imm(qv4, imm64_1)),
             def!(qv7 = band(qv6, qc77)),
             def!(qv8 = isub(qv5, qv7)),
@@ -253,11 +254,10 @@ pub(crate) fn define(shared: &mut SharedDefinitions, x86_instructions: &Instruct
             def!(qv14 = band(qv13, qc0F)),
             def!(qc01 = iconst(Literal::constant(&imm.imm64, 0x0101010101010101))),
             def!(qv15 = imul(qv14, qc01)),
-            def!(qv16 = ushr_imm(qv15, Literal::constant(&imm.imm64, 56))),
+            def!(r = ushr_imm(qv15, Literal::constant(&imm.imm64, 56))),
         ],
     );
 
-    let lv1 = var("lv1");
     let lv3 = var("lv3");
     let lv4 = var("lv4");
     let lv5 = var("lv5");
@@ -271,19 +271,18 @@ pub(crate) fn define(shared: &mut SharedDefinitions, x86_instructions: &Instruct
     let lv13 = var("lv13");
     let lv14 = var("lv14");
     let lv15 = var("lv15");
-    let lv16 = var("lv16");
     let lc77 = var("lc77");
     #[allow(non_snake_case)]
     let lc0F = var("lc0F");
     let lc01 = var("lc01");
 
     group.legalize(
-        def!(lv16 = popcnt.I32(lv1)),
+        def!(r = popcnt.I32(x)),
         vec![
-            def!(lv3 = ushr_imm(lv1, imm64_1)),
+            def!(lv3 = ushr_imm(x, imm64_1)),
             def!(lc77 = iconst(Literal::constant(&imm.imm64, 0x77777777))),
             def!(lv4 = band(lv3, lc77)),
-            def!(lv5 = isub(lv1, lv4)),
+            def!(lv5 = isub(x, lv4)),
             def!(lv6 = ushr_imm(lv4, imm64_1)),
             def!(lv7 = band(lv6, lc77)),
             def!(lv8 = isub(lv5, lv7)),
@@ -296,7 +295,7 @@ pub(crate) fn define(shared: &mut SharedDefinitions, x86_instructions: &Instruct
             def!(lv14 = band(lv13, lc0F)),
             def!(lc01 = iconst(Literal::constant(&imm.imm64, 0x01010101))),
             def!(lv15 = imul(lv14, lc01)),
-            def!(lv16 = ushr_imm(lv15, Literal::constant(&imm.imm64, 24))),
+            def!(r = ushr_imm(lv15, Literal::constant(&imm.imm64, 24))),
         ],
     );