Refactor inserted move handling (#135)

* Return inserted moves from move insertion

* Avoid adding moves to the queue with the same source and dest

Author: elliottt

src/ion/data_structures.rs

@@ -434,8 +434,6 @@ pub struct Env<'a, F: Function> {
     // was to the approprate PReg.
     pub multi_fixed_reg_fixups: Vec<MultiFixedRegFixup>,
 
-    pub inserted_moves: Vec<InsertedMove>,
-
     // Output:
     pub edits: Vec<(PosWithPrio, Edit)>,
     pub allocs: Vec<Allocation>,
@@ -652,6 +650,50 @@ pub enum InsertMovePrio {
     OutEdgeMoves,
 }
 
+#[derive(Debug, Default)]
+pub struct InsertedMoves {
+    pub moves: Vec<InsertedMove>,
+}
+
+impl InsertedMoves {
+    pub fn push(
+        &mut self,
+        pos: ProgPoint,
+        prio: InsertMovePrio,
+        from_alloc: Allocation,
+        to_alloc: Allocation,
+        to_vreg: VReg,
+    ) {
+        trace!(
+            "insert_move: pos {:?} prio {:?} from_alloc {:?} to_alloc {:?} to_vreg {:?}",
+            pos,
+            prio,
+            from_alloc,
+            to_alloc,
+            to_vreg
+        );
+        if from_alloc == to_alloc {
+            trace!(" -> skipping move with same source and  dest");
+            return;
+        }
+        if let Some(from) = from_alloc.as_reg() {
+            debug_assert_eq!(from.class(), to_vreg.class());
+        }
+        if let Some(to) = to_alloc.as_reg() {
+            debug_assert_eq!(to.class(), to_vreg.class());
+        }
+        self.moves.push(InsertedMove {
+            pos_prio: PosWithPrio {
+                pos,
+                prio: prio as u32,
+            },
+            from_alloc,
+            to_alloc,
+            to_vreg,
+        });
+    }
+}
+
 /// The fields in this struct are reversed in sort order so that the entire
 /// struct can be treated as a u64 for sorting purposes.
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]

src/ion/mod.rs

@@ -74,7 +74,6 @@ impl<'a, F: Function> Env<'a, F> {
             preferred_victim_by_class: [PReg::invalid(), PReg::invalid(), PReg::invalid()],
 
             multi_fixed_reg_fixups: vec![],
-            inserted_moves: vec![],
             edits: Vec::with_capacity(n),
             allocs: Vec::with_capacity(4 * n),
             inst_alloc_offsets: vec![],
@@ -108,8 +107,8 @@ impl<'a, F: Function> Env<'a, F> {
         self.process_bundles()?;
         self.try_allocating_regs_for_spilled_bundles();
         self.allocate_spillslots();
-        self.apply_allocations_and_insert_moves();
-        self.resolve_inserted_moves();
+        let moves = self.apply_allocations_and_insert_moves();
+        self.resolve_inserted_moves(moves);
         self.compute_stackmaps();
         Ok(())
     }

src/ion/moves.rs

@@ -13,8 +13,8 @@
 //! Move resolution.
 
 use super::{
-    Env, InsertMovePrio, InsertedMove, LiveRangeFlag, LiveRangeIndex, RedundantMoveEliminator,
-    VRegIndex, SLOT_NONE,
+    Env, InsertMovePrio, InsertedMove, InsertedMoves, LiveRangeFlag, LiveRangeIndex,
+    RedundantMoveEliminator, VRegIndex, SLOT_NONE,
 };
 use crate::ion::data_structures::{
     u128_key, u64_key, BlockparamIn, BlockparamOut, CodeRange, FixedRegFixupLevel, LiveRangeKey,
@@ -24,7 +24,7 @@ use crate::ion::reg_traversal::RegTraversalIter;
 use crate::moves::{MoveAndScratchResolver, ParallelMoves};
 use crate::{
     Allocation, Block, Edit, Function, FxHashMap, Inst, InstPosition, OperandConstraint,
-    OperandKind, OperandPos, PReg, ProgPoint, RegClass, SpillSlot, VReg,
+    OperandKind, OperandPos, PReg, ProgPoint, RegClass, SpillSlot,
 };
 use alloc::vec::Vec;
 use alloc::{format, vec};
@@ -41,39 +41,6 @@ impl<'a, F: Function> Env<'a, F> {
         pos == self.cfginfo.block_exit[block.index()]
     }
 
-    pub fn insert_move(
-        &mut self,
-        pos: ProgPoint,
-        prio: InsertMovePrio,
-        from_alloc: Allocation,
-        to_alloc: Allocation,
-        to_vreg: VReg,
-    ) {
-        trace!(
-            "insert_move: pos {:?} prio {:?} from_alloc {:?} to_alloc {:?} to_vreg {:?}",
-            pos,
-            prio,
-            from_alloc,
-            to_alloc,
-            to_vreg
-        );
-        if let Some(from) = from_alloc.as_reg() {
-            debug_assert_eq!(from.class(), to_vreg.class());
-        }
-        if let Some(to) = to_alloc.as_reg() {
-            debug_assert_eq!(to.class(), to_vreg.class());
-        }
-        self.inserted_moves.push(InsertedMove {
-            pos_prio: PosWithPrio {
-                pos,
-                prio: prio as u32,
-            },
-            from_alloc,
-            to_alloc,
-            to_vreg,
-        });
-    }
-
     pub fn get_alloc(&self, inst: Inst, slot: usize) -> Allocation {
         let inst_allocs = &self.allocs[self.inst_alloc_offsets[inst.index()] as usize..];
         inst_allocs[slot]
@@ -102,11 +69,13 @@ impl<'a, F: Function> Env<'a, F> {
         }
     }
 
-    pub fn apply_allocations_and_insert_moves(&mut self) {
+    pub fn apply_allocations_and_insert_moves(&mut self) -> InsertedMoves {
         trace!("apply_allocations_and_insert_moves");
         trace!("blockparam_ins: {:?}", self.blockparam_ins);
         trace!("blockparam_outs: {:?}", self.blockparam_outs);
 
+        let mut inserted_moves = InsertedMoves::default();
+
         // Now that all splits are done, we can pay the cost once to
         // sort VReg range lists and update with the final ranges.
         for vreg in &mut self.vregs {
@@ -394,7 +363,7 @@ impl<'a, F: Function> Env<'a, F> {
                             vreg.index()
                         );
                         debug_assert_eq!(range.from.pos(), InstPosition::Before);
-                        self.insert_move(
+                        inserted_moves.push(
                             range.from,
                             InsertMovePrio::Regular,
                             prev_alloc,
@@ -670,7 +639,7 @@ impl<'a, F: Function> Env<'a, F> {
                     );
 
                     let (pos, prio) = choose_move_location(self, dest.from, dest.to);
-                    self.insert_move(pos, prio, src, dest.alloc, vreg);
+                    inserted_moves.push(pos, prio, src, dest.alloc, vreg);
                 }
             }
 
@@ -710,7 +679,13 @@ impl<'a, F: Function> Env<'a, F> {
 
                         let (pos, prio) =
                             choose_move_location(self, dest.from_block, dest.to_block);
-                        self.insert_move(pos, prio, src.alloc, dest.alloc, self.vreg(dest.to_vreg));
+                        inserted_moves.push(
+                            pos,
+                            prio,
+                            src.alloc,
+                            dest.alloc,
+                            self.vreg(dest.to_vreg),
+                        );
 
                         // We don't advance the block_param_sources iterator here because there
                         // could be additional destinations that would take from that source. Thus,
@@ -740,7 +715,7 @@ impl<'a, F: Function> Env<'a, F> {
                 FixedRegFixupLevel::Initial => InsertMovePrio::MultiFixedRegInitial,
                 FixedRegFixupLevel::Secondary => InsertMovePrio::MultiFixedRegSecondary,
             };
-            self.insert_move(fixup.pos, prio, from_alloc, to_alloc, self.vreg(fixup.vreg));
+            inserted_moves.push(fixup.pos, prio, from_alloc, to_alloc, self.vreg(fixup.vreg));
             self.set_alloc(
                 fixup.pos.inst(),
                 fixup.to_slot as usize,
@@ -819,7 +794,7 @@ impl<'a, F: Function> Env<'a, F> {
                             );
                         }
                         let input_operand = self.func.inst_operands(inst)[input_idx];
-                        self.insert_move(
+                        inserted_moves.push(
                             ProgPoint::before(inst),
                             InsertMovePrio::ReusedInput,
                             input_alloc,
@@ -835,13 +810,16 @@ impl<'a, F: Function> Env<'a, F> {
         // Sort the debug-locations vector; we provide this
         // invariant to the client.
         self.debug_locations.sort_unstable();
+
+        inserted_moves
     }
 
-    pub fn resolve_inserted_moves(&mut self) {
+    pub fn resolve_inserted_moves(&mut self, mut inserted_moves: InsertedMoves) {
         // For each program point, gather all moves together. Then
         // resolve (see cases below).
         let mut i = 0;
-        self.inserted_moves
+        inserted_moves
+            .moves
             .sort_unstable_by_key(|m| m.pos_prio.key());
 
         // Redundant-move elimination state tracker.
@@ -897,13 +875,13 @@ impl<'a, F: Function> Env<'a, F> {
 
         let mut last_pos = ProgPoint::before(Inst::new(0));
 
-        while i < self.inserted_moves.len() {
+        while i < inserted_moves.moves.len() {
             let start = i;
-            let pos_prio = self.inserted_moves[i].pos_prio;
-            while i < self.inserted_moves.len() && self.inserted_moves[i].pos_prio == pos_prio {
+            let pos_prio = inserted_moves.moves[i].pos_prio;
+            while i < inserted_moves.moves.len() && inserted_moves.moves[i].pos_prio == pos_prio {
                 i += 1;
             }
-            let moves = &self.inserted_moves[start..i];
+            let moves = &inserted_moves.moves[start..i];
 
             redundant_move_process_side_effects(self, &mut redundant_moves, last_pos, pos_prio.pos);
             last_pos = pos_prio.pos;
@@ -919,9 +897,6 @@ impl<'a, F: Function> Env<'a, F> {
             let mut vec_moves: SmallVec<[InsertedMove; 8]> = smallvec![];
 
             for m in moves {
-                if m.from_alloc == m.to_alloc {
-                    continue;
-                }
                 match m.to_vreg.class() {
                     RegClass::Int => {
                         int_moves.push(m.clone());