Make sequence numbers local to instructions (#6043)

* Only allow pp_cmp within a single block

Block order shouldn't matter for codegen and restricting pp_cmp to a
single block will allow making instruction sequence numbers local to a
block.

* Make sequence numbers local to instructions

This allows renumbering to be localized to a single block where previously it
could affect the entire function. Also saves 32bit of overhead per block.

cranelift/codegen/src/dominator_tree.rs

@@ -106,7 +106,7 @@ impl DominatorTree {
         let a = a.into();
         let b = b.into();
         self.rpo_cmp_block(layout.pp_block(a), layout.pp_block(b))
-            .then(layout.pp_cmp(a, b))
+            .then_with(|| layout.pp_cmp(a, b))
     }
 
     /// Returns `true` if `a` dominates `b`.
@@ -578,7 +578,7 @@ impl DominatorTreePreorder {
         let a = a.into();
         let b = b.into();
         self.pre_cmp_block(layout.pp_block(a), layout.pp_block(b))
-            .then(layout.pp_cmp(a, b))
+            .then_with(|| layout.pp_cmp(a, b))
     }
 }
 

cranelift/codegen/src/incremental_cache.rs

@@ -164,7 +164,15 @@ impl<'a> CacheKey<'a> {
         // Make sure the blocks and instructions are sequenced the same way as we might
         // have serialized them earlier. This is the symmetric of what's done in
         // `try_load`.
-        f.stencil.layout.full_renumber();
+        let mut block = f.stencil.layout.entry_block().expect("Missing entry block");
+        loop {
+            f.stencil.layout.full_block_renumber(block);
+            if let Some(next_block) = f.stencil.layout.next_block(block) {
+                block = next_block;
+            } else {
+                break;
+            }
+        }
         CacheKey {
             stencil: &f.stencil,
             parameters: CompileParameters::from_isa(isa),

cranelift/codegen/src/ir/layout.rs

@@ -68,13 +68,11 @@ impl Layout {
 
 /// Sequence numbers.
 ///
-/// All instructions and blocks are given a sequence number that can be used to quickly determine
+/// All instructions are given a sequence number that can be used to quickly determine
-/// their relative position in the layout. The sequence numbers are not contiguous, but are assigned
+/// their relative position in a block. The sequence numbers are not contiguous, but are assigned
 /// like line numbers in BASIC: 10, 20, 30, ...
 ///
-/// The block sequence numbers are strictly increasing, and so are the instruction sequence numbers
+/// Sequence numbers are strictly increasing within a block, but are reset between blocks.
-/// within a block. The instruction sequence numbers are all between the sequence number of their
-/// containing block and the following block.
 ///
 /// The result is that sequence numbers work like BASIC line numbers for the textual form of the IR.
 type SequenceNumber = u32;
@@ -86,7 +84,7 @@ const MAJOR_STRIDE: SequenceNumber = 10;
 const MINOR_STRIDE: SequenceNumber = 2;
 
 /// Limit on the sequence number range we'll renumber locally. If this limit is exceeded, we'll
-/// switch to a full function renumbering.
+/// switch to a full block renumbering.
 const LOCAL_LIMIT: SequenceNumber = 100 * MINOR_STRIDE;
 
 /// Compute the midpoint between `a` and `b`.
@@ -115,7 +113,7 @@ fn test_midpoint() {
 }
 
 impl Layout {
-    /// Compare the program points `a` and `b` relative to this program order.
+    /// Compare the program points `a` and `b` in the same block relative to this program order.
     ///
     /// Return `Less` if `a` appears in the program before `b`.
     ///
@@ -127,81 +125,35 @@ impl Layout {
         A: Into<ProgramPoint>,
         B: Into<ProgramPoint>,
     {
-        let a_seq = self.seq(a);
+        let a = a.into();
-        let b_seq = self.seq(b);
+        let b = b.into();
+        debug_assert_eq!(self.pp_block(a), self.pp_block(b));
+        let a_seq = match a {
+            ProgramPoint::Block(_block) => 0,
+            ProgramPoint::Inst(inst) => self.insts[inst].seq,
+        };
+        let b_seq = match b {
+            ProgramPoint::Block(_block) => 0,
+            ProgramPoint::Inst(inst) => self.insts[inst].seq,
+        };
         a_seq.cmp(&b_seq)
     }
 }
 
 // Private methods for dealing with sequence numbers.
 impl Layout {
-    /// Get the sequence number of a program point that must correspond to an entity in the layout.
-    fn seq<PP: Into<ProgramPoint>>(&self, pp: PP) -> SequenceNumber {
-        // When `PP = Inst` or `PP = Block`, we expect this dynamic type check to be optimized out.
-        match pp.into() {
-            ProgramPoint::Block(block) => self.blocks[block].seq,
-            ProgramPoint::Inst(inst) => self.insts[inst].seq,
-        }
-    }
-
-    /// Get the last sequence number in `block`.
-    fn last_block_seq(&self, block: Block) -> SequenceNumber {
-        // Get the seq of the last instruction if it exists, otherwise use the block header seq.
-        self.blocks[block]
-            .last_inst
-            .map(|inst| self.insts[inst].seq)
-            .unwrap_or(self.blocks[block].seq)
-    }
-
-    /// Assign a valid sequence number to `block` such that the numbers are still monotonic. This may
-    /// require renumbering.
-    fn assign_block_seq(&mut self, block: Block) {
-        debug_assert!(self.is_block_inserted(block));
-
-        // Get the sequence number immediately before `block`, or 0.
-        let prev_seq = self.blocks[block]
-            .prev
-            .map(|prev_block| self.last_block_seq(prev_block))
-            .unwrap_or(0);
-
-        // Get the sequence number immediately following `block`.
-        let next_seq = if let Some(inst) = self.blocks[block].first_inst.expand() {
-            self.insts[inst].seq
-        } else if let Some(next_block) = self.blocks[block].next.expand() {
-            self.blocks[next_block].seq
-        } else {
-            // There is nothing after `block`. We can just use a major stride.
-            self.blocks[block].seq = prev_seq + MAJOR_STRIDE;
-            return;
-        };
-
-        // Check if there is room between these sequence numbers.
-        if let Some(seq) = midpoint(prev_seq, next_seq) {
-            self.blocks[block].seq = seq;
-        } else {
-            // No available integers between `prev_seq` and `next_seq`. We have to renumber.
-            self.renumber_from_block(block, prev_seq + MINOR_STRIDE, prev_seq + LOCAL_LIMIT);
-        }
-    }
-
     /// Assign a valid sequence number to `inst` such that the numbers are still monotonic. This may
     /// require renumbering.
     fn assign_inst_seq(&mut self, inst: Inst) {
-        let block = self
-            .inst_block(inst)
-            .expect("inst must be inserted before assigning an seq");
-
         // Get the sequence number immediately before `inst`.
         let prev_seq = match self.insts[inst].prev.expand() {
             Some(prev_inst) => self.insts[prev_inst].seq,
-            None => self.blocks[block].seq,
+            None => 0,
         };
 
         // Get the sequence number immediately following `inst`.
         let next_seq = if let Some(next_inst) = self.insts[inst].next.expand() {
             self.insts[next_inst].seq
-        } else if let Some(next_block) = self.blocks[block].next.expand() {
-            self.blocks[next_block].seq
         } else {
             // There is nothing after `inst`. We can just use a major stride.
             self.insts[inst].seq = prev_seq + MAJOR_STRIDE;
@@ -213,23 +165,15 @@ impl Layout {
             self.insts[inst].seq = seq;
         } else {
             // No available integers between `prev_seq` and `next_seq`. We have to renumber.
-            self.renumber_from_inst(inst, prev_seq + MINOR_STRIDE, prev_seq + LOCAL_LIMIT);
+            self.renumber_insts(inst, prev_seq + MINOR_STRIDE, prev_seq + LOCAL_LIMIT);
         }
     }
 
     /// Renumber instructions starting from `inst` until the end of the block or until numbers catch
     /// up.
     ///
-    /// Return `None` if renumbering has caught up and the sequence is monotonic again. Otherwise
+    /// If sequence numbers exceed `limit`, switch to a full block renumbering.
-    /// return the last used sequence number.
+    fn renumber_insts(&mut self, inst: Inst, seq: SequenceNumber, limit: SequenceNumber) {
-    ///
-    /// If sequence numbers exceed `limit`, switch to a full function renumbering and return `None`.
-    fn renumber_insts(
-        &mut self,
-        inst: Inst,
-        seq: SequenceNumber,
-        limit: SequenceNumber,
-    ) -> Option<SequenceNumber> {
         let mut inst = inst;
         let mut seq = seq;
 
@@ -238,56 +182,22 @@ impl Layout {
 
             // Next instruction.
             inst = match self.insts[inst].next.expand() {
-                None => return Some(seq),
+                None => return,
                 Some(next) => next,
             };
 
             if seq < self.insts[inst].seq {
                 // Sequence caught up.
-                return None;
+                return;
             }
 
             if seq > limit {
                 // We're pushing too many instructions in front of us.
-                // Switch to a full function renumbering to make some space.
+                // Switch to a full block renumbering to make some space.
-                self.full_renumber();
+                self.full_block_renumber(
-                return None;
+                    self.inst_block(inst)
-            }
+                        .expect("inst must be inserted before assigning an seq"),
-
+                );
-            seq += MINOR_STRIDE;
-        }
-    }
-
-    /// Renumber starting from `block` to `seq` and continuing until the sequence numbers are
-    /// monotonic again.
-    fn renumber_from_block(
-        &mut self,
-        block: Block,
-        first_seq: SequenceNumber,
-        limit: SequenceNumber,
-    ) {
-        let mut block = block;
-        let mut seq = first_seq;
-
-        loop {
-            self.blocks[block].seq = seq;
-
-            // Renumber instructions in `block`. Stop when the numbers catch up.
-            if let Some(inst) = self.blocks[block].first_inst.expand() {
-                seq = match self.renumber_insts(inst, seq + MINOR_STRIDE, limit) {
-                    Some(s) => s,
-                    None => return,
-                }
-            }
-
-            // Advance to the next block.
-            block = match self.blocks[block].next.expand() {
-                Some(next) => next,
-                None => return,
-            };
-
-            // Stop renumbering once the numbers catch up.
-            if seq < self.blocks[block].seq {
                 return;
             }
 
@@ -295,37 +205,21 @@ impl Layout {
         }
     }
 
-    /// Renumber starting from `inst` to `seq` and continuing until the sequence numbers are
+    /// Renumber all instructions in a block.
-    /// monotonic again.
-    fn renumber_from_inst(&mut self, inst: Inst, first_seq: SequenceNumber, limit: SequenceNumber) {
-        if let Some(seq) = self.renumber_insts(inst, first_seq, limit) {
-            // Renumbering spills over into next block.
-            if let Some(next_block) = self.blocks[self.inst_block(inst).unwrap()].next.expand() {
-                self.renumber_from_block(next_block, seq + MINOR_STRIDE, limit);
-            }
-        }
-    }
-
-    /// Renumber all blocks and instructions in the layout.
     ///
     /// This doesn't affect the position of anything, but it gives more room in the internal
     /// sequence numbers for inserting instructions later.
-    pub(crate) fn full_renumber(&mut self) {
+    pub(crate) fn full_block_renumber(&mut self, block: Block) {
         let _tt = timing::layout_renumber();
-        let mut seq = 0;
+        // Avoid 0 as this is reserved for the program point indicating the block itself
-        let mut next_block = self.first_block;
+        let mut seq = MAJOR_STRIDE;
-        while let Some(block) = next_block {
-            self.blocks[block].seq = seq;
-            seq += MAJOR_STRIDE;
-            next_block = self.blocks[block].next.expand();
-
         let mut next_inst = self.blocks[block].first_inst.expand();
         while let Some(inst) = next_inst {
             self.insts[inst].seq = seq;
             seq += MAJOR_STRIDE;
             next_inst = self.insts[inst].next.expand();
         }
-        }
+
         trace!("Renumbered {} program points", seq / MAJOR_STRIDE);
     }
 }
@@ -363,7 +257,6 @@ impl Layout {
             self.first_block = Some(block);
         }
         self.last_block = Some(block);
-        self.assign_block_seq(block);
     }
 
     /// Insert `block` in the layout before the existing block `before`.
@@ -387,7 +280,6 @@ impl Layout {
             None => self.first_block = Some(block),
             Some(a) => self.blocks[a].next = block.into(),
         }
-        self.assign_block_seq(block);
     }
 
     /// Insert `block` in the layout *after* the existing block `after`.
@@ -411,7 +303,6 @@ impl Layout {
             None => self.last_block = Some(block),
             Some(b) => self.blocks[b].prev = block.into(),
         }
-        self.assign_block_seq(block);
     }
 
     /// Remove `block` from the layout.
@@ -491,7 +382,6 @@ struct BlockNode {
     next: PackedOption<Block>,
     first_inst: PackedOption<Inst>,
     last_inst: PackedOption<Inst>,
-    seq: SequenceNumber,
     cold: bool,
 }
 
@@ -706,8 +596,6 @@ impl Layout {
             self.insts[i].block = new_block.into();
             opt_i = self.insts[i].next.into();
         }
-
-        self.assign_block_seq(new_block);
     }
 }
 
@@ -914,14 +802,12 @@ mod tests {
         // Check forward linkage with iterators.
         // Check that layout sequence numbers are strictly monotonic.
         {
-            let mut seq = 0;
             let mut block_iter = layout.blocks();
             for &(block, insts) in blocks {
                 assert!(layout.is_block_inserted(block));
                 assert_eq!(block_iter.next(), Some(block));
-                assert!(layout.blocks[block].seq > seq);
-                seq = layout.blocks[block].seq;
 
+                let mut seq = 0;
                 let mut inst_iter = layout.block_insts(block);
                 for &inst in insts {
                     assert_eq!(layout.inst_block(inst), Some(block));