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.

Sign up for GitHub

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

Jump to bottom

Spill for VReg lifetimes instead of SpillSet lifetimes #171

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Spill for VReg lifetimes instead of SpillSet lifetimes #171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
39e9208
Allocate spill slots to vregs, not spillsets
elliottt Nov 30, 2023
1869c4d
Only write spill slots at defs
elliottt Dec 1, 2023
40409f1
Simplify stack map calculation now that VRegs track spill allocations
elliottt Dec 4, 2023
46ca116
Remove the redundant move eliminator
elliottt Dec 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 10 additions & 8 deletions src/ion/data_structures.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,18 +295,11 @@ const fn no_bloat_capacity<T>() -> usize {

#[derive(Clone, Debug)]
pub struct SpillSet {
pub slot: SpillSlotIndex,
pub reg_hint: PReg,
pub class: RegClass,
pub spill_bundle: LiveBundleIndex,
pub required: bool,
pub splits: u8,

/// The aggregate [`CodeRange`] of all involved [`LiveRange`]s. The effect of this abstraction
/// is that we attempt to allocate one spill slot for the extent of a bundle. For fragmented
/// bundles with lots of open space this abstraction is pessimistic, but when bundles are small
/// or dense this yields similar results to tracking individual live ranges.
pub range: CodeRange,
}

pub(crate) const MAX_SPLITS_PER_SPILLSET: u8 = 2;
Expand All @@ -316,8 +309,17 @@ pub struct VRegData {
pub ranges: LiveRangeList,
pub blockparam: Block,
pub is_ref: bool,

// We don't initially know the RegClass until we observe a use of the VReg.
pub class: Option<RegClass>,

/// The spill slot index is invalid when this VReg has never spilled. Otherwise, it's the
/// authoritative location for the spilled value.
pub slot: SpillSlotIndex,

/// This is the aggregate range for all liveranges that use this VReg. Its value is only valid
/// after liveranges have been built.
pub range: Option<CodeRange>,
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -528,7 +530,7 @@ impl<'a, F: Function> Env<'a, F> {

#[derive(Clone, Debug)]
pub struct SpillSetRanges {
pub btree: BTreeMap<LiveRangeKey, SpillSetIndex>,
pub btree: BTreeMap<LiveRangeKey, VRegIndex>,
}

impl SpillSetRanges {
Expand Down
11 changes: 10 additions & 1 deletion src/ion/liveranges.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use super::{
};
use crate::indexset::IndexSet;
use crate::ion::data_structures::{
BlockparamIn, BlockparamOut, FixedRegFixupLevel, MultiFixedRegFixup,
BlockparamIn, BlockparamOut, FixedRegFixupLevel, MultiFixedRegFixup, SpillSlotIndex,
};
use crate::{
Allocation, Block, Function, FxHashMap, FxHashSet, Inst, InstPosition, Operand,
Expand Down Expand Up @@ -130,6 +130,8 @@ impl<'a, F: Function> Env<'a, F> {
is_ref: false,
// We'll learn the RegClass as we scan the code.
class: None,
slot: SpillSlotIndex::invalid(),
range: None,
},
);
}
Expand Down Expand Up @@ -768,6 +770,13 @@ impl<'a, F: Function> Env<'a, F> {
debug_assert!(last.is_none() || last.unwrap() <= entry.range.from);
last = Some(entry.range.to);
}

// Initialize the code range for this vreg, when there were liveranges associated with
// it.
if let Some(to) = last {
let from = vreg.ranges[0].range.from;
vreg.range = Some(CodeRange { from, to });
}
}

for range in &mut self.ranges {
Expand Down
11 changes: 1 addition & 10 deletions src/ion/merge.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

//! Bundle merging.

use super::{Env, LiveBundleIndex, SpillSet, SpillSlotIndex, VRegIndex};
use super::{Env, LiveBundleIndex, SpillSet, VRegIndex};
use crate::{
ion::data_structures::{BlockparamOut, CodeRange},
Function, Inst, OperandConstraint, OperandKind, PReg, ProgPoint,
Expand Down Expand Up @@ -217,13 +217,6 @@ impl<'a, F: Function> Env<'a, F> {
}
}

if self.bundles[from].spillset != self.bundles[to].spillset {
// Widen the range for the target spillset to include the one being merged in.
let from_range = self.spillsets[self.bundles[from].spillset].range;
let to_range = &mut self.spillsets[self.bundles[to].spillset].range;
*to_range = to_range.join(from_range);
}

if self.bundles[from].cached_stack() {
self.bundles[to].set_cached_stack();
}
Expand Down Expand Up @@ -293,13 +286,11 @@ impl<'a, F: Function> Env<'a, F> {
// Create a spillslot for this bundle.
let reg = self.vreg(vreg);
let ssidx = self.spillsets.push(SpillSet {
slot: SpillSlotIndex::invalid(),
required: false,
class: reg.class(),
reg_hint: PReg::invalid(),
spill_bundle: LiveBundleIndex::invalid(),
splits: 0,
range,
});
self.bundles[bundle].spillset = ssidx;
}
Expand Down
2 changes: 0 additions & 2 deletions src/ion/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ pub(crate) mod reg_traversal;
use reg_traversal::*;
pub(crate) mod requirement;
use requirement::*;
pub(crate) mod redundant_moves;
use redundant_moves::*;
pub(crate) mod liveranges;
use liveranges::*;
pub(crate) mod merge;
Expand Down
150 changes: 65 additions & 85 deletions src/ion/moves.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
//! Move resolution.

use super::{
Env, InsertMovePrio, InsertedMove, InsertedMoves, LiveRangeFlag, LiveRangeIndex,
RedundantMoveEliminator, VRegIndex, SLOT_NONE,
Env, InsertMovePrio, InsertedMove, InsertedMoves, LiveRangeFlag, LiveRangeIndex, VRegIndex,
SLOT_NONE,
};
use crate::ion::data_structures::{
u64_key, BlockparamIn, BlockparamOut, CodeRange, Edits, FixedRegFixupLevel, LiveRangeKey,
Expand Down Expand Up @@ -51,21 +51,25 @@ impl<'a, F: Function> Env<'a, F> {
inst_allocs[slot] = alloc;
}

pub fn get_alloc_for_range(&self, range: LiveRangeIndex) -> Allocation {
pub fn get_spill_alloc_for(&self, vreg: VRegIndex) -> Allocation {
let slot = self.vregs[vreg].slot;
if slot.is_valid() {
self.spillslots[slot.index()].alloc
} else {
Allocation::none()
}
}

pub fn get_alloc_for_range(&self, range: LiveRangeIndex) -> Option<Allocation> {
trace!("get_alloc_for_range: {:?}", range);
let bundle = self.ranges[range].bundle;
trace!(" -> bundle: {:?}", bundle);
let bundledata = &self.bundles[bundle];
trace!(" -> allocation {:?}", bundledata.allocation);
if bundledata.allocation != Allocation::none() {
bundledata.allocation
if bundledata.allocation.is_some() {
Some(bundledata.allocation)
} else {
trace!(" -> spillset {:?}", bundledata.spillset);
trace!(
" -> spill slot {:?}",
self.spillsets[bundledata.spillset].slot
);
self.spillslots[self.spillsets[bundledata.spillset].slot.index()].alloc
None
}
}

Expand Down Expand Up @@ -170,7 +174,7 @@ impl<'a, F: Function> Env<'a, F> {
env: &Env<'a, F>,
from: Block,
to: Block,
) -> (ProgPoint, InsertMovePrio) {
) -> (ProgPoint, ProgPoint, InsertMovePrio) {
let from_last_insn = env.func.block_insns(from).last();
let to_first_insn = env.func.block_insns(to).first();
let from_is_ret = env.func.is_ret(from_last_insn);
Expand Down Expand Up @@ -199,10 +203,12 @@ impl<'a, F: Function> Env<'a, F> {
// instruction of there is only one successor per
// the given CFG information.
ProgPoint::before(from_last_insn),
ProgPoint::before(to_first_insn),
InsertMovePrio::OutEdgeMoves,
)
} else if to_ins <= 1 {
(
ProgPoint::before(to_first_insn),
ProgPoint::before(to_first_insn),
InsertMovePrio::InEdgeMoves,
)
Expand Down Expand Up @@ -279,6 +285,8 @@ impl<'a, F: Function> Env<'a, F> {
continue;
}

let spill_alloc = self.get_spill_alloc_for(vreg);

inter_block_sources.clear();

// For each range in each vreg, insert moves or
Expand All @@ -288,7 +296,7 @@ impl<'a, F: Function> Env<'a, F> {
let mut prev = PrevBuffer::new(blockparam_in_idx);
for range_idx in 0..self.vregs[vreg].ranges.len() {
let entry = self.vregs[vreg].ranges[range_idx];
let alloc = self.get_alloc_for_range(entry.index);
let alloc = self.get_alloc_for_range(entry.index).unwrap_or(spill_alloc);
let range = entry.range;
trace!(
"apply_allocations: vreg {:?} LR {:?} with range {:?} has alloc {:?}",
Expand All @@ -299,6 +307,12 @@ impl<'a, F: Function> Env<'a, F> {
);
debug_assert!(alloc != Allocation::none());

debug_assert!(
spill_alloc.is_none()
|| self.spillsets[self.bundles[self.ranges[entry.index].bundle].spillset]
.required
);

if self.annotations_enabled {
self.annotate(
range.from,
Expand Down Expand Up @@ -345,12 +359,15 @@ impl<'a, F: Function> Env<'a, F> {
// before After (i.e. in the middle of a single
// instruction).
if let Some(prev) = prev.is_valid() {
let prev_alloc = self.get_alloc_for_range(prev.index);
let prev_alloc = self.get_alloc_for_range(prev.index).unwrap_or(spill_alloc);
debug_assert!(prev_alloc != Allocation::none());

if prev.range.to >= range.from
&& (prev.range.to > range.from || !self.is_start_of_block(range.from))
&& !self.ranges[entry.index].has_flag(LiveRangeFlag::StartsAtDef)

// Avoid moving into this liverange, if the target is the spill allocation.
&& alloc != spill_alloc
{
trace!(
"prev LR {} abuts LR {} in same block; moving {} -> {} for v{}",
Expand Down Expand Up @@ -571,6 +588,20 @@ impl<'a, F: Function> Env<'a, F> {
if let OperandConstraint::Reuse(_) = operand.constraint() {
reuse_input_insts.push(inst);
}

// Defs need to write to the spill slot when it's present.
if OperandKind::Def == operand.kind() && spill_alloc.is_some() {
let pos = ProgPoint::before(inst.next());

trace!("def causing write to spill slot {spill_alloc:?} at {pos:?}");
inserted_moves.push(
pos,
InsertMovePrio::Regular,
alloc,
spill_alloc,
self.vreg(vreg),
);
}
}

// Scan debug-labels on this vreg that overlap with
Expand Down Expand Up @@ -637,7 +668,7 @@ impl<'a, F: Function> Env<'a, F> {
dest.to
);

let (pos, prio) = choose_move_location(self, dest.from, dest.to);
let (pos, _, prio) = choose_move_location(self, dest.from, dest.to);
inserted_moves.push(pos, prio, src, dest.alloc, vreg);
}
}
Expand All @@ -653,8 +684,24 @@ impl<'a, F: Function> Env<'a, F> {
for dest in block_param_dests {
let src = dest.source();
let src_alloc = block_param_sources.get(&src).unwrap();
let (pos, prio) = choose_move_location(self, dest.from_block, dest.to_block);
inserted_moves.push(pos, prio, *src_alloc, dest.alloc, self.vreg(dest.to_vreg));
let (pos, spill_pos, prio) =
choose_move_location(self, dest.from_block, dest.to_block);
let vreg = self.vreg(dest.to_vreg);
inserted_moves.push(pos, prio, *src_alloc, dest.alloc, vreg);

// Make sure to populate the spill slot in the `to` block. This should always be a
// `Regular` priority move, as it needs to happen after the `InEdgeMoves` move that
// defines it.
let spill_alloc = self.get_spill_alloc_for(dest.to_vreg);
if spill_alloc.is_some() {
inserted_moves.push(
spill_pos,
InsertMovePrio::Regular,
dest.alloc,
spill_alloc,
vreg,
);
}
}
}

Expand Down Expand Up @@ -780,65 +827,6 @@ impl<'a, F: Function> Env<'a, F> {
.moves
.sort_unstable_by_key(|m| m.pos_prio.key());

// Redundant-move elimination state tracker.
let mut redundant_moves = RedundantMoveEliminator::default();

fn redundant_move_process_side_effects<'a, F: Function>(
this: &Env<'a, F>,
redundant_moves: &mut RedundantMoveEliminator,
from: ProgPoint,
to: ProgPoint,
) {
// If any safepoints in range, clear and return.
// Also, if we cross a block boundary, clear and return.
if this.cfginfo.insn_block[from.inst().index()]
!= this.cfginfo.insn_block[to.inst().index()]
{
redundant_moves.clear();
return;
}
for inst in from.inst().index()..=to.inst().index() {
if this.func.requires_refs_on_stack(Inst::new(inst)) {
redundant_moves.clear();
return;
}
}

let start_inst = if from.pos() == InstPosition::Before {
from.inst()
} else {
from.inst().next()
};
let end_inst = if to.pos() == InstPosition::Before {
to.inst()
} else {
to.inst().next()
};
for inst in start_inst.index()..end_inst.index() {
let inst = Inst::new(inst);
for (i, op) in this.func.inst_operands(inst).iter().enumerate() {
match op.kind() {
OperandKind::Def => {
let alloc = this.get_alloc(inst, i);
redundant_moves.clear_alloc(alloc);
}
_ => {}
}
}
for reg in this.func.inst_clobbers(inst) {
redundant_moves.clear_alloc(Allocation::reg(reg));
}
// The dedicated scratch registers may be clobbered by any
// instruction.
for reg in this.env.scratch_by_class {
if let Some(reg) = reg {
redundant_moves.clear_alloc(Allocation::reg(reg));
}
}
}
}

let mut last_pos = ProgPoint::before(Inst::new(0));
let mut edits = Edits::with_capacity(self.func.num_insts());

while i < inserted_moves.moves.len() {
Expand All @@ -849,9 +837,6 @@ impl<'a, F: Function> Env<'a, F> {
}
let moves = &inserted_moves.moves[start..i];

redundant_move_process_side_effects(self, &mut redundant_moves, last_pos, pos_prio.pos);
last_pos = pos_prio.pos;

// Gather all the moves in each RegClass separately.
// These cannot interact, so it is safe to have separate
// ParallelMove instances. They need to be separate because
Expand Down Expand Up @@ -984,12 +969,7 @@ impl<'a, F: Function> Env<'a, F> {
let src = rewrites.get(&src).cloned().unwrap_or(src);
let dst = rewrites.get(&dst).cloned().unwrap_or(dst);
trace!(" resolved: {} -> {} ({:?})", src, dst, to_vreg);
let action = redundant_moves.process_move(src, dst, to_vreg);
if !action.elide {
edits.add(pos_prio, src, dst);
} else {
trace!(" -> redundant move elided");
}
edits.add(pos_prio, src, dst);
}
}
}
Expand Down