From e22760851020adc3305689967654137b86287947 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Mon, 1 Jun 2020 15:09:00 +0200 Subject: [PATCH] mach backend: use vectors instead of sets to remember set of uses/defs for calls; This avoids the set uniqueness (hashing) test, reduces memory churn when re-mapping virtual register onto real registers, and is generally more memory-efficient. --- Cargo.lock | 4 +- cranelift/codegen/Cargo.toml | 2 +- cranelift/codegen/src/isa/aarch64/abi.rs | 40 ++++++++-------- .../src/isa/aarch64/inst/emit_tests.rs | 8 ++-- cranelift/codegen/src/isa/aarch64/inst/mod.rs | 48 +++++++------------ cranelift/codegen/src/machinst/abi.rs | 3 ++ 6 files changed, 47 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index be772cc0e1..c28a825c54 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1640,9 +1640,9 @@ dependencies = [ [[package]] name = "regalloc" -version = "0.0.24" +version = "0.0.25" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5842bece8a4b1690ffa6d9d959081c1d5d851ee4337a36c0a121fafe8c16add2" +checksum = "cca5b48c9db66c5ba084e4660b4c0cfe8b551a96074bc04b7c11de86ad0bf1f9" dependencies = [ "log", "rustc-hash", diff --git a/cranelift/codegen/Cargo.toml b/cranelift/codegen/Cargo.toml index b86453c505..26e5ceb2e7 100644 --- a/cranelift/codegen/Cargo.toml +++ b/cranelift/codegen/Cargo.toml @@ -25,7 +25,7 @@ smallvec = { version = "1.0.0" } thiserror = "1.0.4" byteorder = { version = "1.3.2", default-features = false } peepmatic-runtime = { path = "../peepmatic/crates/runtime", optional = true } -regalloc = "0.0.24" +regalloc = "0.0.25" # It is a goal of the cranelift-codegen crate to have minimal external dependencies. # Please don't add any unless they are essential to the task of creating binary # machine code. Integration tests that need external dependencies can be diff --git a/cranelift/codegen/src/isa/aarch64/abi.rs b/cranelift/codegen/src/isa/aarch64/abi.rs index 23b824fdb7..1d7cb9e2f2 100644 --- a/cranelift/codegen/src/isa/aarch64/abi.rs +++ b/cranelift/codegen/src/isa/aarch64/abi.rs @@ -655,21 +655,21 @@ fn is_caller_save(call_conv: isa::CallConv, r: RealReg) -> bool { } } -fn get_caller_saves_set(call_conv: isa::CallConv) -> Set> { - let mut set = Set::empty(); +fn get_caller_saves(call_conv: isa::CallConv) -> Vec> { + let mut caller_saved = Vec::new(); for i in 0..29 { let x = writable_xreg(i); if is_caller_save(call_conv, x.to_reg().to_real_reg()) { - set.insert(x); + caller_saved.push(x); } } for i in 0..32 { let v = writable_vreg(i); if is_caller_save(call_conv, v.to_reg().to_real_reg()) { - set.insert(v); + caller_saved.push(v); } } - set + caller_saved } impl ABIBody for AArch64ABIBody { @@ -1111,28 +1111,28 @@ enum CallDest { /// AArch64 ABI object for a function call. pub struct AArch64ABICall { sig: ABISig, - uses: Set, - defs: Set>, + uses: Vec, + defs: Vec>, dest: CallDest, loc: ir::SourceLoc, opcode: ir::Opcode, } -fn abisig_to_uses_and_defs(sig: &ABISig) -> (Set, Set>) { +fn abisig_to_uses_and_defs(sig: &ABISig) -> (Vec, Vec>) { // Compute uses: all arg regs. - let mut uses = Set::empty(); + let mut uses = Vec::new(); for arg in &sig.args { match arg { - &ABIArg::Reg(reg, _) => uses.insert(reg.to_reg()), + &ABIArg::Reg(reg, _) => uses.push(reg.to_reg()), _ => {} } } // Compute defs: all retval regs, and all caller-save (clobbered) regs. - let mut defs = get_caller_saves_set(sig.call_conv); + let mut defs = get_caller_saves(sig.call_conv); for ret in &sig.rets { match ret { - &ABIArg::Reg(reg, _) => defs.insert(Writable::from_reg(reg.to_reg())), + &ABIArg::Reg(reg, _) => defs.push(Writable::from_reg(reg.to_reg())), _ => {} } } @@ -1271,14 +1271,14 @@ impl ABICall for AArch64ABICall { fn emit_call>(&mut self, ctx: &mut C) { let (uses, defs) = ( - mem::replace(&mut self.uses, Set::empty()), - mem::replace(&mut self.defs, Set::empty()), + mem::replace(&mut self.uses, Default::default()), + mem::replace(&mut self.defs, Default::default()), ); match &self.dest { &CallDest::ExtName(ref name, RelocDistance::Near) => ctx.emit(Inst::Call { dest: Box::new(name.clone()), - uses: Box::new(uses), - defs: Box::new(defs), + uses: uses.into_boxed_slice(), + defs: defs.into_boxed_slice(), loc: self.loc, opcode: self.opcode, }), @@ -1291,16 +1291,16 @@ impl ABICall for AArch64ABICall { }); ctx.emit(Inst::CallInd { rn: spilltmp_reg(), - uses: Box::new(uses), - defs: Box::new(defs), + uses: uses.into_boxed_slice(), + defs: defs.into_boxed_slice(), loc: self.loc, opcode: self.opcode, }); } &CallDest::Reg(reg) => ctx.emit(Inst::CallInd { rn: reg, - uses: Box::new(uses), - defs: Box::new(defs), + uses: uses.into_boxed_slice(), + defs: defs.into_boxed_slice(), loc: self.loc, opcode: self.opcode, }), diff --git a/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs b/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs index 1dd6be20eb..b1ef3a4cfa 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs @@ -2114,8 +2114,8 @@ fn test_aarch64_binemit() { insns.push(( Inst::Call { dest: Box::new(ExternalName::testcase("test0")), - uses: Box::new(Set::empty()), - defs: Box::new(Set::empty()), + uses: Vec::new().into_boxed_slice(), + defs: Vec::new().into_boxed_slice(), loc: SourceLoc::default(), opcode: Opcode::Call, }, @@ -2126,8 +2126,8 @@ fn test_aarch64_binemit() { insns.push(( Inst::CallInd { rn: xreg(10), - uses: Box::new(Set::empty()), - defs: Box::new(Set::empty()), + uses: Vec::new().into_boxed_slice(), + defs: Vec::new().into_boxed_slice(), loc: SourceLoc::default(), opcode: Opcode::CallIndirect, }, diff --git a/cranelift/codegen/src/isa/aarch64/inst/mod.rs b/cranelift/codegen/src/isa/aarch64/inst/mod.rs index 1cf307d1d0..85d646d49e 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/mod.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/mod.rs @@ -12,7 +12,7 @@ use crate::machinst::*; use crate::{settings, CodegenError, CodegenResult}; use regalloc::{RealRegUniverse, Reg, RegClass, SpillSlot, VirtualReg, Writable}; -use regalloc::{RegUsageCollector, RegUsageMapper, Set}; +use regalloc::{RegUsageCollector, RegUsageMapper}; use alloc::boxed::Box; use alloc::vec::Vec; @@ -650,16 +650,16 @@ pub enum Inst { /// target. Call { dest: Box, - uses: Box>, - defs: Box>>, + uses: Box<[Reg]>, + defs: Box<[Writable]>, loc: SourceLoc, opcode: Opcode, }, /// A machine indirect-call instruction. CallInd { rn: Reg, - uses: Box>, - defs: Box>>, + uses: Box<[Reg]>, + defs: Box<[Writable]>, loc: SourceLoc, opcode: Opcode, }, @@ -1729,19 +1729,12 @@ fn aarch64_map_regs(inst: &mut Inst, mapper: &RUM) { ref mut defs, .. } => { - // TODO: add `map_mut()` to regalloc.rs's Set. - let new_uses = uses.map(|r| { - let mut r = *r; - map_use(mapper, &mut r); - r - }); - let new_defs = defs.map(|r| { - let mut r = *r; - map_def(mapper, &mut r); - r - }); - *uses = Box::new(new_uses); - *defs = Box::new(new_defs); + for r in uses.iter_mut() { + map_use(mapper, r); + } + for r in defs.iter_mut() { + map_def(mapper, r); + } } &mut Inst::Ret | &mut Inst::EpiloguePlaceholder => {} &mut Inst::CallInd { @@ -1750,19 +1743,12 @@ fn aarch64_map_regs(inst: &mut Inst, mapper: &RUM) { ref mut rn, .. } => { - // TODO: add `map_mut()` to regalloc.rs's Set. - let new_uses = uses.map(|r| { - let mut r = *r; - map_use(mapper, &mut r); - r - }); - let new_defs = defs.map(|r| { - let mut r = *r; - map_def(mapper, &mut r); - r - }); - *uses = Box::new(new_uses); - *defs = Box::new(new_defs); + for r in uses.iter_mut() { + map_use(mapper, r); + } + for r in defs.iter_mut() { + map_def(mapper, r); + } map_use(mapper, rn); } &mut Inst::CondBr { ref mut kind, .. } | &mut Inst::OneWayCondBr { ref mut kind, .. } => { diff --git a/cranelift/codegen/src/machinst/abi.rs b/cranelift/codegen/src/machinst/abi.rs index 0ccfad2e36..e8fbf25db1 100644 --- a/cranelift/codegen/src/machinst/abi.rs +++ b/cranelift/codegen/src/machinst/abi.rs @@ -167,5 +167,8 @@ pub trait ABICall { /// registers are also logically defs, but should never be read; their /// values are "defined" (to the regalloc) but "undefined" in every other /// sense.) + /// + /// This function should only be called once, as it is allowed to re-use + /// parts of the ABICall object in emitting instructions. fn emit_call>(&mut self, ctx: &mut C); }