Skip to content

Commit 906a053

Browse files
AmanieuAmanieu
authored
Remove register class from SpillSlot (#80)
* Remove register class from `SpillSlot` The register allocator was already allowing moves between spillslots and registers of different classes, so this PR formalizes this by making spillslots independent of register class. This also fixes #79 by properly tracking the register class of an `InsertedMove` with the `to_vreg` field which turns out to never be `None` in practice. Removing the `Option` allows the register class of the `VReg` to be used when building the per-class move lists. Fixes #79 * Address review feedback
1 parent 3db1b71 commit 906a053

6 files changed

Lines changed: 40 additions & 74 deletions

File tree

‎fuzz/fuzz_targets/moves.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,14 @@ impl Arbitrary<'_> for TestCase {
4040
Allocation::reg(PReg::new(reg, RegClass::Int))
4141
} else {
4242
let slot = u.int_in_range(0..=31)?;
43-
Allocation::stack(SpillSlot::new(slot, RegClass::Int))
43+
Allocation::stack(SpillSlot::new(slot))
4444
};
4545
let dst = if bool::arbitrary(u)? {
4646
let reg = u.int_in_range(0..=29)?;
4747
Allocation::reg(PReg::new(reg, RegClass::Int))
4848
} else {
4949
let slot = u.int_in_range(0..=31)?;
50-
Allocation::stack(SpillSlot::new(slot, RegClass::Int))
50+
Allocation::stack(SpillSlot::new(slot))
5151
};
5252

5353
// Stop if we are going to write a reg more than once:
@@ -88,7 +88,7 @@ fuzz_target!(|testcase: TestCase| {
8888
let get_stackslot = || {
8989
let slot = next_slot;
9090
next_slot += 1;
91-
Allocation::stack(SpillSlot::new(slot, RegClass::Int))
91+
Allocation::stack(SpillSlot::new(slot))
9292
};
9393
let preferred_victim = PReg::new(0, RegClass::Int);
9494
let scratch_resolver =

‎src/ion/data_structures.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ impl<'a, F: Function> Env<'a, F> {
440440
#[derive(Clone, Debug)]
441441
pub struct SpillSlotData {
442442
pub ranges: LiveRangeSet,
443-
pub class: RegClass,
443+
pub slots: u32,
444444
pub alloc: Allocation,
445445
}
446446

@@ -580,7 +580,7 @@ pub struct InsertedMove {
580580
pub pos_prio: PosWithPrio,
581581
pub from_alloc: Allocation,
582582
pub to_alloc: Allocation,
583-
pub to_vreg: Option<VReg>,
583+
pub to_vreg: VReg,
584584
}
585585

586586
#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)]

‎src/ion/liveranges.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -573,7 +573,7 @@ impl<'a, F: Function> Env<'a, F> {
573573
InsertMovePrio::MultiFixedRegInitial,
574574
Allocation::reg(src_preg),
575575
Allocation::reg(dst_preg),
576-
Some(dst.vreg()),
576+
dst.vreg(),
577577
);
578578
}
579579

@@ -718,14 +718,14 @@ impl<'a, F: Function> Env<'a, F> {
718718
InsertMovePrio::Regular,
719719
Allocation::reg(preg),
720720
Allocation::reg(preg),
721-
Some(dst.vreg()),
721+
dst.vreg(),
722722
);
723723
self.insert_move(
724724
ProgPoint::before(inst.next()),
725725
InsertMovePrio::MultiFixedRegInitial,
726726
Allocation::reg(preg),
727727
Allocation::reg(preg),
728-
Some(src.vreg()),
728+
src.vreg(),
729729
);
730730
} else {
731731
if inst > self.cfginfo.block_entry[block.index()].inst() {
@@ -751,7 +751,7 @@ impl<'a, F: Function> Env<'a, F> {
751751
InsertMovePrio::BlockParam,
752752
Allocation::reg(preg),
753753
Allocation::reg(preg),
754-
Some(dst.vreg()),
754+
dst.vreg(),
755755
);
756756
}
757757
} else {
@@ -781,7 +781,7 @@ impl<'a, F: Function> Env<'a, F> {
781781
InsertMovePrio::PostRegular,
782782
Allocation::reg(preg),
783783
Allocation::reg(preg),
784-
Some(dst.vreg()),
784+
dst.vreg(),
785785
);
786786
}
787787
// Otherwise, if dead, no need to create

‎src/ion/moves.rs‎

Lines changed: 21 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -45,20 +45,21 @@ impl<'a, F: Function> Env<'a, F> {
4545
prio: InsertMovePrio,
4646
from_alloc: Allocation,
4747
to_alloc: Allocation,
48-
to_vreg: Option<VReg>,
48+
to_vreg: VReg,
4949
) {
5050
trace!(
51-
"insert_move: pos {:?} prio {:?} from_alloc {:?} to_alloc {:?}",
51+
"insert_move: pos {:?} prio {:?} from_alloc {:?} to_alloc {:?} to_vreg {:?}",
5252
pos,
5353
prio,
5454
from_alloc,
55-
to_alloc
55+
to_alloc,
56+
to_vreg
5657
);
57-
match (from_alloc.as_reg(), to_alloc.as_reg()) {
58-
(Some(from), Some(to)) => {
59-
debug_assert_eq!(from.class(), to.class());
60-
}
61-
_ => {}
58+
if let Some(from) = from_alloc.as_reg() {
59+
debug_assert_eq!(from.class(), to_vreg.class());
60+
}
61+
if let Some(to) = to_alloc.as_reg() {
62+
debug_assert_eq!(to.class(), to_vreg.class());
6263
}
6364
self.inserted_moves.push(InsertedMove {
6465
pos_prio: PosWithPrio {
@@ -280,7 +281,7 @@ impl<'a, F: Function> Env<'a, F> {
280281
InsertMovePrio::Regular,
281282
prev_alloc,
282283
alloc,
283-
Some(self.vreg(vreg)),
284+
self.vreg(vreg),
284285
);
285286
}
286287
}
@@ -720,7 +721,7 @@ impl<'a, F: Function> Env<'a, F> {
720721
prio,
721722
src.alloc,
722723
dest.alloc,
723-
Some(self.vreg(dest.to_vreg())),
724+
self.vreg(dest.to_vreg()),
724725
);
725726
last = Some(dest.alloc);
726727
}
@@ -741,13 +742,7 @@ impl<'a, F: Function> Env<'a, F> {
741742
FixedRegFixupLevel::Initial => InsertMovePrio::MultiFixedRegInitial,
742743
FixedRegFixupLevel::Secondary => InsertMovePrio::MultiFixedRegSecondary,
743744
};
744-
self.insert_move(
745-
fixup.pos,
746-
prio,
747-
from_alloc,
748-
to_alloc,
749-
Some(self.vreg(fixup.vreg)),
750-
);
745+
self.insert_move(fixup.pos, prio, from_alloc, to_alloc, self.vreg(fixup.vreg));
751746
self.set_alloc(
752747
fixup.pos.inst(),
753748
fixup.to_slot as usize,
@@ -831,7 +826,7 @@ impl<'a, F: Function> Env<'a, F> {
831826
InsertMovePrio::ReusedInput,
832827
input_alloc,
833828
output_alloc,
834-
Some(input_operand.vreg()),
829+
input_operand.vreg(),
835830
);
836831
self.set_alloc(inst, input_idx, output_alloc);
837832
}
@@ -871,7 +866,7 @@ impl<'a, F: Function> Env<'a, F> {
871866
InsertMovePrio::Regular,
872867
from_alloc,
873868
to_alloc,
874-
Some(self.vreg(to_vreg)),
869+
self.vreg(to_vreg),
875870
);
876871
}
877872

@@ -961,13 +956,10 @@ impl<'a, F: Function> Env<'a, F> {
961956
let mut float_moves: SmallVec<[InsertedMove; 8]> = smallvec![];
962957

963958
for m in moves {
964-
if m.from_alloc.is_reg() && m.to_alloc.is_reg() {
965-
debug_assert_eq!(m.from_alloc.class(), m.to_alloc.class());
966-
}
967959
if m.from_alloc == m.to_alloc {
968960
continue;
969961
}
970-
match m.from_alloc.class() {
962+
match m.to_vreg.class() {
971963
RegClass::Int => {
972964
int_moves.push(m.clone());
973965
}
@@ -990,10 +982,8 @@ impl<'a, F: Function> Env<'a, F> {
990982
pos_prio.prio
991983
);
992984
for m in moves {
993-
if (m.from_alloc != m.to_alloc) || m.to_vreg.is_some() {
994-
trace!(" {} -> {}", m.from_alloc, m.to_alloc,);
995-
parallel_moves.add(m.from_alloc, m.to_alloc, m.to_vreg);
996-
}
985+
trace!(" {} -> {}", m.from_alloc, m.to_alloc);
986+
parallel_moves.add(m.from_alloc, m.to_alloc, Some(m.to_vreg));
997987
}
998988

999989
let resolved = parallel_moves.resolve();
@@ -1042,7 +1032,7 @@ impl<'a, F: Function> Env<'a, F> {
10421032
// these placeholders then allocate the actual
10431033
// slots if needed with `self.allocate_spillslot`
10441034
// below.
1045-
Allocation::stack(SpillSlot::new(SpillSlot::MAX - idx, regclass))
1035+
Allocation::stack(SpillSlot::new(SpillSlot::MAX - idx))
10461036
};
10471037
let is_stack_alloc = |alloc: Allocation| {
10481038
if let Some(preg) = alloc.as_reg() {
@@ -1065,11 +1055,12 @@ impl<'a, F: Function> Env<'a, F> {
10651055
let mut rewrites = FxHashMap::default();
10661056
for i in 0..stackslot_idx {
10671057
if i >= self.extra_spillslots_by_class[regclass as usize].len() {
1068-
let slot = self.allocate_spillslot(regclass);
1058+
let slot =
1059+
self.allocate_spillslot(self.func.spillslot_size(regclass) as u32);
10691060
self.extra_spillslots_by_class[regclass as usize].push(slot);
10701061
}
10711062
rewrites.insert(
1072-
Allocation::stack(SpillSlot::new(SpillSlot::MAX - i, regclass)),
1063+
Allocation::stack(SpillSlot::new(SpillSlot::MAX - i)),
10731064
self.extra_spillslots_by_class[regclass as usize][i],
10741065
);
10751066
}

‎src/ion/spill.rs‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
//! Spillslot allocation.
1414
1515
use super::{
16-
AllocRegResult, Env, LiveRangeKey, LiveRangeSet, PReg, PRegIndex, RegClass, RegTraversalIter,
16+
AllocRegResult, Env, LiveRangeKey, LiveRangeSet, PReg, PRegIndex, RegTraversalIter,
1717
SpillSetIndex, SpillSlotData, SpillSlotIndex, SpillSlotList,
1818
};
1919
use crate::{Allocation, Function, SpillSlot};
@@ -165,7 +165,7 @@ impl<'a, F: Function> Env<'a, F> {
165165
self.spillslots.push(SpillSlotData {
166166
ranges: LiveRangeSet::new(),
167167
alloc: Allocation::none(),
168-
class: self.spillsets[spillset.index()].class,
168+
slots: size as u32,
169169
});
170170
self.slots_by_size[size].slots.push(spillslot);
171171
self.slots_by_size[size].probe_start = self.slots_by_size[size].slots.len() - 1;
@@ -176,14 +176,13 @@ impl<'a, F: Function> Env<'a, F> {
176176

177177
// Assign actual slot indices to spillslots.
178178
for i in 0..self.spillslots.len() {
179-
self.spillslots[i].alloc = self.allocate_spillslot(self.spillslots[i].class);
179+
self.spillslots[i].alloc = self.allocate_spillslot(self.spillslots[i].slots);
180180
}
181181

182182
trace!("spillslot allocator done");
183183
}
184184

185-
pub fn allocate_spillslot(&mut self, class: RegClass) -> Allocation {
186-
let size = self.func.spillslot_size(class) as u32;
185+
pub fn allocate_spillslot(&mut self, size: u32) -> Allocation {
187186
let mut offset = self.num_spillslots;
188187
// Align up to `size`.
189188
debug_assert!(size.is_power_of_two());
@@ -195,6 +194,6 @@ impl<'a, F: Function> Env<'a, F> {
195194
};
196195
offset += size;
197196
self.num_spillslots = offset;
198-
Allocation::stack(SpillSlot::new(slot as usize, class))
197+
Allocation::stack(SpillSlot::new(slot as usize))
199198
}
200199
}

‎src/lib.rs‎

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -324,13 +324,11 @@ impl SpillSlot {
324324
/// The maximum spillslot index.
325325
pub const MAX: usize = (1 << 24) - 1;
326326

327-
/// Create a new SpillSlot of a given class.
327+
/// Create a new SpillSlot.
328328
#[inline(always)]
329-
pub fn new(slot: usize, class: RegClass) -> Self {
329+
pub fn new(slot: usize) -> Self {
330330
debug_assert!(slot <= Self::MAX);
331-
SpillSlot {
332-
bits: (slot as u32) | (class as u8 as u32) << 24,
333-
}
331+
SpillSlot { bits: slot as u32 }
334332
}
335333

336334
/// Get the spillslot index for this spillslot.
@@ -339,20 +337,10 @@ impl SpillSlot {
339337
(self.bits & 0x00ffffff) as usize
340338
}
341339

342-
/// Get the class for this spillslot.
343-
#[inline(always)]
344-
pub fn class(self) -> RegClass {
345-
match (self.bits >> 24) as u8 {
346-
0 => RegClass::Int,
347-
1 => RegClass::Float,
348-
_ => unreachable!(),
349-
}
350-
}
351-
352340
/// Get the spillslot `offset` slots away.
353341
#[inline(always)]
354342
pub fn plus(self, offset: usize) -> Self {
355-
SpillSlot::new(self.index() + offset, self.class())
343+
SpillSlot::new(self.index() + offset)
356344
}
357345

358346
/// Get the invalid spillslot, used for initializing data structures.
@@ -965,18 +953,6 @@ pub enum AllocationKind {
965953
Stack = 2,
966954
}
967955

968-
impl Allocation {
969-
/// Get the register class of an allocation's value.
970-
#[inline(always)]
971-
pub fn class(self) -> RegClass {
972-
match self.kind() {
973-
AllocationKind::None => panic!("Allocation::None has no class"),
974-
AllocationKind::Reg => self.as_reg().unwrap().class(),
975-
AllocationKind::Stack => self.as_stack().unwrap().class(),
976-
}
977-
}
978-
}
979-
980956
/// A trait defined by the regalloc client to provide access to its
981957
/// machine-instruction / CFG representation.
982958
///

0 commit comments

Comments
 (0)