Use an FxHashMap in RegDiversions.

Because it's hot and the number of entries can reach the 1000s, so
linear insertion and search is bad.

This reduces runtime for `sqlite` and `UE4Game-HTML5-Shipping` by 3-4%,
and a couple of other benchmarks (`sqlite`, `godot`, `clang`) by smaller
amounts.

It also increases runtime for `mono` and `tanks` by about 1%; this seems
to be due to incidental changes in which functions are inlined more than
algorithmic changes.
This commit is contained in:
Nicholas Nethercote
2018-12-14 09:34:23 +11:00
committed by Dan Gohman
parent c9666381f6
commit 46d9a3cd1a
4 changed files with 40 additions and 36 deletions

View File

@@ -654,10 +654,10 @@ impl<'a> Context<'a> {
where where
Pred: FnMut(&LiveRange, LiveRangeContext<Layout>) -> bool, Pred: FnMut(&LiveRange, LiveRangeContext<Layout>) -> bool,
{ {
for rdiv in self.divert.all() { for (&value, rdiv) in self.divert.iter() {
let lr = self let lr = self
.liveness .liveness
.get(rdiv.value) .get(value)
.expect("Missing live range for diverted register"); .expect("Missing live range for diverted register");
if pred(lr, self.liveness.context(&self.cur.func.layout)) { if pred(lr, self.liveness.context(&self.cur.func.layout)) {
if let Affinity::Reg(rci) = lr.affinity { if let Affinity::Reg(rci) = lr.affinity {
@@ -665,7 +665,7 @@ impl<'a> Context<'a> {
// Stack diversions should not be possible here. The only live transiently // Stack diversions should not be possible here. The only live transiently
// during `shuffle_inputs()`. // during `shuffle_inputs()`.
self.solver.reassign_in( self.solver.reassign_in(
rdiv.value, value,
rc, rc,
rdiv.to.unwrap_reg(), rdiv.to.unwrap_reg(),
rdiv.from.unwrap_reg(), rdiv.from.unwrap_reg(),
@@ -673,7 +673,7 @@ impl<'a> Context<'a> {
} else { } else {
panic!( panic!(
"Diverted register {} with {} affinity", "Diverted register {} with {} affinity",
rdiv.value, value,
lr.affinity.display(&self.reginfo) lr.affinity.display(&self.reginfo)
); );
} }

View File

@@ -7,11 +7,12 @@
//! These register diversions are local to an EBB. No values can be diverted when entering a new //! These register diversions are local to an EBB. No values can be diverted when entering a new
//! EBB. //! EBB.
use fx::FxHashMap;
use ir::{InstructionData, Opcode}; use ir::{InstructionData, Opcode};
use ir::{StackSlot, Value, ValueLoc, ValueLocations}; use ir::{StackSlot, Value, ValueLoc, ValueLocations};
use isa::{RegInfo, RegUnit}; use isa::{RegInfo, RegUnit};
use std::collections::hash_map::{Entry, Iter};
use std::fmt; use std::fmt;
use std::vec::Vec;
/// A diversion of a value from its original location to a new register or stack location. /// A diversion of a value from its original location to a new register or stack location.
/// ///
@@ -22,8 +23,6 @@ use std::vec::Vec;
/// the current one. /// the current one.
#[derive(Clone, Copy, Debug, PartialEq, Eq)] #[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct Diversion { pub struct Diversion {
/// The value that is diverted.
pub value: Value,
/// The original value location. /// The original value location.
pub from: ValueLoc, pub from: ValueLoc,
/// The current value location. /// The current value location.
@@ -32,22 +31,22 @@ pub struct Diversion {
impl Diversion { impl Diversion {
/// Make a new diversion. /// Make a new diversion.
pub fn new(value: Value, from: ValueLoc, to: ValueLoc) -> Self { pub fn new(from: ValueLoc, to: ValueLoc) -> Self {
debug_assert!(from.is_assigned() && to.is_assigned()); debug_assert!(from.is_assigned() && to.is_assigned());
Self { value, from, to } Self { from, to }
} }
} }
/// Keep track of diversions in an EBB. /// Keep track of diversions in an EBB.
pub struct RegDiversions { pub struct RegDiversions {
current: Vec<Diversion>, current: FxHashMap<Value, Diversion>,
} }
impl RegDiversions { impl RegDiversions {
/// Create a new empty diversion tracker. /// Create a new empty diversion tracker.
pub fn new() -> Self { pub fn new() -> Self {
Self { Self {
current: Vec::new(), current: FxHashMap::default(),
} }
} }
@@ -63,12 +62,12 @@ impl RegDiversions {
/// Get the current diversion of `value`, if any. /// Get the current diversion of `value`, if any.
pub fn diversion(&self, value: Value) -> Option<&Diversion> { pub fn diversion(&self, value: Value) -> Option<&Diversion> {
self.current.iter().find(|d| d.value == value) self.current.get(&value)
} }
/// Get all current diversions. /// Get all current diversions.
pub fn all(&self) -> &[Diversion] { pub fn iter(&self) -> Iter<'_, Value, Diversion> {
self.current.as_slice() self.current.iter()
} }
/// Get the current location for `value`. Fall back to the assignment map for non-diverted /// Get the current location for `value`. Fall back to the assignment map for non-diverted
@@ -95,15 +94,22 @@ impl RegDiversions {
/// The `from` location must match an existing `to` location, if any. /// The `from` location must match an existing `to` location, if any.
pub fn divert(&mut self, value: Value, from: ValueLoc, to: ValueLoc) { pub fn divert(&mut self, value: Value, from: ValueLoc, to: ValueLoc) {
debug_assert!(from.is_assigned() && to.is_assigned()); debug_assert!(from.is_assigned() && to.is_assigned());
if let Some(i) = self.current.iter().position(|d| d.value == value) { match self.current.entry(value) {
debug_assert_eq!(self.current[i].to, from, "Bad regmove chain for {}", value); Entry::Occupied(mut e) => {
if self.current[i].from != to { // TODO: non-lexical lifetimes should allow removal of the scope and early return.
self.current[i].to = to; {
} else { let d = e.get_mut();
self.current.swap_remove(i); debug_assert_eq!(d.to, from, "Bad regmove chain for {}", value);
if d.from != to {
d.to = to;
return;
}
}
e.remove();
}
Entry::Vacant(e) => {
e.insert(Diversion::new(from, to));
} }
} else {
self.current.push(Diversion::new(value, from, to));
} }
} }
@@ -154,10 +160,7 @@ impl RegDiversions {
/// ///
/// Returns the `to` location of the removed diversion. /// Returns the `to` location of the removed diversion.
pub fn remove(&mut self, value: Value) -> Option<ValueLoc> { pub fn remove(&mut self, value: Value) -> Option<ValueLoc> {
self.current self.current.remove(&value).map(|d| d.to)
.iter()
.position(|d| d.value == value)
.map(|i| self.current.swap_remove(i).to)
} }
/// Return an object that can display the diversions. /// Return an object that can display the diversions.
@@ -172,11 +175,11 @@ pub struct DisplayDiversions<'a>(&'a RegDiversions, Option<&'a RegInfo>);
impl<'a> fmt::Display for DisplayDiversions<'a> { impl<'a> fmt::Display for DisplayDiversions<'a> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{{")?; write!(f, "{{")?;
for div in self.0.all() { for (value, div) in self.0.iter() {
write!( write!(
f, f,
" {}: {} -> {}", " {}: {} -> {}",
div.value, value,
div.from.display(self.1), div.from.display(self.1),
div.to.display(self.1) div.to.display(self.1)
)? )?
@@ -201,7 +204,6 @@ mod tests {
assert_eq!( assert_eq!(
divs.diversion(v1), divs.diversion(v1),
Some(&Diversion { Some(&Diversion {
value: v1,
from: ValueLoc::Reg(10), from: ValueLoc::Reg(10),
to: ValueLoc::Reg(12), to: ValueLoc::Reg(12),
}) })

View File

@@ -318,14 +318,14 @@ impl<'a> LocationVerifier<'a> {
dfg.display_inst(inst, self.isa) dfg.display_inst(inst, self.isa)
), ),
SingleDest(ebb, _) => { SingleDest(ebb, _) => {
for d in divert.all() { for (&value, d) in divert.iter() {
let lr = &liveness[d.value]; let lr = &liveness[value];
if lr.is_livein(ebb, liveness.context(&self.func.layout)) { if lr.is_livein(ebb, liveness.context(&self.func.layout)) {
return fatal!( return fatal!(
errors, errors,
inst, inst,
"{} is diverted to {} and live in to {}", "{} is diverted to {} and live in to {}",
d.value, value,
d.to.display(&self.reginfo), d.to.display(&self.reginfo),
ebb ebb
); );
@@ -333,15 +333,15 @@ impl<'a> LocationVerifier<'a> {
} }
} }
Table(jt, ebb) => { Table(jt, ebb) => {
for d in divert.all() { for (&value, d) in divert.iter() {
let lr = &liveness[d.value]; let lr = &liveness[value];
if let Some(ebb) = ebb { if let Some(ebb) = ebb {
if lr.is_livein(ebb, liveness.context(&self.func.layout)) { if lr.is_livein(ebb, liveness.context(&self.func.layout)) {
return fatal!( return fatal!(
errors, errors,
inst, inst,
"{} is diverted to {} and live in to {}", "{} is diverted to {} and live in to {}",
d.value, value,
d.to.display(&self.reginfo), d.to.display(&self.reginfo),
ebb ebb
); );
@@ -353,7 +353,7 @@ impl<'a> LocationVerifier<'a> {
errors, errors,
inst, inst,
"{} is diverted to {} and live in to {}", "{} is diverted to {} and live in to {}",
d.value, value,
d.to.display(&self.reginfo), d.to.display(&self.reginfo),
ebb ebb
); );

View File

@@ -97,6 +97,7 @@ where
} }
/// Resize the map to have `n` entries by adding default entries as needed. /// Resize the map to have `n` entries by adding default entries as needed.
#[inline]
pub fn resize(&mut self, n: usize) { pub fn resize(&mut self, n: usize) {
self.elems.resize(n, self.default.clone()); self.elems.resize(n, self.default.clone());
} }
@@ -125,6 +126,7 @@ where
K: EntityRef, K: EntityRef,
V: Clone, V: Clone,
{ {
#[inline]
fn index_mut(&mut self, k: K) -> &mut V { fn index_mut(&mut self, k: K) -> &mut V {
let i = k.index(); let i = k.index();
if i >= self.elems.len() { if i >= self.elems.len() {