Skip to content

Commit

Permalink
Rewrite the optimiser to be a one pass optimiser.
Browse files Browse the repository at this point in the history
[Well, OK the first lie is in the first line. Because we have to do
backwards code generation, we actually have to do a second pass. But
that's a boring detail, and something that predates this commit.]

This commit moves the optimiser into a model that's closer to RPython:
rather than multiple forward stages each doing one optimisation, we have
a single forward stage that does all optimisations in one go. This is
quicker (fairly obviously!) but also simpler, because it means that we
can make strong assumptions at the point of the wavefront: no-one can
change our knowledge of earlier parts of the trace.

The crucial difference between this commit and what came before is that
as well as mutating the IR as we go along (e.g. constant folding) we
maintain an analysis of what values an instruction can produce, which we
gradually refine. Crucially, mutation can only happen at the current
instruction, but analysis refinement can happen on previous
instructions.

Consider a trace along these lines:

```
%0: i8 = load_ti ... ; at this point we know nothing about %0
%1: i8 = add %0, 1i8 ; we still know nothing about %0
%2: i1 = eq %0, 0i8  ; we know that henceforth %0 must be 0i8
guard true %2
%4: I8 = mul %0, %0  ; we know this must equal the constant 0i8
```

Notice that it's only at the point of the guard that our analysis allows
us to start using the knowledge "%0 = 0i8" for optimisations.

The code is currently woefully incomplete (but if I fill in anything
else, I trip up on the "removing guards tends to kill things" bug that I
believe is fixed but not yet merged), I haven't yet bothered porting
across one old optimisation (`mul_chain`), and I haven't really got a
good API for sharing analysis and mutations (which is currently done
inconsistently).

All that said, this nudges the optimiser forward just about enough that
I think it's worth considering for merging now, and then it can be
improved in-tree.
  • Loading branch information
ltratt committed Oct 3, 2024
1 parent 536ef29 commit 9aa070c
Show file tree
Hide file tree
Showing 6 changed files with 611 additions and 457 deletions.
24 changes: 23 additions & 1 deletion ykrt/src/compile/jitc_yk/aot_ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -508,20 +508,42 @@ impl BBlockId {
}
}

/// Predicates for use in numeric comparisons.
/// Predicates for use in numeric comparisons. These are directly based on [LLVM's `icmp`
/// semantics](https://llvm.org/docs/LangRef.html#icmp-instruction). All quotes below are taken
/// from there.
#[deku_derive(DekuRead)]
#[derive(Debug, Eq, PartialEq, Clone, Copy)]
#[deku(type = "u8")]
pub(crate) enum Predicate {
/// "eq: yields true if the operands are equal, false otherwise. No sign
/// interpretation is necessary or performed."
Equal = 0,
/// "ne: yields true if the operands are unequal, false otherwise. No sign
/// interpretation is necessary or performed."
NotEqual,
/// "ugt: interprets the operands as unsigned values and yields true if op1 is
/// greater than op2."
UnsignedGreater,
/// "uge: interprets the operands as unsigned values and yields true if op1 is
/// greater than or equal to op2."
UnsignedGreaterEqual,
/// "ule: interprets the operands as unsigned values and yields true if op1 is
/// less than or equal to op2."
UnsignedLess,
/// "ule: interprets the operands as unsigned values and yields true if op1 is
/// less than or equal to op2."
UnsignedLessEqual,
/// "sgt: interprets the operands as signed values and yields true if op1 is greater
/// than op2."
SignedGreater,
/// "sge: interprets the operands as signed values and yields true if op1 is
/// greater than or equal to op2."
SignedGreaterEqual,
/// "slt: interprets the operands as signed values and yields true if op1 is less
/// than op2."
SignedLess,
/// "sle: interprets the operands as signed values and yields true if op1 is less
/// than or equal to op2."
SignedLessEqual,
}

Expand Down
12 changes: 4 additions & 8 deletions ykrt/src/compile/jitc_yk/codegen/x64/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,15 +404,11 @@ impl<'a> Assemble<'a> {
}
}

fn cg_binop(
&mut self,
iidx: jit_ir::InstIdx,
jit_ir::BinOpInst { lhs, binop, rhs }: &jit_ir::BinOpInst,
) {
let lhs = lhs.unpack(self.m);
let rhs = rhs.unpack(self.m);
fn cg_binop(&mut self, iidx: jit_ir::InstIdx, inst: &jit_ir::BinOpInst) {
let lhs = inst.lhs(self.m);
let rhs = inst.rhs(self.m);

match binop {
match inst.binop() {
BinOp::Add => {
let size = lhs.byte_size(self.m);
let [lhs_reg, rhs_reg] = self.ra.assign_gp_regs(
Expand Down
27 changes: 15 additions & 12 deletions ykrt/src/compile/jitc_yk/jit_ir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ impl Module {
/// This function has very few uses and unless you explicitly know why you're using it, you
/// should instead use [Self::inst_no_copies] because not handling `Copy` instructions
/// correctly leads to undefined behaviour.
fn inst_raw(&self, iidx: InstIdx) -> Inst {
pub(crate) fn inst_raw(&self, iidx: InstIdx) -> Inst {
self.insts[usize::from(iidx)]
}

Expand Down Expand Up @@ -1166,7 +1166,7 @@ impl fmt::Display for DisplayableOperand<'_> {
/// Note that this struct deliberately does not implement `PartialEq` (or `Eq`): two instances of
/// `Const` may represent the same underlying constant, but (because of floats), you as the user
/// need to determine what notion of equality you wish to use on a given const.
#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq)]
pub(crate) enum Const {
Float(TyIdx, f64),
/// A constant integer at most 64 bits wide. This can be treated a signed or unsigned integer
Expand All @@ -1185,15 +1185,6 @@ impl Const {
}
}

/// If this constant is an integer that can be represented in 64 bits, return it as an `i64`.
pub(crate) fn int_to_u64(&self) -> Option<u64> {
match self {
Const::Float(_, _) => None,
Const::Int(_, x) => Some(*x),
Const::Ptr(_) => None,
}
}

/// Create an integer of the same underlying type and with the value `x`.
///
/// # Panics
Expand Down Expand Up @@ -1909,7 +1900,7 @@ pub(crate) struct BinOpInst {
/// The left-hand side of the operation.
pub(crate) lhs: PackedOperand,
/// The operation to perform.
pub(crate) binop: BinOp,
binop: BinOp,
/// The right-hand side of the operation.
pub(crate) rhs: PackedOperand,
}
Expand All @@ -1923,6 +1914,18 @@ impl BinOpInst {
}
}

pub(crate) fn lhs(&self, m: &Module) -> Operand {
self.lhs.unpack(m)
}

pub(crate) fn binop(&self) -> BinOp {
self.binop
}

pub(crate) fn rhs(&self, m: &Module) -> Operand {
self.rhs.unpack(m)
}

/// Returns the type index of the operands being added.
pub(crate) fn tyidx(&self, m: &Module) -> TyIdx {
self.lhs.unpack(m).tyidx(m)
Expand Down
82 changes: 82 additions & 0 deletions ykrt/src/compile/jitc_yk/opt/analyse.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
//! Analyse a trace and gradually refine what values we know a previous instruction can produce.
use super::{
super::jit_ir::{GuardInst, Inst, InstIdx, Module, Operand, Predicate},
Value,
};

/// Ongoing analysis of a trace: what value can a given instruction in the past produce?
///
/// Note that the analysis is forward-looking: just because an instruction's `Value` is (say) a
/// `Const` now does not mean it would be valid to assume that at earlier points it is safe to
/// assume it was also a `Const`.
pub(super) struct Analyse {
/// For each instruction, what have we learnt about its [Value] so far?
values: Vec<Value>,
}

impl Analyse {
pub(super) fn new(m: &Module) -> Analyse {
Analyse {
values: vec![Value::Unknown; m.insts_len()],
}
}

/// Map `op` based on our analysis so far. In some cases this will return `op` unchanged, but
/// in others it may be able to turn what looks like a variable reference into a constant.
pub(super) fn op_map(&mut self, m: &Module, op: Operand) -> Operand {
match op {
Operand::Var(iidx) => match self.values[usize::from(iidx)] {
Value::Unknown => {
// Since we last saw an `ICmp` instruction, we may have gathered new knowledge
// that allows us to turn it into a constant.
if let (iidx, Inst::ICmp(inst)) = m.inst_decopy(iidx) {
let lhs = self.op_map(m, inst.lhs(m));
let pred = inst.predicate();
let rhs = self.op_map(m, inst.rhs(m));
if let (&Operand::Const(lhs_cidx), &Operand::Const(rhs_cidx)) = (&lhs, &rhs)
{
if pred == Predicate::Equal && m.const_(lhs_cidx) == m.const_(rhs_cidx)
{
self.values[usize::from(iidx)] = Value::Const(m.true_constidx());
return Operand::Const(m.true_constidx());
}
}
}
op
}
Value::Const(cidx) => Operand::Const(cidx),
},
Operand::Const(_) => op,
}
}

/// Update our idea of what value the instruction at `iidx` can produce.
pub(super) fn set_value(&mut self, iidx: InstIdx, v: Value) {
self.values[usize::from(iidx)] = v;
}

/// Use the guard `inst` to update our knowledge about the variable used as its condition.
pub(super) fn guard(&mut self, m: &Module, inst: GuardInst) {
if let Operand::Var(iidx) = inst.cond(m) {
if let (_, Inst::ICmp(inst)) = m.inst_decopy(iidx) {
let lhs = self.op_map(m, inst.lhs(m));
let pred = inst.predicate();
let rhs = self.op_map(m, inst.rhs(m));
match (&lhs, &rhs) {
(&Operand::Const(_), &Operand::Const(_)) => {
// This will have been handled by icmp/guard optimisations.
unreachable!();
}
(&Operand::Var(iidx), &Operand::Const(cidx))
| (&Operand::Const(cidx), &Operand::Var(iidx)) => {
if pred == Predicate::Equal {
self.set_value(iidx, Value::Const(cidx));
}
}
(&Operand::Var(_), &Operand::Var(_)) => (),
}
}
}
}
}
Loading

0 comments on commit 9aa070c

Please sign in to comment.