From eab337c71a3955339105dc9c01aeae9b118c516f Mon Sep 17 00:00:00 2001 From: Jonas Kruckenberg Date: Fri, 26 Apr 2024 11:50:43 +0200 Subject: [PATCH 1/2] refactor(cranelift-codegen): remove `SpinLock` This removes the dependency on `std::sync::SpinLock` by lifting the state out of a static and into the `Callee` struct. --- cranelift/codegen/src/isa/aarch64/abi.rs | 11 ++--------- cranelift/codegen/src/isa/riscv64/abi.rs | 6 ++---- cranelift/codegen/src/isa/s390x/abi.rs | 6 ++---- cranelift/codegen/src/isa/x64/abi.rs | 11 ++--------- cranelift/codegen/src/machinst/abi.rs | 8 ++++++-- 5 files changed, 14 insertions(+), 28 deletions(-) diff --git a/cranelift/codegen/src/isa/aarch64/abi.rs b/cranelift/codegen/src/isa/aarch64/abi.rs index 1c3ffc37bc14..e83bb23e32ec 100644 --- a/cranelift/codegen/src/isa/aarch64/abi.rs +++ b/cranelift/codegen/src/isa/aarch64/abi.rs @@ -16,7 +16,6 @@ use alloc::boxed::Box; use alloc::vec::Vec; use regalloc2::{MachineEnv, PReg, PRegSet}; use smallvec::{smallvec, SmallVec}; -use std::sync::OnceLock; // We use a generic implementation that factors out AArch64 and x64 ABI commonalities, because // these ABIs are very similar. @@ -1154,14 +1153,8 @@ impl ABIMachineSpec for AArch64MachineDeps { s.nominal_sp_to_fp } - fn get_machine_env(flags: &settings::Flags, _call_conv: isa::CallConv) -> &MachineEnv { - if flags.enable_pinned_reg() { - static MACHINE_ENV: OnceLock = OnceLock::new(); - MACHINE_ENV.get_or_init(|| create_reg_env(true)) - } else { - static MACHINE_ENV: OnceLock = OnceLock::new(); - MACHINE_ENV.get_or_init(|| create_reg_env(false)) - } + fn get_machine_env(flags: &settings::Flags) -> MachineEnv { + create_reg_env(flags.enable_pinned_reg()) } fn get_regs_clobbered_by_call(_call_conv: isa::CallConv) -> PRegSet { diff --git a/cranelift/codegen/src/isa/riscv64/abi.rs b/cranelift/codegen/src/isa/riscv64/abi.rs index b72768b2bddc..00aab2cce4fd 100644 --- a/cranelift/codegen/src/isa/riscv64/abi.rs +++ b/cranelift/codegen/src/isa/riscv64/abi.rs @@ -21,7 +21,6 @@ use alloc::vec::Vec; use regalloc2::{MachineEnv, PReg, PRegSet}; use smallvec::{smallvec, SmallVec}; -use std::sync::OnceLock; /// Support for the Riscv64 ABI from the callee side (within a function body). pub(crate) type Riscv64Callee = Callee; @@ -704,9 +703,8 @@ impl ABIMachineSpec for Riscv64MachineDeps { s.nominal_sp_to_fp } - fn get_machine_env(_flags: &settings::Flags, _call_conv: isa::CallConv) -> &MachineEnv { - static MACHINE_ENV: OnceLock = OnceLock::new(); - MACHINE_ENV.get_or_init(create_reg_enviroment) + fn get_machine_env(_flags: &settings::Flags) -> MachineEnv { + create_reg_enviroment() } fn get_regs_clobbered_by_call(_call_conv_of_callee: isa::CallConv) -> PRegSet { diff --git a/cranelift/codegen/src/isa/s390x/abi.rs b/cranelift/codegen/src/isa/s390x/abi.rs index 321ec1e5d50c..728e0508c425 100644 --- a/cranelift/codegen/src/isa/s390x/abi.rs +++ b/cranelift/codegen/src/isa/s390x/abi.rs @@ -81,7 +81,6 @@ use crate::{CodegenError, CodegenResult}; use alloc::vec::Vec; use regalloc2::{MachineEnv, PRegSet}; use smallvec::{smallvec, SmallVec}; -use std::sync::OnceLock; // We use a generic implementation that factors out ABI commonalities. @@ -786,9 +785,8 @@ impl ABIMachineSpec for S390xMachineDeps { s.initial_sp_offset } - fn get_machine_env(_flags: &settings::Flags, _call_conv: isa::CallConv) -> &MachineEnv { - static MACHINE_ENV: OnceLock = OnceLock::new(); - MACHINE_ENV.get_or_init(create_machine_env) + fn get_machine_env(_flags: &settings::Flags) -> MachineEnv { + create_machine_env() } fn get_regs_clobbered_by_call(_call_conv_of_callee: isa::CallConv) -> PRegSet { diff --git a/cranelift/codegen/src/isa/x64/abi.rs b/cranelift/codegen/src/isa/x64/abi.rs index c23d75509acc..1a6b831e66ea 100644 --- a/cranelift/codegen/src/isa/x64/abi.rs +++ b/cranelift/codegen/src/isa/x64/abi.rs @@ -13,7 +13,6 @@ use alloc::vec::Vec; use args::*; use regalloc2::{MachineEnv, PReg, PRegSet}; use smallvec::{smallvec, SmallVec}; -use std::sync::OnceLock; /// This is the limit for the size of argument and return-value areas on the /// stack. We place a reasonable limit here to avoid integer overflow issues @@ -936,14 +935,8 @@ impl ABIMachineSpec for X64ABIMachineSpec { s.nominal_sp_to_fp() } - fn get_machine_env(flags: &settings::Flags, _call_conv: isa::CallConv) -> &MachineEnv { - if flags.enable_pinned_reg() { - static MACHINE_ENV: OnceLock = OnceLock::new(); - MACHINE_ENV.get_or_init(|| create_reg_env_systemv(true)) - } else { - static MACHINE_ENV: OnceLock = OnceLock::new(); - MACHINE_ENV.get_or_init(|| create_reg_env_systemv(false)) - } + fn get_machine_env(flags: &settings::Flags) -> MachineEnv { + create_reg_env_systemv(flags.enable_pinned_reg()) } fn get_regs_clobbered_by_call(call_conv_of_callee: isa::CallConv) -> PRegSet { diff --git a/cranelift/codegen/src/machinst/abi.rs b/cranelift/codegen/src/machinst/abi.rs index a5de3824dba7..ed1673b23164 100644 --- a/cranelift/codegen/src/machinst/abi.rs +++ b/cranelift/codegen/src/machinst/abi.rs @@ -626,7 +626,7 @@ pub trait ABIMachineSpec { fn get_nominal_sp_to_fp(s: &::State) -> i64; /// Get the ABI-dependent MachineEnv for managing register allocation. - fn get_machine_env(flags: &settings::Flags, call_conv: isa::CallConv) -> &MachineEnv; + fn get_machine_env(flags: &settings::Flags) -> MachineEnv; /// Get all caller-save registers, that is, registers that we expect /// not to be saved across a call to a callee with the given ABI. @@ -1118,6 +1118,7 @@ pub struct Callee { /// manually register-allocated and carefully only use caller-saved /// registers and keep nothing live after this sequence of instructions. stack_limit: Option<(Reg, SmallInstVec)>, + machine_env: MachineEnv, _mach: PhantomData, } @@ -1227,6 +1228,8 @@ impl Callee { let tail_args_size = sigs[sig].sized_stack_arg_space; + let machine_env = M::get_machine_env(&flags); + Ok(Self { ir_sig: ensure_struct_return_ptr_is_returned(&f.signature), sig, @@ -1244,6 +1247,7 @@ impl Callee { isa_flags: isa_flags.clone(), is_leaf: f.is_leaf(), stack_limit, + machine_env, _mach: PhantomData, }) } @@ -1450,7 +1454,7 @@ impl Callee { /// Get the ABI-dependent MachineEnv for managing register allocation. pub fn machine_env(&self, sigs: &SigSet) -> &MachineEnv { - M::get_machine_env(&self.flags, self.call_conv(sigs)) + &self.machine_env } /// The offsets of all sized stack slots (not spill slots) for debuginfo purposes. From 68392776238cf95756277a8167c90d84843ea0eb Mon Sep 17 00:00:00 2001 From: Jonas Kruckenberg Date: Fri, 26 Apr 2024 12:02:02 +0200 Subject: [PATCH 2/2] cleanup --- cranelift/codegen/src/machinst/abi.rs | 2 +- cranelift/codegen/src/machinst/vcode.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cranelift/codegen/src/machinst/abi.rs b/cranelift/codegen/src/machinst/abi.rs index ed1673b23164..2fce9928d50b 100644 --- a/cranelift/codegen/src/machinst/abi.rs +++ b/cranelift/codegen/src/machinst/abi.rs @@ -1453,7 +1453,7 @@ impl Callee { } /// Get the ABI-dependent MachineEnv for managing register allocation. - pub fn machine_env(&self, sigs: &SigSet) -> &MachineEnv { + pub fn machine_env(&self) -> &MachineEnv { &self.machine_env } diff --git a/cranelift/codegen/src/machinst/vcode.rs b/cranelift/codegen/src/machinst/vcode.rs index 53cc6e7eaf8c..651a52c7ae0f 100644 --- a/cranelift/codegen/src/machinst/vcode.rs +++ b/cranelift/codegen/src/machinst/vcode.rs @@ -637,7 +637,7 @@ impl VCode { /// Get the ABI-dependent MachineEnv for managing register allocation. pub fn machine_env(&self) -> &MachineEnv { - self.abi.machine_env(&self.sigs) + self.abi.machine_env() } /// Get the number of blocks. Block indices will be in the range `0 ..