Skip to content

Commit

Permalink
Deal properly with OutputCanBeSameAsInput instructions.
Browse files Browse the repository at this point in the history
The challenge here for the reverse analyser is that some instructions --
currently `PtrAdd`s and `Load`s -- can keep a variable alive without
having the register allocator explicitly move it. Fortunately the logic
for this is relatively simple.
  • Loading branch information
ltratt committed Dec 29, 2024
1 parent 4192d08 commit b3ee23f
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 8 deletions.
4 changes: 2 additions & 2 deletions ykrt/src/compile/jitc_yk/codegen/x64/lsregalloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,8 @@ impl<'a> LSRegAlloc<'a> {
cur_iidx: InstIdx,
query_iidx: InstIdx,
) -> bool {
usize::from(cur_iidx)
< usize::from(self.rev_an.inst_vals_alive_until[usize::from(query_iidx)])
self.rev_an
.is_inst_var_still_used_after(cur_iidx, query_iidx)
}

/// Is the value produced by instruction `query_iidx` used at or after instruction `cur_idx`?
Expand Down
34 changes: 28 additions & 6 deletions ykrt/src/compile/jitc_yk/codegen/x64/rev_analyse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,16 @@ impl<'a> RevAnalyse<'a> {
!self.used_insts[usize::from(iidx)]
}

/// Is the value produced by instruction `query_iidx` used after (but not including!)
/// instruction `cur_idx`?
pub(crate) fn is_inst_var_still_used_after(
&self,
cur_iidx: InstIdx,
query_iidx: InstIdx,
) -> bool {
usize::from(cur_iidx) < usize::from(self.inst_vals_alive_until[usize::from(query_iidx)])
}

/// Propagate the hint for the instruction being processed at `iidx` to `op`, if appropriate
/// for `op`.
fn push_reg_hint(&mut self, iidx: InstIdx, op: Operand) {
Expand All @@ -183,6 +193,19 @@ impl<'a> RevAnalyse<'a> {
}
}

/// Propagate the hint for the [RegConstraint::OutputCanBeSameAsInput] instruction being
/// processed at `iidx` to `op`, if appropriate for `op`.
///
/// Note: this function should only be used for situations where an instruction can, with no
/// special help from the register allocator, move a value.
fn push_reg_hint_outputcanbesameasinput(&mut self, iidx: InstIdx, op: Operand) {
if let Operand::Var(op_iidx) = op {
if !self.is_inst_var_still_used_after(iidx, op_iidx) {
self.reg_hints[usize::from(op_iidx)] = self.reg_hints[usize::from(iidx)];
}
}
}

/// Set the hint for to `op` to `reg`, if appropriate for `op`.
fn push_reg_hint_fixed(&mut self, op: Operand, reg: Register) {
if let Operand::Var(op_iidx) = op {
Expand All @@ -203,13 +226,12 @@ impl<'a> RevAnalyse<'a> {
self.push_reg_hint_fixed(binst.lhs(self.m), Register::GP(Rq::RAX));
}
BinOp::Sub => match (binst.lhs(self.m), binst.rhs(self.m)) {
(_, Operand::Const(_)) => {
self.push_reg_hint(iidx, binst.lhs(self.m));
(Operand::Const(_), _) => {
self.push_reg_hint(iidx, binst.rhs(self.m));
}
(Operand::Var(_), _) => {
_ => {
self.push_reg_hint(iidx, binst.lhs(self.m));
}
_ => (),
},
_ => (),
}
Expand All @@ -220,7 +242,7 @@ impl<'a> RevAnalyse<'a> {
}

fn an_ptradd(&mut self, iidx: InstIdx, painst: PtrAddInst) {
self.push_reg_hint(iidx, painst.ptr(self.m));
self.push_reg_hint_outputcanbesameasinput(iidx, painst.ptr(self.m));
}

fn an_dynptradd(&mut self, iidx: InstIdx, dpainst: DynPtrAddInst) {
Expand All @@ -236,7 +258,7 @@ impl<'a> RevAnalyse<'a> {
if let Operand::Var(y) = pa_inst.ptr(self.m) {
if self.inst_vals_alive_until[usize::from(y)] < iidx {
self.inst_vals_alive_until[usize::from(y)] = iidx;
self.push_reg_hint(iidx, pa_inst.ptr(self.m));
self.push_reg_hint_outputcanbesameasinput(iidx, pa_inst.ptr(self.m));
}
self.used_insts.set(usize::from(y), true);
}
Expand Down

0 comments on commit b3ee23f

Please sign in to comment.