Encourage better optimization of ProgramOrder::cmp.

The ProgramOrder::cmp() comparison is often used where one or both
arguments are statically known to be an Inst or Ebb. Give the compiler a
better chance to discover this via inlining and other optimizations.

- Make cmp() generic with Into<ExpandedProgramPoint> bounds.
- Implement the natural From<T> traits for ExpandedProgramPoint.
- Make Layout::pp_seq() generic with the same bound.

Now, with inlining and constant folding, passing an Inst argument to
PO::cmp() will result in a call to a monomorphized Layout::seq::<Inst>()
which can avoid the dynamic match to select a table for looking up the
sequence number.

The result is that comparing two program points of statically known type
results in two direct table lookups and a sequence number comparison.

This all uses ExpandedProgramPoint because it is more likely to be
transparent to the constant folder than the bit-packed ProgramPoint
type.

lib/cretonne/src/ir/layout.rs

@@ -7,7 +7,7 @@ use std::cmp;
 use std::iter::{Iterator, IntoIterator};
 use entity_map::{EntityMap, EntityRef};
 use ir::entities::{Ebb, NO_EBB, Inst, NO_INST};
-use ir::progpoint::{ProgramPoint, ProgramOrder, ExpandedProgramPoint};
+use ir::progpoint::{ProgramOrder, ExpandedProgramPoint};
 
 /// The `Layout` struct determines the layout of EBBs and instructions in a function. It does not
 /// contain definitions of instructions or EBBs, but depends on `Inst` and `Ebb` entity references
@@ -93,9 +93,12 @@ fn test_midpoint() {
 }
 
 impl ProgramOrder for Layout {
-    fn cmp(&self, a: ProgramPoint, b: ProgramPoint) -> cmp::Ordering {
+    fn cmp<A, B>(&self, a: A, b: B) -> cmp::Ordering
-        let a_seq = self.pp_seq(a);
+        where A: Into<ExpandedProgramPoint>,
-        let b_seq = self.pp_seq(b);
+              B: Into<ExpandedProgramPoint>
+    {
+        let a_seq = self.seq(a);
+        let b_seq = self.seq(b);
         a_seq.cmp(&b_seq)
     }
 }
@@ -103,8 +106,9 @@ impl ProgramOrder for Layout {
 // 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 pp_seq(&self, pp: ProgramPoint) -> SequenceNumber {
+    fn seq<PP: Into<ExpandedProgramPoint>>(&self, pp: PP) -> SequenceNumber {
-        match pp.expand() {
+        // When `PP = Inst` or `PP = Ebb`, we expect this dynamic type check to be optimized out.
+        match pp.into() {
             ExpandedProgramPoint::Ebb(ebb) => self.ebbs[ebb].seq,
             ExpandedProgramPoint::Inst(inst) => self.insts[inst].seq,
         }
@@ -1240,8 +1244,8 @@ mod tests {
         }
 
         // Check ProgramOrder.
-        assert_eq!(layout.cmp(e2.into(), e2.into()), Ordering::Equal);
+        assert_eq!(layout.cmp(e2, e2), Ordering::Equal);
-        assert_eq!(layout.cmp(e2.into(), i2.into()), Ordering::Less);
+        assert_eq!(layout.cmp(e2, i2), Ordering::Less);
-        assert_eq!(layout.cmp(i3.into(), i2.into()), Ordering::Greater);
+        assert_eq!(layout.cmp(i3, i2), Ordering::Greater);
     }
 }

lib/cretonne/src/ir/progpoint.rs

@@ -34,6 +34,7 @@ impl From<Ebb> for ProgramPoint {
 
 /// An expanded program point directly exposes the variants, but takes twice the space to
 /// represent.
+#[derive(PartialEq, Eq, Clone, Copy)]
 pub enum ExpandedProgramPoint {
     // An instruction in the function.
     Inst(Inst),
@@ -41,20 +42,31 @@ pub enum ExpandedProgramPoint {
     Ebb(Ebb),
 }
 
-impl ProgramPoint {
+impl From<Inst> for ExpandedProgramPoint {
-    /// Expand compact program point representation.
+    fn from(inst: Inst) -> ExpandedProgramPoint {
-    pub fn expand(self) -> ExpandedProgramPoint {
+        ExpandedProgramPoint::Inst(inst)
-        if self.0 & 1 == 0 {
+    }
-            ExpandedProgramPoint::Inst(Inst::new((self.0 / 2) as usize))
+}
+
+impl From<Ebb> for ExpandedProgramPoint {
+    fn from(ebb: Ebb) -> ExpandedProgramPoint {
+        ExpandedProgramPoint::Ebb(ebb)
+    }
+}
+
+impl From<ProgramPoint> for ExpandedProgramPoint {
+    fn from(pp: ProgramPoint) -> ExpandedProgramPoint {
+        if pp.0 & 1 == 0 {
+            ExpandedProgramPoint::Inst(Inst::new((pp.0 / 2) as usize))
         } else {
-            ExpandedProgramPoint::Ebb(Ebb::new((self.0 / 2) as usize))
+            ExpandedProgramPoint::Ebb(Ebb::new((pp.0 / 2) as usize))
         }
     }
 }
 
 impl fmt::Display for ProgramPoint {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match self.expand() {
+        match (*self).into() {
             ExpandedProgramPoint::Inst(x) => write!(f, "{}", x),
             ExpandedProgramPoint::Ebb(x) => write!(f, "{}", x),
         }
@@ -72,9 +84,16 @@ impl fmt::Debug for ProgramPoint {
 /// `ProgramPoint` objects don't carry enough information to be ordered independently, they need a
 /// context providing the program order.
 pub trait ProgramOrder {
-    /// Compare the program points `a` and `b` relative to this program order. Return `Less` if `a`
+    /// Compare the program points `a` and `b` relative to this program order.
-    /// appears in the program before `b`.
+    ///
-    fn cmp(&self, a: ProgramPoint, b: ProgramPoint) -> cmp::Ordering;
+    /// Return `Less` if `a` appears in the program before `b`.
+    ///
+    /// This is declared as a generic such that it can be called with `Inst` and `Ebb` arguments
+    /// directly. Depending on the implementation, there is a good chance performance will be
+    /// improved for those cases where the type of either argument is known statically.
+    fn cmp<A, B>(&self, a: A, b: B) -> cmp::Ordering
+        where A: Into<ExpandedProgramPoint>,
+              B: Into<ExpandedProgramPoint>;
 }
 
 #[cfg(test)]