Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: use &'static [PReg] instead of Vec<PReg> #208

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,7 @@ impl<'a, F: Function> Checker<'a, F> {
bb_in.insert(f.entry_block(), CheckerState::default());

let mut stack_pregs = PRegSet::empty();
for &preg in &machine_env.fixed_stack_slots {
for &preg in machine_env.fixed_stack_slots {
stack_pregs.add(preg);
}

Expand Down
6 changes: 3 additions & 3 deletions src/fastalloc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,9 @@ impl<'a, F: Function> Env<'a, F> {
fn new(func: &'a F, env: &'a MachineEnv) -> Self {
use alloc::vec;
let mut regs = [
env.preferred_regs_by_class[RegClass::Int as usize].clone(),
env.preferred_regs_by_class[RegClass::Float as usize].clone(),
env.preferred_regs_by_class[RegClass::Vector as usize].clone(),
Vec::from(env.preferred_regs_by_class[RegClass::Int as usize]),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't love this, open for better ideas

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use .to_owned() instead of Vec::from?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, yeah doesn't really change anything about it but might look nicer

Vec::from(env.preferred_regs_by_class[RegClass::Float as usize]),
Vec::from(env.preferred_regs_by_class[RegClass::Vector as usize]),
];
regs[0].extend(
env.non_preferred_regs_by_class[RegClass::Int as usize]
Expand Down
16 changes: 9 additions & 7 deletions src/fastalloc/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,15 +143,17 @@ impl RealFunction {
fn mach_env(no_of_regs: usize) -> MachineEnv {
MachineEnv {
preferred_regs_by_class: [
(0..no_of_regs)
.map(|no| PReg::new(no, RegClass::Int))
.collect(),
vec![],
vec![],
Vec::leak(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We definitely can't leak while fuzzing -- can we instead have a few static MachineEnvs in scope, and pick one of them? We don't necessarily need to fuzz with arbitrary no_of_regs, just with a few values that are interesting -- the two extremes of its range and a few in the middle, for example.

(0..no_of_regs)
.map(|no| PReg::new(no, RegClass::Int))
.collect(),
),
&[],
&[],
],
non_preferred_regs_by_class: [vec![], vec![], vec![]],
non_preferred_regs_by_class: [&[], &[], &[]],
scratch_by_class: [None, None, None],
fixed_stack_slots: vec![],
fixed_stack_slots: &[],
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/ion/liveranges.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ impl<'a, F: Function> Env<'a, F> {
is_stack: false,
},
);
for &preg in &self.env.fixed_stack_slots {
for &preg in self.env.fixed_stack_slots {
self.pregs[preg.index()].is_stack = true;
}
for class in 0..self.preferred_victim_by_class.len() {
Expand Down
16 changes: 11 additions & 5 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,13 +338,13 @@ impl From<&MachineEnv> for PRegSet {
fn from(env: &MachineEnv) -> Self {
let mut res = Self::default();

for class in env.preferred_regs_by_class.iter() {
for class in env.preferred_regs_by_class {
for preg in class {
res.add(*preg)
}
}

for class in env.non_preferred_regs_by_class.iter() {
for class in env.non_preferred_regs_by_class {
for preg in class {
res.add(*preg)
}
Expand Down Expand Up @@ -1434,6 +1434,12 @@ impl<'a> Iterator for OutputIter<'a> {
/// are available to allocate and what register may be used as a
/// scratch register for each class, and some other miscellaneous info
/// as well.
///
/// Note that `MachineEnv` is designed to be a global configuration struct that programs
/// will have very few of and generally want to keep around for their entire lifetime.
/// In order to make static initialization easier the registers lists are static slices instead
/// of `Vec`s. If your use case depends on dynamically creating the registers lists, consider
/// `[Vec::leak`].
#[derive(Clone, Debug)]
#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))]
pub struct MachineEnv {
Expand All @@ -1442,7 +1448,7 @@ pub struct MachineEnv {
///
/// If an explicit scratch register is provided in `scratch_by_class` then
/// it must not appear in this list.
pub preferred_regs_by_class: [Vec<PReg>; 3],
pub preferred_regs_by_class: [&'static [PReg]; 3],

/// Non-preferred physical registers for each class. These are the
/// registers that will be allocated if a preferred register is
Expand All @@ -1451,7 +1457,7 @@ pub struct MachineEnv {
///
/// If an explicit scratch register is provided in `scratch_by_class` then
/// it must not appear in this list.
pub non_preferred_regs_by_class: [Vec<PReg>; 3],
pub non_preferred_regs_by_class: [&'static [PReg]; 3],

/// Optional dedicated scratch register per class. This is needed to perform
/// moves between registers when cyclic move patterns occur. The
Expand All @@ -1476,7 +1482,7 @@ pub struct MachineEnv {
///
/// `PReg`s in this list cannot be used as an allocatable or scratch
/// register.
pub fixed_stack_slots: Vec<PReg>,
pub fixed_stack_slots: &'static [PReg],
}

/// The output of the register allocator.
Expand Down
Loading