Skip to content

Commit 34a9ae7

Browse files
elliotttelliottt
authored
Misc refactorings (#116)
* Use a while-let instead of checking is_empty and popping * This conditional should always be true, as we expect the input is in ssa * Use iter_mut instead of iterating the index * We don't support multiple defs of the same vreg anymore * Drain instead of clear
1 parent 1e8da4f commit 34a9ae7

1 file changed

Lines changed: 136 additions & 141 deletions

File tree

‎src/ion/liveranges.rs‎

Lines changed: 136 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -334,8 +334,7 @@ impl<'a, F: Function> Env<'a, F> {
334334
workqueue_set.insert(block);
335335
}
336336

337-
while !workqueue.is_empty() {
338-
let block = workqueue.pop_front().unwrap();
337+
while let Some(block) = workqueue.pop_front() {
339338
workqueue_set.remove(&block);
340339
let insns = self.func.block_insns(block);
341340

@@ -522,140 +521,141 @@ impl<'a, F: Function> Env<'a, F> {
522521

523522
// If this is a move, handle specially.
524523
if let Some((src, dst)) = self.func.is_move(inst) {
525-
// We can completely skip the move if it is
526-
// trivial (vreg to same vreg).
527-
if src.vreg() != dst.vreg() {
528-
trace!(" -> move inst{}: src {} -> dst {}", inst.index(), src, dst);
529-
530-
debug_assert_eq!(src.class(), dst.class());
531-
debug_assert_eq!(src.kind(), OperandKind::Use);
532-
debug_assert_eq!(src.pos(), OperandPos::Early);
533-
debug_assert_eq!(dst.kind(), OperandKind::Def);
534-
debug_assert_eq!(dst.pos(), OperandPos::Late);
535-
536-
// Redefine src and dst operands to have
537-
// positions of After and Before respectively
538-
// (see note below), and to have Any
539-
// constraints if they were originally Reg.
540-
let src_constraint = match src.constraint() {
541-
OperandConstraint::Reg => OperandConstraint::Any,
542-
x => x,
543-
};
544-
let dst_constraint = match dst.constraint() {
545-
OperandConstraint::Reg => OperandConstraint::Any,
546-
x => x,
547-
};
548-
let src = Operand::new(
549-
src.vreg(),
550-
src_constraint,
551-
OperandKind::Use,
552-
OperandPos::Late,
524+
assert!(
525+
src.vreg() != dst.vreg(),
526+
"Invalid move: overwriting an SSA value"
527+
);
528+
529+
trace!(" -> move inst{}: src {} -> dst {}", inst.index(), src, dst);
530+
531+
debug_assert_eq!(src.class(), dst.class());
532+
debug_assert_eq!(src.kind(), OperandKind::Use);
533+
debug_assert_eq!(src.pos(), OperandPos::Early);
534+
debug_assert_eq!(dst.kind(), OperandKind::Def);
535+
debug_assert_eq!(dst.pos(), OperandPos::Late);
536+
537+
// Redefine src and dst operands to have
538+
// positions of After and Before respectively
539+
// (see note below), and to have Any
540+
// constraints if they were originally Reg.
541+
let src_constraint = match src.constraint() {
542+
OperandConstraint::Reg => OperandConstraint::Any,
543+
x => x,
544+
};
545+
let dst_constraint = match dst.constraint() {
546+
OperandConstraint::Reg => OperandConstraint::Any,
547+
x => x,
548+
};
549+
let src = Operand::new(
550+
src.vreg(),
551+
src_constraint,
552+
OperandKind::Use,
553+
OperandPos::Late,
554+
);
555+
let dst = Operand::new(
556+
dst.vreg(),
557+
dst_constraint,
558+
OperandKind::Def,
559+
OperandPos::Early,
560+
);
561+
562+
if self.annotations_enabled {
563+
self.annotate(
564+
ProgPoint::after(inst),
565+
format!(
566+
" prog-move v{} ({:?}) -> v{} ({:?})",
567+
src.vreg().vreg(),
568+
src_constraint,
569+
dst.vreg().vreg(),
570+
dst_constraint,
571+
),
553572
);
554-
let dst = Operand::new(
555-
dst.vreg(),
556-
dst_constraint,
557-
OperandKind::Def,
558-
OperandPos::Early,
573+
}
574+
575+
// N.B.: in order to integrate with the move
576+
// resolution that joins LRs in general, we
577+
// conceptually treat the move as happening
578+
// between the move inst's After and the next
579+
// inst's Before. Thus the src LR goes up to
580+
// (exclusive) next-inst-pre, and the dst LR
581+
// starts at next-inst-pre. We have to take
582+
// care in our move insertion to handle this
583+
// like other inter-inst moves, i.e., at
584+
// `Regular` priority, so it properly happens
585+
// in parallel with other inter-LR moves.
586+
//
587+
// Why the progpoint between move and next
588+
// inst, and not the progpoint between prev
589+
// inst and move? Because a move can be the
590+
// first inst in a block, but cannot be the
591+
// last; so the following progpoint is always
592+
// within the same block, while the previous
593+
// one may be an inter-block point (and the
594+
// After of the prev inst in a different
595+
// block).
596+
597+
// Handle the def w.r.t. liveranges: trim the
598+
// start of the range and mark it dead at this
599+
// point in our backward scan.
600+
let pos = ProgPoint::before(inst.next());
601+
let mut dst_lr = vreg_ranges[dst.vreg().vreg()];
602+
if !live.get(dst.vreg().vreg()) {
603+
let from = pos;
604+
let to = pos.next();
605+
dst_lr = self.add_liverange_to_vreg(
606+
VRegIndex::new(dst.vreg().vreg()),
607+
CodeRange { from, to },
559608
);
609+
trace!(" -> invalid LR for def; created {:?}", dst_lr);
610+
}
611+
trace!(" -> has existing LR {:?}", dst_lr);
612+
// Trim the LR to start here.
613+
if self.ranges[dst_lr.index()].range.from
614+
== self.cfginfo.block_entry[block.index()]
615+
{
616+
trace!(" -> started at block start; trimming to {:?}", pos);
617+
self.ranges[dst_lr.index()].range.from = pos;
618+
}
619+
self.ranges[dst_lr.index()].set_flag(LiveRangeFlag::StartsAtDef);
620+
live.set(dst.vreg().vreg(), false);
621+
vreg_ranges[dst.vreg().vreg()] = LiveRangeIndex::invalid();
622+
623+
// Handle the use w.r.t. liveranges: make it live
624+
// and create an initial LR back to the start of
625+
// the block.
626+
let pos = ProgPoint::after(inst);
627+
let src_lr = if !live.get(src.vreg().vreg()) {
628+
let range = CodeRange {
629+
from: self.cfginfo.block_entry[block.index()],
630+
to: pos.next(),
631+
};
632+
let src_lr =
633+
self.add_liverange_to_vreg(VRegIndex::new(src.vreg().vreg()), range);
634+
vreg_ranges[src.vreg().vreg()] = src_lr;
635+
src_lr
636+
} else {
637+
vreg_ranges[src.vreg().vreg()]
638+
};
560639

561-
if self.annotations_enabled {
562-
self.annotate(
563-
ProgPoint::after(inst),
564-
format!(
565-
" prog-move v{} ({:?}) -> v{} ({:?})",
566-
src.vreg().vreg(),
567-
src_constraint,
568-
dst.vreg().vreg(),
569-
dst_constraint,
570-
),
571-
);
572-
}
640+
trace!(" -> src LR {:?}", src_lr);
573641

574-
// N.B.: in order to integrate with the move
575-
// resolution that joins LRs in general, we
576-
// conceptually treat the move as happening
577-
// between the move inst's After and the next
578-
// inst's Before. Thus the src LR goes up to
579-
// (exclusive) next-inst-pre, and the dst LR
580-
// starts at next-inst-pre. We have to take
581-
// care in our move insertion to handle this
582-
// like other inter-inst moves, i.e., at
583-
// `Regular` priority, so it properly happens
584-
// in parallel with other inter-LR moves.
585-
//
586-
// Why the progpoint between move and next
587-
// inst, and not the progpoint between prev
588-
// inst and move? Because a move can be the
589-
// first inst in a block, but cannot be the
590-
// last; so the following progpoint is always
591-
// within the same block, while the previous
592-
// one may be an inter-block point (and the
593-
// After of the prev inst in a different
594-
// block).
595-
596-
// Handle the def w.r.t. liveranges: trim the
597-
// start of the range and mark it dead at this
598-
// point in our backward scan.
599-
let pos = ProgPoint::before(inst.next());
600-
let mut dst_lr = vreg_ranges[dst.vreg().vreg()];
601-
if !live.get(dst.vreg().vreg()) {
602-
let from = pos;
603-
let to = pos.next();
604-
dst_lr = self.add_liverange_to_vreg(
605-
VRegIndex::new(dst.vreg().vreg()),
606-
CodeRange { from, to },
607-
);
608-
trace!(" -> invalid LR for def; created {:?}", dst_lr);
609-
}
610-
trace!(" -> has existing LR {:?}", dst_lr);
611-
// Trim the LR to start here.
612-
if self.ranges[dst_lr.index()].range.from
613-
== self.cfginfo.block_entry[block.index()]
614-
{
615-
trace!(" -> started at block start; trimming to {:?}", pos);
616-
self.ranges[dst_lr.index()].range.from = pos;
617-
}
618-
self.ranges[dst_lr.index()].set_flag(LiveRangeFlag::StartsAtDef);
619-
live.set(dst.vreg().vreg(), false);
620-
vreg_ranges[dst.vreg().vreg()] = LiveRangeIndex::invalid();
621-
622-
// Handle the use w.r.t. liveranges: make it live
623-
// and create an initial LR back to the start of
624-
// the block.
625-
let pos = ProgPoint::after(inst);
626-
let src_lr = if !live.get(src.vreg().vreg()) {
627-
let range = CodeRange {
628-
from: self.cfginfo.block_entry[block.index()],
629-
to: pos.next(),
630-
};
631-
let src_lr = self
632-
.add_liverange_to_vreg(VRegIndex::new(src.vreg().vreg()), range);
633-
vreg_ranges[src.vreg().vreg()] = src_lr;
634-
src_lr
635-
} else {
636-
vreg_ranges[src.vreg().vreg()]
637-
};
642+
// Add to live-set.
643+
let src_is_dead_after_move = !live.get(src.vreg().vreg());
644+
live.set(src.vreg().vreg(), true);
638645

639-
trace!(" -> src LR {:?}", src_lr);
640-
641-
// Add to live-set.
642-
let src_is_dead_after_move = !live.get(src.vreg().vreg());
643-
live.set(src.vreg().vreg(), true);
644-
645-
// Add to program-moves lists.
646-
self.prog_move_srcs.push((
647-
(VRegIndex::new(src.vreg().vreg()), inst),
648-
Allocation::none(),
649-
));
650-
self.prog_move_dsts.push((
651-
(VRegIndex::new(dst.vreg().vreg()), inst.next()),
652-
Allocation::none(),
653-
));
654-
self.stats.prog_moves += 1;
655-
if src_is_dead_after_move {
656-
self.stats.prog_moves_dead_src += 1;
657-
self.prog_move_merges.push((src_lr, dst_lr));
658-
}
646+
// Add to program-moves lists.
647+
self.prog_move_srcs.push((
648+
(VRegIndex::new(src.vreg().vreg()), inst),
649+
Allocation::none(),
650+
));
651+
self.prog_move_dsts.push((
652+
(VRegIndex::new(dst.vreg().vreg()), inst.next()),
653+
Allocation::none(),
654+
));
655+
self.stats.prog_moves += 1;
656+
if src_is_dead_after_move {
657+
self.stats.prog_moves_dead_src += 1;
658+
self.prog_move_merges.push((src_lr, dst_lr));
659659
}
660660

661661
continue;
@@ -959,12 +959,9 @@ impl<'a, F: Function> Env<'a, F> {
959959
}
960960
}
961961

962-
for range in 0..self.ranges.len() {
963-
self.ranges[range].uses.reverse();
964-
debug_assert!(self.ranges[range]
965-
.uses
966-
.windows(2)
967-
.all(|win| win[0].pos <= win[1].pos));
962+
for range in &mut self.ranges {
963+
range.uses.reverse();
964+
debug_assert!(range.uses.windows(2).all(|win| win[0].pos <= win[1].pos));
968965
}
969966

970967
// Insert safepoint virtual stack uses, if needed.
@@ -1032,7 +1029,7 @@ impl<'a, F: Function> Env<'a, F> {
10321029

10331030
pub fn fixup_multi_fixed_vregs(&mut self) {
10341031
// Do a fixed-reg cleanup pass: if there are any LiveRanges with
1035-
// multiple uses (or defs) at the same ProgPoint and there is
1032+
// multiple uses at the same ProgPoint and there is
10361033
// more than one FixedReg constraint at that ProgPoint, we
10371034
// need to record all but one of them in a special fixup list
10381035
// and handle them later; otherwise, bundle-splitting to
@@ -1154,15 +1151,13 @@ impl<'a, F: Function> Env<'a, F> {
11541151
}
11551152
}
11561153

1157-
for &(clobber, pos) in &extra_clobbers {
1154+
for (clobber, pos) in extra_clobbers.drain(..) {
11581155
let range = CodeRange {
11591156
from: pos,
11601157
to: pos.next(),
11611158
};
11621159
self.add_liverange_to_preg(range, clobber);
11631160
}
1164-
1165-
extra_clobbers.clear();
11661161
}
11671162
}
11681163
}

0 commit comments

Comments
 (0)