Merge pull request #1410 from ltratt/single_pass_optimiser

Rewrite the optimiser to be a one pass optimiser.

ykrt/src/compile/jitc_yk/aot_ir.rs

@@ -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,
 }
 

ykrt/src/compile/jitc_yk/codegen/x64/mod.rs

@@ -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(

ykrt/src/compile/jitc_yk/jit_ir/mod.rs

@@ -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)]
     }
 
@@ -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
@@ -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
@@ -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,
 }
@@ -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)

ykrt/src/compile/jitc_yk/opt/analyse.rs

@@ -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(_)) => (),
+                }
+            }
+        }
+    }
+}