Reorganize the instruction builder traits.

Leave the primary InstBuilderBase trait alone, but add an alternative
InstInserterBase trait that can be implemented instead by builders that
always allocate new instructions with dfg.make_inst().

Any implementation of InstInserterBase can be used as an instruction
builder by wrapping it in an InsertBuilder. The InsertBuilder type adds
additional functionality via the with_results() method which makes it
possible to override the result values on the instruction that is built.

The motivation for this shuffle is that the with_result() functionality
can now be reused by different kinds of instruction builders, as long as
they insert new instructions. So ReplaceBuilder doesn't get
with_results().

lib/cretonne/src/ir/builder.rs

@@ -4,7 +4,7 @@
 //! function. Many of its methods are generated from the meta language instruction definitions.
 
 use ir::types;
-use ir::{InstructionData, DataFlowGraph, Cursor, CursorBase};
+use ir::{InstructionData, DataFlowGraph};
 use ir::{Opcode, Type, Inst, Value, Ebb, JumpTable, SigRef, FuncRef, StackSlot, ValueList,
          MemFlags};
 use ir::immediates::{Imm64, Uimm8, Ieee32, Ieee64, Offset32, Uoffset32};
@@ -43,23 +43,44 @@ include!(concat!(env!("OUT_DIR"), "/builder.rs"));
 /// Any type implementing `InstBuilderBase` gets all the `InstBuilder` methods for free.
 impl<'f, T: InstBuilderBase<'f>> InstBuilder<'f> for T {}
 
-/// Builder that inserts an instruction at the current cursor position.
+/// Base trait for instruction inserters.
 ///
-/// An `InsertBuilder` holds mutable references to a data flow graph and a layout cursor. It
+/// This is an alternative base trait for an instruction builder to implement.
-/// provides convenience methods for creating and inserting instructions at the current cursor
+///
-/// position.
+/// An instruction inserter can be adapted into an instruction builder by wrapping it in an
-pub struct InsertBuilder<'c, 'fc: 'c, 'fd> {
+/// `InsertBuilder`. This provides some common functionality for instruction builders that insert
-    pos: &'c mut Cursor<'fc>,
+/// new instructions, as opposed to the `ReplaceBuilder` which overwrites existing instructions.
-    dfg: &'fd mut DataFlowGraph,
+pub trait InstInserterBase<'f>: Sized {
+    /// Get an immutable reference to the data flow graph.
+    fn data_flow_graph(&self) -> &DataFlowGraph;
+
+    /// Get a mutable reference to the data flow graph.
+    fn data_flow_graph_mut(&mut self) -> &mut DataFlowGraph;
+
+    /// Insert a new instruction which belongs to the DFG.
+    fn insert_built_inst(self, inst: Inst, ctrl_typevar: Type) -> &'f mut DataFlowGraph;
 }
 
-impl<'c, 'fc, 'fd> InsertBuilder<'c, 'fc, 'fd> {
+use std::marker::PhantomData;
+
+/// Builder that inserts an instruction at the current position.
+///
+/// An `InsertBuilder` is a wrapper for an `InstInserterBase` that turns it into an instruction
+/// builder with some additional facilities for creating instructions that reuse existing values as
+/// their results.
+pub struct InsertBuilder<'f, IIB: InstInserterBase<'f>> {
+    inserter: IIB,
+    unused: PhantomData<&'f u32>,
+}
+
+impl<'f, IIB: InstInserterBase<'f>> InsertBuilder<'f, IIB> {
     /// Create a new builder which inserts instructions at `pos`.
     /// The `dfg` and `pos.layout` references should be from the same `Function`.
-    pub fn new(dfg: &'fd mut DataFlowGraph,
+    pub fn new(inserter: IIB) -> InsertBuilder<'f, IIB> {
-               pos: &'c mut Cursor<'fc>)
+        InsertBuilder {
-               -> InsertBuilder<'c, 'fc, 'fd> {
+            inserter,
-        InsertBuilder { dfg, pos }
+            unused: PhantomData,
+        }
     }
 
     /// Reuse result values in `reuse`.
@@ -69,13 +90,13 @@ impl<'c, 'fc, 'fd> InsertBuilder<'c, 'fc, 'fd> {
     /// missing result values will be allocated as normal.
     ///
     /// The `reuse` argument is expected to be an array of `Option<Value>`.
-    pub fn with_results<Array>(self, reuse: Array) -> InsertReuseBuilder<'c, 'fc, 'fd, Array>
+    pub fn with_results<Array>(self, reuse: Array) -> InsertReuseBuilder<'f, IIB, Array>
         where Array: AsRef<[Option<Value>]>
     {
         InsertReuseBuilder {
-            dfg: self.dfg,
+            inserter: self.inserter,
-            pos: self.pos,
             reuse,
+            unused: PhantomData,
         }
     }
 
@@ -86,57 +107,65 @@ impl<'c, 'fc, 'fd> InsertBuilder<'c, 'fc, 'fd> {
     ///
     /// This method should only be used when building an instruction with exactly one result. Use
     /// `with_results()` for the more general case.
-    pub fn with_result(self, v: Value) -> InsertReuseBuilder<'c, 'fc, 'fd, [Option<Value>; 1]> {
+    pub fn with_result(self, v: Value) -> InsertReuseBuilder<'f, IIB, [Option<Value>; 1]> {
         // TODO: Specialize this to return a different builder that just attaches `v` instead of
         // calling `make_inst_results_reusing()`.
         self.with_results([Some(v)])
     }
 }
 
-impl<'c, 'fc, 'fd> InstBuilderBase<'fd> for InsertBuilder<'c, 'fc, 'fd> {
+impl<'f, IIB: InstInserterBase<'f>> InstBuilderBase<'f> for InsertBuilder<'f, IIB> {
     fn data_flow_graph(&self) -> &DataFlowGraph {
-        self.dfg
+        self.inserter.data_flow_graph()
     }
 
     fn data_flow_graph_mut(&mut self) -> &mut DataFlowGraph {
-        self.dfg
+        self.inserter.data_flow_graph_mut()
     }
 
-    fn build(self, data: InstructionData, ctrl_typevar: Type) -> (Inst, &'fd mut DataFlowGraph) {
+    fn build(mut self, data: InstructionData, ctrl_typevar: Type) -> (Inst, &'f mut DataFlowGraph) {
-        let inst = self.dfg.make_inst(data);
+        let inst;
-        self.dfg.make_inst_results(inst, ctrl_typevar);
+        {
-        self.pos.insert_inst(inst);
+            let dfg = self.inserter.data_flow_graph_mut();
-        (inst, self.dfg)
+            inst = dfg.make_inst(data);
+            dfg.make_inst_results(inst, ctrl_typevar);
+        }
+        (inst, self.inserter.insert_built_inst(inst, ctrl_typevar))
     }
 }
 
 /// Builder that inserts a new instruction like `InsertBuilder`, but reusing result values.
-pub struct InsertReuseBuilder<'c, 'fc: 'c, 'fd, Array>
+pub struct InsertReuseBuilder<'f, IIB, Array>
-    where Array: AsRef<[Option<Value>]>
+    where IIB: InstInserterBase<'f>,
+          Array: AsRef<[Option<Value>]>
 {
-    pos: &'c mut Cursor<'fc>,
+    inserter: IIB,
-    dfg: &'fd mut DataFlowGraph,
     reuse: Array,
+    unused: PhantomData<&'f u32>,
 }
 
-impl<'c, 'fc, 'fd, Array> InstBuilderBase<'fd> for InsertReuseBuilder<'c, 'fc, 'fd, Array>
+impl<'f, IIB, Array> InstBuilderBase<'f> for InsertReuseBuilder<'f, IIB, Array>
-    where Array: AsRef<[Option<Value>]>
+    where IIB: InstInserterBase<'f>,
+          Array: AsRef<[Option<Value>]>
 {
     fn data_flow_graph(&self) -> &DataFlowGraph {
-        self.dfg
+        self.inserter.data_flow_graph()
     }
 
     fn data_flow_graph_mut(&mut self) -> &mut DataFlowGraph {
-        self.dfg
+        self.inserter.data_flow_graph_mut()
     }
 
-    fn build(self, data: InstructionData, ctrl_typevar: Type) -> (Inst, &'fd mut DataFlowGraph) {
+    fn build(mut self, data: InstructionData, ctrl_typevar: Type) -> (Inst, &'f mut DataFlowGraph) {
-        let inst = self.dfg.make_inst(data);
+        let inst;
+        {
+            let dfg = self.inserter.data_flow_graph_mut();
+            inst = dfg.make_inst(data);
             // Make an `Interator<Item = Option<Value>>`.
             let ru = self.reuse.as_ref().iter().cloned();
-        self.dfg.make_inst_results_reusing(inst, ctrl_typevar, ru);
+            dfg.make_inst_results_reusing(inst, ctrl_typevar, ru);
-        self.pos.insert_inst(inst);
+        }
-        (inst, self.dfg)
+        (inst, self.inserter.insert_built_inst(inst, ctrl_typevar))
     }
 }
 

lib/cretonne/src/ir/dfg.rs

@@ -5,7 +5,7 @@ use isa::TargetIsa;
 use ir::builder::{InsertBuilder, ReplaceBuilder};
 use ir::extfunc::ExtFuncData;
 use ir::instructions::{Opcode, InstructionData, CallInfo};
-use ir::layout::Cursor;
+use ir::layout::{Cursor, LayoutCursorInserter};
 use ir::types;
 use ir::{Ebb, Inst, Value, Type, SigRef, Signature, FuncRef, ValueList, ValueListPool};
 use write::write_operands;
@@ -486,8 +486,8 @@ impl DataFlowGraph {
     /// Create an `InsertBuilder` that will insert an instruction at the cursor's current position.
     pub fn ins<'c, 'fc: 'c, 'fd>(&'fd mut self,
                                  at: &'c mut Cursor<'fc>)
-                                 -> InsertBuilder<'c, 'fc, 'fd> {
+                                 -> InsertBuilder<'fd, LayoutCursorInserter<'c, 'fc, 'fd>> {
-        InsertBuilder::new(self, at)
+        InsertBuilder::new(LayoutCursorInserter::new(at, self))
     }
 
     /// Create a `ReplaceBuilder` that will replace `inst` with a new instruction in place.

lib/cretonne/src/ir/layout.rs

@@ -7,7 +7,8 @@ use std::cmp;
 use std::iter::{Iterator, IntoIterator};
 use entity_map::EntityMap;
 use packed_option::PackedOption;
-use ir::entities::{Ebb, Inst};
+use ir::{Ebb, Inst, Type, DataFlowGraph};
+use ir::builder::InstInserterBase;
 use ir::progpoint::{ProgramOrder, ExpandedProgramPoint};
 
 /// The `Layout` struct determines the layout of EBBs and instructions in a function. It does not
@@ -1044,6 +1045,38 @@ impl<'f> Cursor<'f> {
     }
 }
 
+/// An instruction inserter which can be used to build and insert instructions at a cursor
+/// position.
+///
+/// This is used by `dfg.ins()`.
+pub struct LayoutCursorInserter<'c, 'fc: 'c, 'fd> {
+    pos: &'c mut Cursor<'fc>,
+    dfg: &'fd mut DataFlowGraph,
+}
+
+impl<'c, 'fc: 'c, 'fd> LayoutCursorInserter<'c, 'fc, 'fd> {
+    /// Create a new inserter. Don't use this, use `dfg.ins(pos)`.
+    pub fn new(pos: &'c mut Cursor<'fc>,
+               dfg: &'fd mut DataFlowGraph)
+               -> LayoutCursorInserter<'c, 'fc, 'fd> {
+        LayoutCursorInserter { pos, dfg }
+    }
+}
+
+impl<'c, 'fc: 'c, 'fd> InstInserterBase<'fd> for LayoutCursorInserter<'c, 'fc, 'fd> {
+    fn data_flow_graph(&self) -> &DataFlowGraph {
+        self.dfg
+    }
+
+    fn data_flow_graph_mut(&mut self) -> &mut DataFlowGraph {
+        self.dfg
+    }
+
+    fn insert_built_inst(self, inst: Inst, _ctrl_typevar: Type) -> &'fd mut DataFlowGraph {
+        self.pos.insert_inst(inst);
+        self.dfg
+    }
+}
 
 #[cfg(test)]
 mod tests {

lib/cretonne/src/ir/mod.rs

@@ -17,22 +17,21 @@ mod memflags;
 mod progpoint;
 mod valueloc;
 
-pub use ir::funcname::FunctionName;
+pub use ir::builder::{InstBuilder, InstBuilderBase, InstInserterBase, InsertBuilder};
+pub use ir::dfg::{DataFlowGraph, ValueDef};
+pub use ir::entities::{Ebb, Inst, Value, StackSlot, JumpTable, FuncRef, SigRef};
 pub use ir::extfunc::{Signature, CallConv, ArgumentType, ArgumentExtension, ArgumentPurpose,
                       ExtFuncData};
-pub use ir::types::Type;
+pub use ir::funcname::FunctionName;
-pub use ir::entities::{Ebb, Inst, Value, StackSlot, JumpTable, FuncRef, SigRef};
-pub use ir::instructions::{Opcode, InstructionData, VariableArgs, ValueList, ValueListPool};
-pub use ir::stackslot::{StackSlots, StackSlotKind, StackSlotData};
-pub use ir::jumptable::JumpTableData;
-pub use ir::valueloc::{ValueLoc, ArgumentLoc};
-pub use ir::dfg::{DataFlowGraph, ValueDef};
-pub use ir::layout::{Layout, CursorBase, Cursor};
 pub use ir::function::Function;
-pub use ir::builder::InstBuilder;
+pub use ir::instructions::{Opcode, InstructionData, VariableArgs, ValueList, ValueListPool};
-pub use ir::progpoint::{ProgramPoint, ProgramOrder, ExpandedProgramPoint};
+pub use ir::jumptable::JumpTableData;
+pub use ir::layout::{Layout, CursorBase, Cursor};
 pub use ir::memflags::MemFlags;
-pub use ir::builder::InstBuilderBase;
+pub use ir::progpoint::{ProgramPoint, ProgramOrder, ExpandedProgramPoint};
+pub use ir::stackslot::{StackSlots, StackSlotKind, StackSlotData};
+pub use ir::types::Type;
+pub use ir::valueloc::{ValueLoc, ArgumentLoc};
 
 use binemit;
 use entity_map::EntityMap;