From ae28ef90ef672dff53d986fa944d78642e59d0cc Mon Sep 17 00:00:00 2001 From: Jakob Stoklund Olesen Date: Thu, 5 Jan 2017 14:03:09 -0800 Subject: [PATCH] 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 bounds. - Implement the natural From 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::() 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 | 22 ++++++++++-------- lib/cretonne/src/ir/progpoint.rs | 39 ++++++++++++++++++++++++-------- 2 files changed, 42 insertions(+), 19 deletions(-) diff --git a/lib/cretonne/src/ir/layout.rs b/lib/cretonne/src/ir/layout.rs index 16f5aa5483..cecc8645ef 100644 --- a/lib/cretonne/src/ir/layout.rs +++ b/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 { - let a_seq = self.pp_seq(a); - let b_seq = self.pp_seq(b); + fn cmp(&self, a: A, b: B) -> cmp::Ordering + where A: Into, + B: Into + { + 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 { - match pp.expand() { + fn seq>(&self, pp: PP) -> SequenceNumber { + // 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.into(), i2.into()), Ordering::Less); - assert_eq!(layout.cmp(i3.into(), i2.into()), Ordering::Greater); + assert_eq!(layout.cmp(e2, e2), Ordering::Equal); + assert_eq!(layout.cmp(e2, i2), Ordering::Less); + assert_eq!(layout.cmp(i3, i2), Ordering::Greater); } } diff --git a/lib/cretonne/src/ir/progpoint.rs b/lib/cretonne/src/ir/progpoint.rs index c609a03d8f..ec2aa562cf 100644 --- a/lib/cretonne/src/ir/progpoint.rs +++ b/lib/cretonne/src/ir/progpoint.rs @@ -34,6 +34,7 @@ impl From 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 { - /// Expand compact program point representation. - pub fn expand(self) -> ExpandedProgramPoint { - if self.0 & 1 == 0 { - ExpandedProgramPoint::Inst(Inst::new((self.0 / 2) as usize)) +impl From for ExpandedProgramPoint { + fn from(inst: Inst) -> ExpandedProgramPoint { + ExpandedProgramPoint::Inst(inst) + } +} + +impl From for ExpandedProgramPoint { + fn from(ebb: Ebb) -> ExpandedProgramPoint { + ExpandedProgramPoint::Ebb(ebb) + } +} + +impl From 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` - /// appears in the program before `b`. - fn cmp(&self, a: ProgramPoint, b: ProgramPoint) -> cmp::Ordering; + /// Compare the program points `a` and `b` relative to this program order. + /// + /// 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(&self, a: A, b: B) -> cmp::Ordering + where A: Into, + B: Into; } #[cfg(test)]