From 67025b6890540afe9c5e91a9249291f17b8906fe Mon Sep 17 00:00:00 2001 From: Vineeth Kashyap Date: Mon, 13 Apr 2026 16:56:35 -0400 Subject: [PATCH 1/2] Move a fusing operation from lowering to destacking, where it belongs. --- .../specializer/src/destack/instr_utils.rs | 23 +- .../specializer/src/destack/ssa_conversion.rs | 14 +- .../specializer/src/destack/ssa_function.rs | 42 +++- .../specializer/src/lower/translate.rs | 208 +++++++++--------- .../src/stackless_exec_ir/display.rs | 36 ++- .../specializer/src/stackless_exec_ir/mod.rs | 42 +++- .../tests/test_cases/masm/arg_regs.exp | 30 ++- .../tests/test_cases/masm/control_flow.exp | 21 +- .../tests/test_cases/masm/vid_type_reset.exp | 24 +- .../tests/test_cases/move/basic.exp | 19 +- .../tests/test_cases/move/fibonacci.exp | 34 ++- 11 files changed, 309 insertions(+), 184 deletions(-) diff --git a/third_party/move/mono-move/specializer/src/destack/instr_utils.rs b/third_party/move/mono-move/specializer/src/destack/instr_utils.rs index 73c2a00f604..4f0c268330b 100644 --- a/third_party/move/mono-move/specializer/src/destack/instr_utils.rs +++ b/third_party/move/mono-move/specializer/src/destack/instr_utils.rs @@ -179,6 +179,8 @@ pub(crate) fn extract_imm_value(instr: &Instr) -> Option<(Slot, ImmValue)> { | Instr::Branch(_) | Instr::BrTrue(_, _) | Instr::BrFalse(_, _) + | Instr::BrCmp(_, _, _, _) + | Instr::BrCmpImm(_, _, _, _) | Instr::Ret(_) | Instr::Abort(_) | Instr::AbortMsg(_, _) => None, @@ -189,6 +191,7 @@ pub(crate) fn extract_imm_value(instr: &Instr) -> Option<(Slot, ImmValue)> { /// without changing the result). #[inline] pub(crate) fn is_commutative(op: &BinaryOp) -> bool { + use crate::stackless_exec_ir::CmpOp; matches!( op, BinaryOp::Add @@ -196,8 +199,8 @@ pub(crate) fn is_commutative(op: &BinaryOp) -> bool { | BinaryOp::BitOr | BinaryOp::BitAnd | BinaryOp::Xor - | BinaryOp::Eq - | BinaryOp::Neq + | BinaryOp::Cmp(CmpOp::Eq) + | BinaryOp::Cmp(CmpOp::Neq) | BinaryOp::Or | BinaryOp::And ) @@ -391,6 +394,11 @@ fn visit_slots( Instr::Branch(_) => {}, Instr::BrTrue(_, cond) | Instr::BrFalse(_, cond) => used::(*cond, &mut f), + Instr::BrCmp(_, _, lhs, rhs) => { + used::(*lhs, &mut f); + used::(*rhs, &mut f); + }, + Instr::BrCmpImm(_, _, src, _) => used::(*src, &mut f), Instr::Ret(rets) => uses::(rets, &mut f), Instr::Abort(code) => used::(*code, &mut f), Instr::AbortMsg(code, msg) => { @@ -662,6 +670,17 @@ fn rewrite_instr_slots { + if USES { + rewrite_slot(lhs, &mut f); + rewrite_slot(rhs, &mut f); + } + }, + Instr::BrCmpImm(_, _, src, _) => { + if USES { + rewrite_slot(src, &mut f); + } + }, Instr::Ret(rets) => { if USES { rewrite_slots(rets, &mut f); diff --git a/third_party/move/mono-move/specializer/src/destack/ssa_conversion.rs b/third_party/move/mono-move/specializer/src/destack/ssa_conversion.rs index 9fab538bdb2..bf9317ead42 100644 --- a/third_party/move/mono-move/specializer/src/destack/ssa_conversion.rs +++ b/third_party/move/mono-move/specializer/src/destack/ssa_conversion.rs @@ -11,7 +11,7 @@ use super::{ ssa_function::SSAFunction, type_conversion::{convert_sig_token, convert_sig_tokens}, }; -use crate::stackless_exec_ir::{BasicBlock, BinaryOp, Instr, Label, Slot, UnaryOp}; +use crate::stackless_exec_ir::{BasicBlock, BinaryOp, CmpOp, Instr, Label, Slot, UnaryOp}; use anyhow::{bail, ensure, Context, Result}; use move_binary_format::{ access::ModuleAccess, @@ -441,12 +441,12 @@ impl<'a> SsaConverter<'a> { B::Shl => self.convert_binop(BinaryOp::Shl, false)?, B::Shr => self.convert_binop(BinaryOp::Shr, false)?, // --- Comparisons / logical (result type = bool) --- - B::Lt => self.convert_binop(BinaryOp::Lt, true)?, - B::Gt => self.convert_binop(BinaryOp::Gt, true)?, - B::Le => self.convert_binop(BinaryOp::Le, true)?, - B::Ge => self.convert_binop(BinaryOp::Ge, true)?, - B::Eq => self.convert_binop(BinaryOp::Eq, true)?, - B::Neq => self.convert_binop(BinaryOp::Neq, true)?, + B::Lt => self.convert_binop(BinaryOp::Cmp(CmpOp::Lt), true)?, + B::Gt => self.convert_binop(BinaryOp::Cmp(CmpOp::Gt), true)?, + B::Le => self.convert_binop(BinaryOp::Cmp(CmpOp::Le), true)?, + B::Ge => self.convert_binop(BinaryOp::Cmp(CmpOp::Ge), true)?, + B::Eq => self.convert_binop(BinaryOp::Cmp(CmpOp::Eq), true)?, + B::Neq => self.convert_binop(BinaryOp::Cmp(CmpOp::Neq), true)?, B::Or => self.convert_binop(BinaryOp::Or, true)?, B::And => self.convert_binop(BinaryOp::And, true)?, diff --git a/third_party/move/mono-move/specializer/src/destack/ssa_function.rs b/third_party/move/mono-move/specializer/src/destack/ssa_function.rs index bb11c6df558..f27e119596f 100644 --- a/third_party/move/mono-move/specializer/src/destack/ssa_function.rs +++ b/third_party/move/mono-move/specializer/src/destack/ssa_function.rs @@ -9,7 +9,7 @@ //! once within its block and never crosses a block boundary. use super::instr_utils::{extract_imm_value, is_commutative}; -use crate::stackless_exec_ir::{BasicBlock, Instr}; +use crate::stackless_exec_ir::{BasicBlock, BinaryOp, Instr}; use move_vm_types::loaded_data::runtime_types::Type; /// Intermediate SSA representation of a single function, before slot allocation. @@ -31,6 +31,9 @@ impl SSAFunction { for block in &mut self.blocks { fuse_pairs(&mut block.instrs, try_fuse_field_access); fuse_pairs(&mut block.instrs, try_fuse_immediate_binop); + // Must run after try_fuse_immediate_binop so that BinaryOpImm is + // available for the BrCmpImm variant. + fuse_pairs(&mut block.instrs, try_fuse_compare_branch); } self } @@ -112,15 +115,48 @@ fn try_fuse_field_access(first: &Instr, second: &Instr) -> Option { } } +/// Try to fuse a comparison + conditional branch pair into a single `BrCmp`/`BrCmpImm`. +/// +/// Handles both `BrTrue` (keeps the comparison operator) and `BrFalse` (negates it). +fn try_fuse_compare_branch(first: &Instr, second: &Instr) -> Option { + match (first, second) { + // BinaryOp(dst, Cmp(cmp), lhs, rhs) + BrTrue(label, dst) + (Instr::BinaryOp(dst, BinaryOp::Cmp(cmp), lhs, rhs), Instr::BrTrue(label, cond)) + if *dst == *cond => + { + Some(Instr::BrCmp(*label, *cmp, *lhs, *rhs)) + }, + // BinaryOp(dst, Cmp(cmp), lhs, rhs) + BrFalse(label, dst) + (Instr::BinaryOp(dst, BinaryOp::Cmp(cmp), lhs, rhs), Instr::BrFalse(label, cond)) + if *dst == *cond => + { + Some(Instr::BrCmp(*label, cmp.negate(), *lhs, *rhs)) + }, + // BinaryOpImm(dst, Cmp(cmp), src, imm) + BrTrue(label, dst) + (Instr::BinaryOpImm(dst, BinaryOp::Cmp(cmp), src, imm), Instr::BrTrue(label, cond)) + if *dst == *cond => + { + Some(Instr::BrCmpImm(*label, *cmp, *src, *imm)) + }, + // BinaryOpImm(dst, Cmp(cmp), src, imm) + BrFalse(label, dst) + (Instr::BinaryOpImm(dst, BinaryOp::Cmp(cmp), src, imm), Instr::BrFalse(label, cond)) + if *dst == *cond => + { + Some(Instr::BrCmpImm(*label, cmp.negate(), *src, *imm)) + }, + _ => None, + } +} + /// Try to fuse a `Ld*` + `BinaryOp` pair into a `BinaryOpImm` instruction. fn try_fuse_immediate_binop(first: &Instr, second: &Instr) -> Option { let (tmp, imm) = extract_imm_value(first)?; match second { Instr::BinaryOp(dst, op, lhs, rhs) if *rhs == tmp => { - Some(Instr::BinaryOpImm(*dst, op.clone(), *lhs, imm)) + Some(Instr::BinaryOpImm(*dst, *op, *lhs, imm)) }, Instr::BinaryOp(dst, op, lhs, rhs) if *lhs == tmp && is_commutative(op) => { - Some(Instr::BinaryOpImm(*dst, op.clone(), *rhs, imm)) + Some(Instr::BinaryOpImm(*dst, *op, *rhs, imm)) }, _ => None, } diff --git a/third_party/move/mono-move/specializer/src/lower/translate.rs b/third_party/move/mono-move/specializer/src/lower/translate.rs index c1489c94fbb..ff3e32ade17 100644 --- a/third_party/move/mono-move/specializer/src/lower/translate.rs +++ b/third_party/move/mono-move/specializer/src/lower/translate.rs @@ -4,7 +4,7 @@ //! Lowers stackless exec IR to micro-ops. use super::context::{LoweringContext, SlotInfo}; -use crate::stackless_exec_ir::{BinaryOp, FunctionIR, ImmValue, Instr, Label, Slot}; +use crate::stackless_exec_ir::{BinaryOp, CmpOp, FunctionIR, ImmValue, Instr, Label, Slot}; use anyhow::{bail, Result}; use mono_move_core::{CodeOffset, FrameOffset, MicroOp}; use move_vm_types::loaded_data::runtime_types::Type; @@ -13,15 +13,8 @@ pub fn lower_function(func_ir: &FunctionIR, ctx: &LoweringContext) -> Result LoweringState<'a> { ) } - /// Lower one IR instruction. Returns true if the next instruction was fused (skip it). - fn lower_instr( - &mut self, - func_ir: &FunctionIR, - instr: &Instr, - next: Option<&Instr>, - ) -> Result { + /// Lower one IR instruction. + fn lower_instr(&mut self, func_ir: &FunctionIR, instr: &Instr) -> Result<()> { match instr { // --- Loads --- Instr::LdU64(dst, v) => { @@ -232,10 +220,6 @@ impl<'a> LoweringState<'a> { Instr::BinaryOp(dst, op, lhs, rhs) => { let lhs_ty = self.slot_type(*lhs); if Self::is_u64_type(lhs_ty) { - // Try compare+branch fusion - if let Some(fused) = self.try_fuse_compare_branch(op, dst, *lhs, *rhs, next) { - return Ok(fused); - } let l = self.slot(*lhs); let r = self.slot(*rhs); let d = self.def_slot(*dst); @@ -256,11 +240,6 @@ impl<'a> LoweringState<'a> { Instr::BinaryOpImm(dst, op, src, imm) => { let src_ty = self.slot_type(*src); if Self::is_u64_type(src_ty) { - // Try compare+branch fusion - if let Some(fused) = self.try_fuse_compare_branch_imm(op, dst, *src, imm, next) - { - return Ok(fused); - } let s = self.slot(*src); let d = self.def_slot(*dst); let v = imm_to_u64(imm); @@ -300,7 +279,90 @@ impl<'a> LoweringState<'a> { }); }, Instr::BrFalse(Label(_l), _cond) => { - bail!("standalone BrFalse not yet lowered (expected fusion)"); + bail!("standalone BrFalse not yet lowered (expected compare+branch fusion)"); + }, + + // --- Fused compare+branch --- + Instr::BrCmp(Label(l), op, lhs, rhs) => { + let lhs_ty = self.slot_type(*lhs); + if Self::is_u64_type(lhs_ty) { + let l_slot = self.slot(*lhs); + let r_slot = self.slot(*rhs); + let idx = self.ops.len(); + self.branch_fixups.push(idx); + match op { + CmpOp::Lt => self.emit(MicroOp::JumpLessU64 { + target: CodeOffset(encode_label(*l)), + lhs: FrameOffset(l_slot.offset), + rhs: FrameOffset(r_slot.offset), + }), + CmpOp::Ge => self.emit(MicroOp::JumpGreaterEqualU64 { + target: CodeOffset(encode_label(*l)), + lhs: FrameOffset(l_slot.offset), + rhs: FrameOffset(r_slot.offset), + }), + // x > y ↔ y < x + CmpOp::Gt => self.emit(MicroOp::JumpLessU64 { + target: CodeOffset(encode_label(*l)), + lhs: FrameOffset(r_slot.offset), + rhs: FrameOffset(l_slot.offset), + }), + // x <= y ↔ y >= x + CmpOp::Le => self.emit(MicroOp::JumpGreaterEqualU64 { + target: CodeOffset(encode_label(*l)), + lhs: FrameOffset(r_slot.offset), + rhs: FrameOffset(l_slot.offset), + }), + CmpOp::Neq => self.emit(MicroOp::JumpNotEqualU64 { + target: CodeOffset(encode_label(*l)), + lhs: FrameOffset(l_slot.offset), + rhs: FrameOffset(r_slot.offset), + }), + CmpOp::Eq => { + bail!("BrCmp Eq for u64-sized type not yet lowered") + }, + } + } else { + bail!("BrCmp for non-u64 type not yet lowered"); + } + }, + Instr::BrCmpImm(Label(l), op, src, imm) => { + let src_ty = self.slot_type(*src); + if Self::is_u64_type(src_ty) { + let s = self.slot(*src); + let v = imm_to_u64(imm); + let idx = self.ops.len(); + self.branch_fixups.push(idx); + match op { + CmpOp::Ge => self.emit(MicroOp::JumpGreaterEqualU64Imm { + target: CodeOffset(encode_label(*l)), + src: FrameOffset(s.offset), + imm: v, + }), + CmpOp::Lt => self.emit(MicroOp::JumpLessU64Imm { + target: CodeOffset(encode_label(*l)), + src: FrameOffset(s.offset), + imm: v, + }), + // x > c ↔ x >= c+1 + CmpOp::Gt => self.emit(MicroOp::JumpGreaterEqualU64Imm { + target: CodeOffset(encode_label(*l)), + src: FrameOffset(s.offset), + imm: v + 1, + }), + // x <= c ↔ x < c+1 + CmpOp::Le => self.emit(MicroOp::JumpLessU64Imm { + target: CodeOffset(encode_label(*l)), + src: FrameOffset(s.offset), + imm: v + 1, + }), + CmpOp::Eq | CmpOp::Neq => { + bail!("BrCmpImm {:?} for u64-sized type not yet lowered", op) + }, + } + } else { + bail!("BrCmpImm for non-u64 type not yet lowered"); + } }, // --- Calls --- @@ -323,7 +385,7 @@ impl<'a> LoweringState<'a> { _ => bail!("instruction {:?} not yet lowered", instr), } - Ok(false) + Ok(()) } fn lower_call(&mut self, _func_ir: &FunctionIR, args: &[Slot], rets: &[Slot]) { @@ -376,82 +438,17 @@ impl<'a> LoweringState<'a> { } } - /// Try to fuse a comparison (slot-slot) + branch into a single micro-op. - fn try_fuse_compare_branch( - &mut self, - op: &BinaryOp, - dst: &Slot, - lhs: Slot, - rhs: Slot, - next: Option<&Instr>, - ) -> Option { - let next = next?; - match (op, next) { - (BinaryOp::Lt, Instr::BrTrue(Label(l), cond)) if cond == dst => { - let l_slot = self.slot(lhs); - let r_slot = self.slot(rhs); - let idx = self.ops.len(); - self.branch_fixups.push(idx); - self.emit(MicroOp::JumpLessU64 { - target: CodeOffset(encode_label(*l)), - lhs: FrameOffset(l_slot.offset), - rhs: FrameOffset(r_slot.offset), - }); - Some(true) - }, - _ => None, - } - } - - /// Try to fuse a comparison (slot-imm) + branch into a single micro-op. - fn try_fuse_compare_branch_imm( - &mut self, - op: &BinaryOp, - dst: &Slot, - src: Slot, - imm: &ImmValue, - next: Option<&Instr>, - ) -> Option { - let next = next?; - match (op, next) { - (BinaryOp::Le, Instr::BrFalse(Label(l), cond)) if cond == dst => { - // le src, #c + br_false L => jump if src >= c+1 - let s = self.slot(src); - let v = imm_to_u64(imm); - let idx = self.ops.len(); - self.branch_fixups.push(idx); - self.emit(MicroOp::JumpGreaterEqualU64Imm { - target: CodeOffset(encode_label(*l)), - src: FrameOffset(s.offset), - imm: v + 1, - }); - Some(true) - }, - (BinaryOp::Lt, Instr::BrFalse(Label(l), cond)) if cond == dst => { - // lt src, #c + br_false L => jump if src >= c - let s = self.slot(src); - let v = imm_to_u64(imm); - let idx = self.ops.len(); - self.branch_fixups.push(idx); - self.emit(MicroOp::JumpGreaterEqualU64Imm { - target: CodeOffset(encode_label(*l)), - src: FrameOffset(s.offset), - imm: v, - }); - Some(true) - }, - _ => None, - } - } - fn fixup_branches(&mut self) -> Result<()> { for &idx in &self.branch_fixups { // Extract the encoded label from the op, resolve it, then patch. let encoded = match &self.ops[idx] { - MicroOp::Jump { target } => target.0, - MicroOp::JumpNotZeroU64 { target, .. } => target.0, - MicroOp::JumpGreaterEqualU64Imm { target, .. } => target.0, - MicroOp::JumpLessU64 { target, .. } => target.0, + MicroOp::Jump { target } + | MicroOp::JumpNotZeroU64 { target, .. } + | MicroOp::JumpGreaterEqualU64Imm { target, .. } + | MicroOp::JumpLessU64Imm { target, .. } + | MicroOp::JumpLessU64 { target, .. } + | MicroOp::JumpGreaterEqualU64 { target, .. } + | MicroOp::JumpNotEqualU64 { target, .. } => target.0, other => bail!( "unexpected non-branch op at fixup index {}: {:?}", idx, @@ -461,10 +458,13 @@ impl<'a> LoweringState<'a> { let label = decode_label(encoded); let resolved = self.resolve_label(label)?; match &mut self.ops[idx] { - MicroOp::Jump { target } => target.0 = resolved, - MicroOp::JumpNotZeroU64 { target, .. } => target.0 = resolved, - MicroOp::JumpGreaterEqualU64Imm { target, .. } => target.0 = resolved, - MicroOp::JumpLessU64 { target, .. } => target.0 = resolved, + MicroOp::Jump { target } + | MicroOp::JumpNotZeroU64 { target, .. } + | MicroOp::JumpGreaterEqualU64Imm { target, .. } + | MicroOp::JumpLessU64Imm { target, .. } + | MicroOp::JumpLessU64 { target, .. } + | MicroOp::JumpGreaterEqualU64 { target, .. } + | MicroOp::JumpNotEqualU64 { target, .. } => target.0 = resolved, _ => unreachable!(), } } diff --git a/third_party/move/mono-move/specializer/src/stackless_exec_ir/display.rs b/third_party/move/mono-move/specializer/src/stackless_exec_ir/display.rs index 2ae084510ff..6a597a3782a 100644 --- a/third_party/move/mono-move/specializer/src/stackless_exec_ir/display.rs +++ b/third_party/move/mono-move/specializer/src/stackless_exec_ir/display.rs @@ -4,7 +4,7 @@ //! Human-readable display for the stackless execution IR. //! Resolves pool indices using the CompiledModule for readable output. -use super::{BinaryOp, FunctionIR, ImmValue, Instr, ModuleIR, Slot, UnaryOp}; +use super::{BinaryOp, CmpOp, FunctionIR, ImmValue, Instr, ModuleIR, Slot, UnaryOp}; use move_binary_format::{ access::ModuleAccess, file_format::{ @@ -769,6 +769,22 @@ fn display_instr( Instr::Branch(l) => write!(f, "branch L{}", l.0), Instr::BrTrue(l, c) => write!(f, "br_true L{}, {}", l.0, slot_name(*c)), Instr::BrFalse(l, c) => write!(f, "br_false L{}, {}", l.0, slot_name(*c)), + Instr::BrCmp(l, op, lhs, rhs) => write!( + f, + "br_{} L{}, {}, {}", + cmp_op_name(op), + l.0, + slot_name(*lhs), + slot_name(*rhs) + ), + Instr::BrCmpImm(l, op, src, imm) => write!( + f, + "br_{} L{}, {}, {}", + cmp_op_name(op), + l.0, + slot_name(*src), + imm_value(imm) + ), Instr::Ret(rs) => write!(f, "ret {}", slot_names(rs)), Instr::Abort(c) => write!(f, "abort {}", slot_name(*c)), Instr::AbortMsg(c, m) => write!(f, "abort_msg {}, {}", slot_name(*c), slot_name(*m)), @@ -807,17 +823,23 @@ fn binary_op_name(op: &BinaryOp) -> &'static str { BinaryOp::Xor => "xor", BinaryOp::Shl => "shl", BinaryOp::Shr => "shr", - BinaryOp::Lt => "lt", - BinaryOp::Gt => "gt", - BinaryOp::Le => "le", - BinaryOp::Ge => "ge", - BinaryOp::Eq => "eq", - BinaryOp::Neq => "neq", + BinaryOp::Cmp(cmp) => cmp_op_name(cmp), BinaryOp::Or => "or", BinaryOp::And => "and", } } +fn cmp_op_name(op: &CmpOp) -> &'static str { + match op { + CmpOp::Lt => "lt", + CmpOp::Gt => "gt", + CmpOp::Le => "le", + CmpOp::Ge => "ge", + CmpOp::Eq => "eq", + CmpOp::Neq => "neq", + } +} + fn imm_value(imm: &ImmValue) -> String { match imm { ImmValue::Bool(true) => "#true".to_string(), diff --git a/third_party/move/mono-move/specializer/src/stackless_exec_ir/mod.rs b/third_party/move/mono-move/specializer/src/stackless_exec_ir/mod.rs index f493665519d..cba5e633771 100644 --- a/third_party/move/mono-move/specializer/src/stackless_exec_ir/mod.rs +++ b/third_party/move/mono-move/specializer/src/stackless_exec_ir/mod.rs @@ -63,7 +63,7 @@ impl Slot { pub struct Label(pub u16); /// Unary operations. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum UnaryOp { CastU8, CastU16, @@ -82,8 +82,33 @@ pub enum UnaryOp { FreezeRef, } +/// Comparison operations that produce a boolean result. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum CmpOp { + Lt, + Gt, + Le, + Ge, + Eq, + Neq, +} + +impl CmpOp { + /// Return the logically negated comparison. + pub fn negate(self) -> Self { + match self { + CmpOp::Lt => CmpOp::Ge, + CmpOp::Ge => CmpOp::Lt, + CmpOp::Gt => CmpOp::Le, + CmpOp::Le => CmpOp::Gt, + CmpOp::Eq => CmpOp::Neq, + CmpOp::Neq => CmpOp::Eq, + } + } +} + /// Binary operations. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum BinaryOp { Add, Sub, @@ -95,18 +120,13 @@ pub enum BinaryOp { Xor, Shl, Shr, - Lt, - Gt, - Le, - Ge, - Eq, - Neq, + Cmp(CmpOp), Or, And, } /// Immediate values for `BinaryOpImm`. Restricted to small types. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum ImmValue { Bool(bool), U8(u8), @@ -230,6 +250,10 @@ pub enum Instr { Branch(Label), BrTrue(Label, Slot), BrFalse(Label, Slot), + /// `BrCmp(target, op, lhs, rhs)` — branch to `target` if `op(lhs, rhs)` is true. + BrCmp(Label, CmpOp, Slot, Slot), + /// `BrCmpImm(target, op, src, imm)` — branch to `target` if `op(src, imm)` is true. + BrCmpImm(Label, CmpOp, Slot, ImmValue), Ret(Vec), Abort(Slot), AbortMsg(Slot, Slot), diff --git a/third_party/move/mono-move/specializer/tests/test_cases/masm/arg_regs.exp b/third_party/move/mono-move/specializer/tests/test_cases/masm/arg_regs.exp index 96935282e29..b029454e368 100644 --- a/third_party/move/mono-move/specializer/tests/test_cases/masm/arg_regs.exp +++ b/third_party/move/mono-move/specializer/tests/test_cases/masm/arg_regs.exp @@ -165,23 +165,21 @@ fun width_from_max(): u64 { } fun call_in_loop(r0): u64 { - slots: params(1), locals(1), temps(1), xfer(1) + slots: params(1), locals(1), temps(0), xfer(1) r0: u64 r1: u64 - r2: bool code: L3: 0: r1 := ld_u64 0 L2: - 1: r2 := gt r0, #0 - 2: br_false L0, r2 + 1: br_le L0, r0, #0 L1: - 3: [x0] := call identity, [r0] - 4: r1 := add x0, r1 - 5: r0 := sub r0, #1 - 6: branch L2 + 2: [x0] := call identity, [r0] + 3: r1 := add x0, r1 + 4: r0 := sub r0, #1 + 5: branch L2 L0: - 7: ret [r1] + 6: ret [r1] } fun mixed_at_callsite(r0): u64 { @@ -373,7 +371,19 @@ fun width_from_max() { 8: Return } -fun call_in_loop(): skipped (lowering: BinaryOpImm Gt for u64-sized type not yet lowered) +fun call_in_loop() { + frame_data_size: 16 + code: + 0: StoreImm8 [8] <- #0 + 1: JumpLessU64Imm @7 [0] < #1 + 2: Move8 [40] <- [0] + 3: CallFunc #0 + 4: AddU64 [8] <- [40] + [8] + 5: SubU64Imm [0] <- [0] - #1 + 6: Jump @1 + 7: Move8 [0] <- [8] + 8: Return +} fun mixed_at_callsite() { frame_data_size: 8 diff --git a/third_party/move/mono-move/specializer/tests/test_cases/masm/control_flow.exp b/third_party/move/mono-move/specializer/tests/test_cases/masm/control_flow.exp index 61e56a01ff0..bbfd89e7473 100644 --- a/third_party/move/mono-move/specializer/tests/test_cases/masm/control_flow.exp +++ b/third_party/move/mono-move/specializer/tests/test_cases/masm/control_flow.exp @@ -1,18 +1,16 @@ === Module 0x66::test === fun loop_down(r0) { - slots: params(1), locals(0), temps(1) + slots: params(1), locals(0), temps(0) r0: u64 - r1: bool code: L2: - 0: r1 := gt r0, #0 - 1: br_false L0, r1 + 0: br_le L0, r0, #0 L1: - 2: r0 := sub r0, #1 - 3: branch L2 + 1: r0 := sub r0, #1 + 2: branch L2 L0: - 4: ret [] + 3: ret [] } fun conditional(r0): u64 { @@ -33,7 +31,14 @@ fun conditional(r0): u64 { === micro-ops === === Module 0x66::test === -fun loop_down(): skipped (lowering: BinaryOpImm Gt for u64-sized type not yet lowered) +fun loop_down() { + frame_data_size: 8 + code: + 0: JumpLessU64Imm @3 [0] < #1 + 1: SubU64Imm [0] <- [0] - #1 + 2: Jump @0 + 3: Return +} fun conditional() { frame_data_size: 16 diff --git a/third_party/move/mono-move/specializer/tests/test_cases/masm/vid_type_reset.exp b/third_party/move/mono-move/specializer/tests/test_cases/masm/vid_type_reset.exp index aee74bcc78a..4d8d57b5f1d 100644 --- a/third_party/move/mono-move/specializer/tests/test_cases/masm/vid_type_reset.exp +++ b/third_party/move/mono-move/specializer/tests/test_cases/masm/vid_type_reset.exp @@ -1,22 +1,28 @@ === Module 0x66::test === fun vid_type_mismatch(r0): u64 { - slots: params(1), locals(0), temps(2) + slots: params(1), locals(0), temps(1) r0: u64 - r1: bool - r2: u64 + r1: u64 code: L2: - 0: r1 := gt r0, #10 - 1: br_false L0, r1 + 0: br_le L0, r0, #10 L1: - 2: ret [r0] + 1: ret [r0] L0: - 3: r2 := add r0, #1 - 4: ret [r2] + 2: r1 := add r0, #1 + 3: ret [r1] } === micro-ops === === Module 0x66::test === -fun vid_type_mismatch(): skipped (lowering: BinaryOpImm Gt for u64-sized type not yet lowered) +fun vid_type_mismatch() { + frame_data_size: 16 + code: + 0: JumpLessU64Imm @2 [0] < #11 + 1: Return + 2: AddU64Imm [8] <- [0] + #1 + 3: Move8 [0] <- [8] + 4: Return +} diff --git a/third_party/move/mono-move/specializer/tests/test_cases/move/basic.exp b/third_party/move/mono-move/specializer/tests/test_cases/move/basic.exp index f36bd1a7bf9..528fb86450f 100644 --- a/third_party/move/mono-move/specializer/tests/test_cases/move/basic.exp +++ b/third_party/move/mono-move/specializer/tests/test_cases/move/basic.exp @@ -75,18 +75,16 @@ fun make_point(r0, r1): Point { } fun max(r0, r1): u64 { - slots: params(2), locals(0), temps(1) + slots: params(2), locals(0), temps(0) r0: u64 r1: u64 - r2: bool code: L2: - 0: r2 := gt r0, r1 - 1: br_false L0, r2 + 0: br_le L0, r0, r1 L1: - 2: ret [r0] + 1: ret [r0] L0: - 3: ret [r1] + 2: ret [r1] } === micro-ops === @@ -104,4 +102,11 @@ fun get_x(): skipped (lowering: instruction ReadField(Home(1), FieldHandleIndex( fun make_point(): skipped (not all types are concrete) -fun max(): skipped (lowering: BinaryOp Gt for u64-sized type not yet lowered) +fun max() { + frame_data_size: 16 + code: + 0: JumpGreaterEqualU64 @2 [8] >= [0] + 1: Return + 2: Move8 [0] <- [8] + 3: Return +} diff --git a/third_party/move/mono-move/specializer/tests/test_cases/move/fibonacci.exp b/third_party/move/mono-move/specializer/tests/test_cases/move/fibonacci.exp index af8569b1134..fc4a6948b71 100644 --- a/third_party/move/mono-move/specializer/tests/test_cases/move/fibonacci.exp +++ b/third_party/move/mono-move/specializer/tests/test_cases/move/fibonacci.exp @@ -28,39 +28,37 @@ l0: copy_loc l0 === Module 0x42::fibonacci === fun fib(r0): u64 { - slots: params(1), locals(0), temps(2), xfer(1) + slots: params(1), locals(0), temps(1), xfer(1) r0: u64 - r1: bool - r2: u64 + r1: u64 code: L2: - 0: r1 := le r0, #1 - 1: br_false L0, r1 + 0: br_gt L0, r0, #1 L1: - 2: ret [r0] + 1: ret [r0] L0: - 3: x0 := sub r0, #1 - 4: [r2] := call fib, [x0] - 5: x0 := sub r0, #2 - 6: [x0] := call fib, [x0] - 7: r2 := add r2, x0 - 8: ret [r2] + 2: x0 := sub r0, #1 + 3: [r1] := call fib, [x0] + 4: x0 := sub r0, #2 + 5: [x0] := call fib, [x0] + 6: r1 := add r1, x0 + 7: ret [r1] } === micro-ops === === Module 0x42::fibonacci === fun fib() { - frame_data_size: 24 + frame_data_size: 16 code: 0: JumpGreaterEqualU64Imm @2 [0] >= #2 1: Return - 2: SubU64Imm [48] <- [0] - #1 + 2: SubU64Imm [40] <- [0] - #1 3: CallFunc #0 - 4: Move8 [16] <- [48] - 5: SubU64Imm [48] <- [0] - #2 + 4: Move8 [8] <- [40] + 5: SubU64Imm [40] <- [0] - #2 6: CallFunc #0 - 7: AddU64 [16] <- [16] + [48] - 8: Move8 [0] <- [16] + 7: AddU64 [8] <- [8] + [40] + 8: Move8 [0] <- [8] 9: Return } From 2f56ed63d42c7cbc8cb6f5db12c1a5a9d6713749 Mon Sep 17 00:00:00 2001 From: Vineeth Kashyap Date: Tue, 14 Apr 2026 17:09:28 -0400 Subject: [PATCH 2/2] address review comments --- .../mono-move/core/src/instruction/gas.rs | 14 +++++++++ .../mono-move/core/src/instruction/mod.rs | 24 +++++++++++++++ .../move/mono-move/runtime/src/interpreter.rs | 18 ++++++++++++ .../move/mono-move/runtime/src/verifier.rs | 18 ++++++++++++ .../specializer/src/lower/translate.rs | 29 ++++++++++++++----- .../tests/test_cases/masm/arg_regs.exp | 2 +- .../tests/test_cases/masm/control_flow.exp | 2 +- .../tests/test_cases/masm/vid_type_reset.exp | 2 +- .../tests/test_cases/move/fibonacci.exp | 2 +- 9 files changed, 99 insertions(+), 12 deletions(-) diff --git a/third_party/move/mono-move/core/src/instruction/gas.rs b/third_party/move/mono-move/core/src/instruction/gas.rs index c6bacc566f3..2069ea89438 100644 --- a/third_party/move/mono-move/core/src/instruction/gas.rs +++ b/third_party/move/mono-move/core/src/instruction/gas.rs @@ -31,6 +31,8 @@ impl HasCfgInfo for MicroOp { | MicroOp::JumpNotZeroU64 { target, .. } | MicroOp::JumpGreaterEqualU64Imm { target, .. } | MicroOp::JumpLessU64Imm { target, .. } + | MicroOp::JumpGreaterU64Imm { target, .. } + | MicroOp::JumpLessEqualU64Imm { target, .. } | MicroOp::JumpLessU64 { target, .. } | MicroOp::JumpGreaterEqualU64 { target, .. } | MicroOp::JumpNotEqualU64 { target, .. } => Some(target.0 as usize), @@ -101,6 +103,16 @@ impl RemapTargets for MicroOp { src, imm, }, + MicroOp::JumpGreaterU64Imm { target, src, imm } => MicroOp::JumpGreaterU64Imm { + target: co(target), + src, + imm, + }, + MicroOp::JumpLessEqualU64Imm { target, src, imm } => MicroOp::JumpLessEqualU64Imm { + target: co(target), + src, + imm, + }, MicroOp::JumpGreaterEqualU64 { target, lhs, rhs } => MicroOp::JumpGreaterEqualU64 { target: co(target), lhs, @@ -181,6 +193,8 @@ impl GasSchedule for MicroOpGasSchedule { MicroOp::JumpNotZeroU64 { .. } | MicroOp::JumpGreaterEqualU64Imm { .. } | MicroOp::JumpLessU64Imm { .. } + | MicroOp::JumpGreaterU64Imm { .. } + | MicroOp::JumpLessEqualU64Imm { .. } | MicroOp::JumpLessU64 { .. } | MicroOp::JumpGreaterEqualU64 { .. } | MicroOp::JumpNotEqualU64 { .. } => 3, diff --git a/third_party/move/mono-move/core/src/instruction/mod.rs b/third_party/move/mono-move/core/src/instruction/mod.rs index 073ca9bc263..d8dd0b7f6a7 100644 --- a/third_party/move/mono-move/core/src/instruction/mod.rs +++ b/third_party/move/mono-move/core/src/instruction/mod.rs @@ -317,6 +317,20 @@ pub enum MicroOp { imm: u64, }, + /// Jump to `target` if the u64 at `src` is **>** `imm`. + JumpGreaterU64Imm { + target: CodeOffset, + src: FrameOffset, + imm: u64, + }, + + /// Jump to `target` if the u64 at `src` is **<=** `imm`. + JumpLessEqualU64Imm { + target: CodeOffset, + src: FrameOffset, + imm: u64, + }, + /// Jump to `target` if u64 at `lhs` < u64 at `rhs`. JumpLessU64 { target: CodeOffset, @@ -633,6 +647,16 @@ impl fmt::Display for MicroOp { target.0, src.0, imm ) }, + MicroOp::JumpGreaterU64Imm { target, src, imm } => { + write!(f, "JumpGreaterU64Imm @{} [{}] > #{}", target.0, src.0, imm) + }, + MicroOp::JumpLessEqualU64Imm { target, src, imm } => { + write!( + f, + "JumpLessEqualU64Imm @{} [{}] <= #{}", + target.0, src.0, imm + ) + }, MicroOp::JumpLessU64 { target, lhs, rhs } => { write!(f, "JumpLessU64 @{} [{}] < [{}]", target.0, lhs.0, rhs.0) }, diff --git a/third_party/move/mono-move/runtime/src/interpreter.rs b/third_party/move/mono-move/runtime/src/interpreter.rs index 6ac9fa75ebe..62b0c39f117 100644 --- a/third_party/move/mono-move/runtime/src/interpreter.rs +++ b/third_party/move/mono-move/runtime/src/interpreter.rs @@ -244,6 +244,24 @@ impl InterpreterContext<'_, G> { return Ok(StepResult::Continue); }, + MicroOp::JumpGreaterU64Imm { target, src, imm } => { + self.pc = if read_u64(fp, src) > imm { + target.into() + } else { + self.pc + 1 + }; + return Ok(StepResult::Continue); + }, + + MicroOp::JumpLessEqualU64Imm { target, src, imm } => { + self.pc = if read_u64(fp, src) <= imm { + target.into() + } else { + self.pc + 1 + }; + return Ok(StepResult::Continue); + }, + MicroOp::JumpLessU64 { target, lhs, rhs } => { self.pc = if read_u64(fp, lhs) < read_u64(fp, rhs) { target.into() diff --git a/third_party/move/mono-move/runtime/src/verifier.rs b/third_party/move/mono-move/runtime/src/verifier.rs index 12aec061220..3ad61946701 100644 --- a/third_party/move/mono-move/runtime/src/verifier.rs +++ b/third_party/move/mono-move/runtime/src/verifier.rs @@ -243,6 +243,24 @@ impl FunctionVerifier<'_> { self.check_jump(pc, target); }, + MicroOp::JumpGreaterU64Imm { + target, + src, + imm: _, + } => { + self.check_frame_access_8(pc, src); + self.check_jump(pc, target); + }, + + MicroOp::JumpLessEqualU64Imm { + target, + src, + imm: _, + } => { + self.check_frame_access_8(pc, src); + self.check_jump(pc, target); + }, + MicroOp::JumpLessU64 { target, lhs, rhs } | MicroOp::JumpGreaterEqualU64 { target, lhs, rhs } | MicroOp::JumpNotEqualU64 { target, lhs, rhs } => { diff --git a/third_party/move/mono-move/specializer/src/lower/translate.rs b/third_party/move/mono-move/specializer/src/lower/translate.rs index ff3e32ade17..5eda2127a0e 100644 --- a/third_party/move/mono-move/specializer/src/lower/translate.rs +++ b/third_party/move/mono-move/specializer/src/lower/translate.rs @@ -273,13 +273,24 @@ impl<'a> LoweringState<'a> { let s = self.slot(*cond); let idx = self.ops.len(); self.branch_fixups.push(idx); + // [TODO]: we are representing booleans as 0/1 in u64 slots here. + // This needs to be updated with a more compact boolean representation. self.emit(MicroOp::JumpNotZeroU64 { target: CodeOffset(encode_label(*l)), src: FrameOffset(s.offset), }); }, - Instr::BrFalse(Label(_l), _cond) => { - bail!("standalone BrFalse not yet lowered (expected compare+branch fusion)"); + Instr::BrFalse(Label(l), cond) => { + let s = self.slot(*cond); + let idx = self.ops.len(); + self.branch_fixups.push(idx); + // [TODO]: we are representing booleans as 0/1 in u64 slots here. + // This needs to be updated with a more compact boolean representation. + self.emit(MicroOp::JumpLessU64Imm { + target: CodeOffset(encode_label(*l)), + src: FrameOffset(s.offset), + imm: 1, + }); }, // --- Fused compare+branch --- @@ -344,17 +355,15 @@ impl<'a> LoweringState<'a> { src: FrameOffset(s.offset), imm: v, }), - // x > c ↔ x >= c+1 - CmpOp::Gt => self.emit(MicroOp::JumpGreaterEqualU64Imm { + CmpOp::Gt => self.emit(MicroOp::JumpGreaterU64Imm { target: CodeOffset(encode_label(*l)), src: FrameOffset(s.offset), - imm: v + 1, + imm: v, }), - // x <= c ↔ x < c+1 - CmpOp::Le => self.emit(MicroOp::JumpLessU64Imm { + CmpOp::Le => self.emit(MicroOp::JumpLessEqualU64Imm { target: CodeOffset(encode_label(*l)), src: FrameOffset(s.offset), - imm: v + 1, + imm: v, }), CmpOp::Eq | CmpOp::Neq => { bail!("BrCmpImm {:?} for u64-sized type not yet lowered", op) @@ -446,6 +455,8 @@ impl<'a> LoweringState<'a> { | MicroOp::JumpNotZeroU64 { target, .. } | MicroOp::JumpGreaterEqualU64Imm { target, .. } | MicroOp::JumpLessU64Imm { target, .. } + | MicroOp::JumpGreaterU64Imm { target, .. } + | MicroOp::JumpLessEqualU64Imm { target, .. } | MicroOp::JumpLessU64 { target, .. } | MicroOp::JumpGreaterEqualU64 { target, .. } | MicroOp::JumpNotEqualU64 { target, .. } => target.0, @@ -462,6 +473,8 @@ impl<'a> LoweringState<'a> { | MicroOp::JumpNotZeroU64 { target, .. } | MicroOp::JumpGreaterEqualU64Imm { target, .. } | MicroOp::JumpLessU64Imm { target, .. } + | MicroOp::JumpGreaterU64Imm { target, .. } + | MicroOp::JumpLessEqualU64Imm { target, .. } | MicroOp::JumpLessU64 { target, .. } | MicroOp::JumpGreaterEqualU64 { target, .. } | MicroOp::JumpNotEqualU64 { target, .. } => target.0 = resolved, diff --git a/third_party/move/mono-move/specializer/tests/test_cases/masm/arg_regs.exp b/third_party/move/mono-move/specializer/tests/test_cases/masm/arg_regs.exp index b029454e368..110d47b0187 100644 --- a/third_party/move/mono-move/specializer/tests/test_cases/masm/arg_regs.exp +++ b/third_party/move/mono-move/specializer/tests/test_cases/masm/arg_regs.exp @@ -375,7 +375,7 @@ fun call_in_loop() { frame_data_size: 16 code: 0: StoreImm8 [8] <- #0 - 1: JumpLessU64Imm @7 [0] < #1 + 1: JumpLessEqualU64Imm @7 [0] <= #0 2: Move8 [40] <- [0] 3: CallFunc #0 4: AddU64 [8] <- [40] + [8] diff --git a/third_party/move/mono-move/specializer/tests/test_cases/masm/control_flow.exp b/third_party/move/mono-move/specializer/tests/test_cases/masm/control_flow.exp index bbfd89e7473..6de731a2b21 100644 --- a/third_party/move/mono-move/specializer/tests/test_cases/masm/control_flow.exp +++ b/third_party/move/mono-move/specializer/tests/test_cases/masm/control_flow.exp @@ -34,7 +34,7 @@ fun conditional(r0): u64 { fun loop_down() { frame_data_size: 8 code: - 0: JumpLessU64Imm @3 [0] < #1 + 0: JumpLessEqualU64Imm @3 [0] <= #0 1: SubU64Imm [0] <- [0] - #1 2: Jump @0 3: Return diff --git a/third_party/move/mono-move/specializer/tests/test_cases/masm/vid_type_reset.exp b/third_party/move/mono-move/specializer/tests/test_cases/masm/vid_type_reset.exp index 4d8d57b5f1d..078ea826d51 100644 --- a/third_party/move/mono-move/specializer/tests/test_cases/masm/vid_type_reset.exp +++ b/third_party/move/mono-move/specializer/tests/test_cases/masm/vid_type_reset.exp @@ -20,7 +20,7 @@ fun vid_type_mismatch(r0): u64 { fun vid_type_mismatch() { frame_data_size: 16 code: - 0: JumpLessU64Imm @2 [0] < #11 + 0: JumpLessEqualU64Imm @2 [0] <= #10 1: Return 2: AddU64Imm [8] <- [0] + #1 3: Move8 [0] <- [8] diff --git a/third_party/move/mono-move/specializer/tests/test_cases/move/fibonacci.exp b/third_party/move/mono-move/specializer/tests/test_cases/move/fibonacci.exp index fc4a6948b71..4fdaa1d2cef 100644 --- a/third_party/move/mono-move/specializer/tests/test_cases/move/fibonacci.exp +++ b/third_party/move/mono-move/specializer/tests/test_cases/move/fibonacci.exp @@ -51,7 +51,7 @@ fun fib(r0): u64 { fun fib() { frame_data_size: 16 code: - 0: JumpGreaterEqualU64Imm @2 [0] >= #2 + 0: JumpGreaterU64Imm @2 [0] > #1 1: Return 2: SubU64Imm [40] <- [0] - #1 3: CallFunc #0