Simplify the back-end of InstBuilder.

We don't want to distinguish between single-result and multiple-result
instructions any longer.

- Merge the simple_instruction() and complex_instruction() builder
  methods into a single build() that can handle all cases.
- All format constructors now take a ctrl_type argument. Previously,
  some would take a result_type argument.
- Instruction constructors no longer attempt to compute a single result
  type. Just pass a ctrl_type and let the backend decide.

Fix one format constructor call in legalizer/split.rs which now takes a
ctrl_type instead of a result type.

lib/cretonne/meta/gen_instr.py

@@ -498,21 +498,12 @@ def gen_format_constructor(iform, fmt):
     Emit a method for creating and inserting inserting an `iform` instruction,
     where `iform` is an instruction format.
 
-    Instruction formats that can produce multiple results take a `ctrl_typevar`
+    All instruction formats take an `opcode` argument and a `ctrl_typevar`
-    argument for deducing the result types. Others take a `result_type`
+    argument for deducing the result types.
-    argument.
     """
 
     # Construct method arguments.
-    args = ['self', 'opcode: Opcode']
+    args = ['self', 'opcode: Opcode', 'ctrl_typevar: Type']
-
-    if iform.multiple_results:
-        args.append('ctrl_typevar: Type')
-        # `dfg::make_inst_results` will compute the result type.
-        result_type = 'types::VOID'
-    else:
-        args.append('result_type: Type')
-        result_type = 'result_type'
 
     # Normal operand arguments. Start with the immediate operands.
     for f in iform.imm_fields:
@@ -537,16 +528,12 @@ def gen_format_constructor(iform, fmt):
         with fmt.indented(
                 'let data = InstructionData::{} {{'.format(iform.name), '};'):
             fmt.line('opcode: opcode,')
-            fmt.line('ty: {},'.format(result_type))
+            fmt.line('ty: types::VOID,')
             if iform.multiple_results:
                 fmt.line('second_result: None.into(),')
             gen_member_inits(iform, fmt)
 
-        # Create result values if necessary.
+        fmt.line('self.build(data, ctrl_typevar)')
-        if iform.multiple_results:
-            fmt.line('self.complex_instruction(data, ctrl_typevar)')
-        else:
-            fmt.line('self.simple_instruction(data)')
 
 
 def gen_member_inits(iform, fmt):
@@ -631,34 +618,19 @@ def gen_inst_builder(inst, fmt):
         if inst.is_polymorphic and not inst.use_typevar_operand:
             # This was an explicit method argument.
             args.append(inst.ctrl_typevar.name)
-        elif len(inst.value_results) == 0:
+        elif len(inst.value_results) == 0 or not inst.is_polymorphic:
+            # No controlling type variable needed.
             args.append('types::VOID')
-        elif inst.is_polymorphic:
+        else:
+            assert inst.is_polymorphic and inst.use_typevar_operand
             # Infer the controlling type variable from the input operands.
             opnum = inst.value_opnums[inst.format.typevar_operand]
             fmt.line(
                     'let ctrl_typevar = self.data_flow_graph().value_type({});'
                     .format(inst.ins[opnum].name))
-            if inst.format.multiple_results:
             # The format constructor will resolve the result types from the
             # type var.
             args.append('ctrl_typevar')
-            elif inst.outs[inst.value_results[0]].typevar == inst.ctrl_typevar:
-                # The format constructor expects a simple result type.
-                # No type transformation needed from the controlling type
-                # variable.
-                args.append('ctrl_typevar')
-            else:
-                # The format constructor expects a simple result type.
-                # TODO: This formula could be resolved ahead of time.
-                args.append(
-                        'Opcode::{}.constraints().result_type(0, ctrl_typevar)'
-                        .format(inst.camel_name))
-        else:
-            # This non-polymorphic instruction has a fixed result type.
-            args.append(
-                    inst.outs[inst.value_results[0]]
-                    .typevar.singleton_type.rust_name())
 
         # Now add all of the immediate operands to the constructor arguments.
         for opnum in inst.imm_opnums:
@@ -717,8 +689,8 @@ def gen_builder(insts, fmt):
             The `InstrBuilder` trait has one method per instruction opcode for
             conveniently constructing the instruction with minimum arguments.
             Polymorphic instructions infer their result types from the input
-            arguments when possible. In some cases, an explicit `result_type`
+            arguments when possible. In some cases, an explicit `ctrl_typevar`
-            or `ctrl_typevar` argument is required.
+            argument is required.
 
             The opcode methods return the new instruction's result values, or
             the `Inst` itself for instructions that don't have any results.

lib/cretonne/src/ir/builder.rs

@@ -24,21 +24,11 @@ pub trait InstBuilderBase<'f>: Sized {
     fn data_flow_graph(&self) -> &DataFlowGraph;
     fn data_flow_graph_mut(&mut self) -> &mut DataFlowGraph;
 
-    /// Insert a simple instruction and return a reference to it.
+    /// Insert an instruction and return a reference to it, consuming the builder.
     ///
-    /// A 'simple' instruction has at most one result, and the `data.ty` field must contain the
+    /// The result types may depend on a controlling type variable. For non-polymorphic
-    /// result type or `VOID` for an instruction with no result values.
+    /// instructions with multiple results, pass `VOID` for the `ctrl_typevar` argument.
-    fn simple_instruction(self, data: InstructionData) -> (Inst, &'f mut DataFlowGraph);
+    fn build(self, data: InstructionData, ctrl_typevar: Type) -> (Inst, &'f mut DataFlowGraph);
-
-    /// Insert a simple instruction and return a reference to it.
-    ///
-    /// A 'complex' instruction may produce multiple results, and the result types may depend on a
-    /// controlling type variable. For non-polymorphic instructions with multiple results, pass
-    /// `VOID` for the `ctrl_typevar` argument.
-    fn complex_instruction(self,
-                           data: InstructionData,
-                           ctrl_typevar: Type)
-                           -> (Inst, &'f mut DataFlowGraph);
 }
 
 // Include trait code generated by `lib/cretonne/meta/gen_instr.py`.
@@ -79,16 +69,7 @@ impl<'c, 'fc, 'fd> InstBuilderBase<'fd> for InsertBuilder<'c, 'fc, 'fd> {
         self.dfg
     }
 
-    fn simple_instruction(self, data: InstructionData) -> (Inst, &'fd mut DataFlowGraph) {
+    fn build(self, data: InstructionData, ctrl_typevar: Type) -> (Inst, &'fd mut DataFlowGraph) {
-        let inst = self.dfg.make_inst(data);
-        self.pos.insert_inst(inst);
-        (inst, self.dfg)
-    }
-
-    fn complex_instruction(self,
-                           data: InstructionData,
-                           ctrl_typevar: Type)
-                           -> (Inst, &'fd mut DataFlowGraph) {
         let inst = self.dfg.make_inst(data);
         self.dfg.make_inst_results(inst, ctrl_typevar);
         self.pos.insert_inst(inst);
@@ -98,20 +79,12 @@ impl<'c, 'fc, 'fd> InstBuilderBase<'fd> for InsertBuilder<'c, 'fc, 'fd> {
 
 /// Instruction builder that replaces an existing instruction.
 ///
-/// The inserted instruction will have the same `Inst` number as the old one. This is the only way
+/// The inserted instruction will have the same `Inst` number as the old one.
-/// of rewriting the first result value of an instruction since this is a `ExpandedValue::Direct`
-/// variant which encodes the instruction number directly.
 ///
-/// If the old instruction produced a value, the same value number will refer to the new
+/// If the old instruction still has result values attached, it is assumed that the new instruction
-/// instruction's first result, so if that value has any uses the type should stay the same.
+/// produces the same number and types of results. The old result values are preserved. If the
-///
+/// replacement instruction format does not support multiple results, the builder panics. It is a
-/// If the old instruction still has secondary result values attached, it is assumed that the new
+/// bug to leave result values dangling.
-/// instruction produces the same number and types of results. The old secondary values are
-/// preserved. If the replacement instruction format does not support multiple results, the builder
-/// panics. It is a bug to leave result values dangling.
-///
-/// If the old instruction was capable of producing secondary results, but the values have been
-/// detached, new result values are generated by calling `DataFlowGraph::make_inst_results()`.
 pub struct ReplaceBuilder<'f> {
     dfg: &'f mut DataFlowGraph,
     inst: Inst,
@@ -136,46 +109,20 @@ impl<'f> InstBuilderBase<'f> for ReplaceBuilder<'f> {
         self.dfg
     }
 
-    fn simple_instruction(self, data: InstructionData) -> (Inst, &'f mut DataFlowGraph) {
+    fn build(self, data: InstructionData, ctrl_typevar: Type) -> (Inst, &'f mut DataFlowGraph) {
-        // The replacement instruction cannot generate multiple results, so verify that the old
-        // instruction's secondary results have been detached.
-        let old_second_value = self.dfg[self.inst].second_result();
-        assert_eq!(old_second_value,
-                   None,
-                   "Secondary result values {:?} would be left dangling by replacing {} with {}",
-                   self.dfg.inst_results(self.inst),
-                   self.dfg[self.inst].opcode(),
-                   data.opcode());
-
-        // Splat the new instruction on top of the old one.
-        self.dfg[self.inst] = data;
-        (self.inst, self.dfg)
-    }
-
-    fn complex_instruction(self,
-                           data: InstructionData,
-                           ctrl_typevar: Type)
-                           -> (Inst, &'f mut DataFlowGraph) {
-        // If the old instruction still has secondary results attached, we'll keep them.
-        let old_second_value = self.dfg[self.inst].second_result();
-
         // Splat the new instruction on top of the old one.
         self.dfg[self.inst] = data;
 
-        if old_second_value.is_none() {
+        if !self.dfg.has_results(self.inst) {
-            // The old secondary values were either detached or non-existent.
+            // The old result values were either detached or non-existent.
-            // Construct new ones and set the first result type too.
+            // Construct new ones.
             self.dfg.make_inst_results(self.inst, ctrl_typevar);
         } else {
             // Reattach the old secondary values.
+            let old_second_value = self.dfg.inst_results(self.inst).get(1).cloned();
             if let Some(val_ref) = self.dfg[self.inst].second_result_mut() {
                 // Don't check types here. Leave that to the verifier.
                 *val_ref = old_second_value.into();
-            } else {
-                // Actually, this instruction format should have called `simple_instruction()`, but
-                // we don't have a rule against calling `complex_instruction()` even when it is
-                // overkill.
-                panic!("Secondary result values left dangling");
             }
 
             // Normally, make_inst_results() would also set the first result type, but we're not

lib/cretonne/src/ir/dfg.rs

@@ -633,7 +633,12 @@ impl DataFlowGraph {
             .expect("Instruction has no results")
     }
 
-    /// Iterate through all the results of an instruction.
+    /// Test if `inst` has any result values currently.
+    pub fn has_results(&self, inst: Inst) -> bool {
+        !self.results[inst].is_empty()
+    }
+
+    /// Return all the results of an instruction.
     pub fn inst_results(&self, inst: Inst) -> &[Value] {
         self.results[inst].as_slice(&self.value_lists)
     }

lib/cretonne/src/legalizer/split.rs

@@ -228,7 +228,7 @@ fn split_value(dfg: &mut DataFlowGraph,
                 // need to insert a split instruction before returning.
                 pos.goto_top(ebb);
                 pos.next_inst();
-                let concat_inst = dfg.ins(pos).Binary(concat, ty, lo, hi).0;
+                let concat_inst = dfg.ins(pos).Binary(concat, split_type, lo, hi).0;
                 let concat_val = dfg.first_result(concat_inst);
                 dfg.change_to_alias(value, concat_val);