From b766d174b1d7f9ff7135926f1ed6264b93cefe17 Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Fri, 22 May 2026 00:21:59 -0400 Subject: [PATCH 1/5] ZJIT: Remove defunct LIR LiveReg instruction We don't use this anymore since we switched to global linear scan register allocation. --- zjit/src/backend/arm64/mod.rs | 1 - zjit/src/backend/lir.rs | 15 --------------- zjit/src/backend/x86_64/mod.rs | 1 - 3 files changed, 17 deletions(-) diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index abe439cb1233df..2af243e082c8c5 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -1587,7 +1587,6 @@ impl Assembler { Insn::CSelGE { truthy, falsy, out } => { csel(cb, out.into(), truthy.into(), falsy.into(), Condition::GE); } - Insn::LiveReg { .. } => (), // just a reg alloc signal, no code }; insn_idx += 1; diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index 0ef79b215eddc2..5d0c9b1f2dad06 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -787,9 +787,6 @@ pub enum Insn { // Load effective address Lea { opnd: Opnd, out: Opnd }, - /// Take a specific register. Signal the register allocator to not use it. - LiveReg { opnd: Opnd, out: Opnd }, - // A low-level instruction that loads a value into a register. Load { opnd: Opnd, out: Opnd }, @@ -935,7 +932,6 @@ impl Insn { Insn::Label(_) => "Label", Insn::LeaJumpTarget { .. } => "LeaJumpTarget", Insn::Lea { .. } => "Lea", - Insn::LiveReg { .. } => "LiveReg", Insn::Load { .. } => "Load", Insn::LoadInto { .. } => "LoadInto", Insn::LoadSExt { .. } => "LoadSExt", @@ -974,7 +970,6 @@ impl Insn { Insn::CSelZ { out, .. } | Insn::Lea { out, .. } | Insn::LeaJumpTarget { out, .. } | - Insn::LiveReg { out, .. } | Insn::Load { out, .. } | Insn::LoadSExt { out, .. } | Insn::LShift { out, .. } | @@ -1007,7 +1002,6 @@ impl Insn { Insn::CSelZ { out, .. } | Insn::Lea { out, .. } | Insn::LeaJumpTarget { out, .. } | - Insn::LiveReg { out, .. } | Insn::Load { out, .. } | Insn::LoadSExt { out, .. } | Insn::LShift { out, .. } | @@ -1198,7 +1192,6 @@ impl<'a> Iterator for InsnOpndIterator<'a> { Insn::CRet(opnd) | Insn::JmpOpnd(opnd) | Insn::Lea { opnd, .. } | - Insn::LiveReg { opnd, .. } | Insn::Load { opnd, .. } | Insn::LoadSExt { opnd, .. } | Insn::Not { opnd, .. } => { @@ -1378,7 +1371,6 @@ impl<'a> InsnOpndMutIterator<'a> { Insn::CRet(opnd) | Insn::JmpOpnd(opnd) | Insn::Lea { opnd, .. } | - Insn::LiveReg { opnd, .. } | Insn::Load { opnd, .. } | Insn::LoadSExt { opnd, .. } | Insn::Not { opnd, .. } => { @@ -3660,13 +3652,6 @@ impl Assembler { out } - #[must_use] - pub fn live_reg_opnd(&mut self, opnd: Opnd) -> Opnd { - let out = self.new_vreg(Opnd::match_num_bits(&[opnd])); - self.push_insn(Insn::LiveReg { opnd, out }); - out - } - #[must_use] pub fn load(&mut self, opnd: Opnd) -> Opnd { let out = self.new_vreg(Opnd::match_num_bits(&[opnd])); diff --git a/zjit/src/backend/x86_64/mod.rs b/zjit/src/backend/x86_64/mod.rs index 80abd15a6b6d4c..bd1e7d7467cac8 100644 --- a/zjit/src/backend/x86_64/mod.rs +++ b/zjit/src/backend/x86_64/mod.rs @@ -1109,7 +1109,6 @@ impl Assembler { Insn::CSelGE { truthy, falsy, out } => { emit_csel(cb, *truthy, *falsy, *out, cmovge, cmovl); } - Insn::LiveReg { .. } => (), // just a reg alloc signal, no code }; insn_idx += 1; From 8b33519a5fd3ba8055fdc3a0a36f6859d1c1f8ce Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Fri, 22 May 2026 00:44:19 -0400 Subject: [PATCH 2/5] ZJIT: Add for_each_operand and for_each_operand_mut to LIR Replace the operand iterator structs with the simpler, shorter visit-closure pattern that we also have in HIR. --- zjit/src/backend/arm64/mod.rs | 5 +- zjit/src/backend/lir.rs | 573 ++++++++++----------------------- zjit/src/backend/x86_64/mod.rs | 5 +- 3 files changed, 177 insertions(+), 406 deletions(-) diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index 2af243e082c8c5..32ba1e3de00b32 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -409,9 +409,8 @@ impl Assembler { // if they were just unsigned integer. let is_load = matches!(insn, Insn::Load { .. } | Insn::LoadInto { .. }); let is_jump = insn.is_jump(); - let mut opnd_iter = insn.opnd_iter_mut(); - while let Some(opnd) = opnd_iter.next() { + insn.for_each_operand_mut(|opnd| { if let Opnd::Value(value) = opnd { if value.special_const_p() { *opnd = Opnd::UImm(value.as_u64()); @@ -419,7 +418,7 @@ impl Assembler { *opnd = asm.load(*opnd); } }; - } + }); // We are replacing instructions here so we know they are already // being used. It is okay not to use their output here. diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index 5d0c9b1f2dad06..a3ee8f51f1a91d 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -846,17 +846,140 @@ pub enum Insn { Xor { left: Opnd, right: Opnd, out: Opnd } } +macro_rules! target_for_each_operand_impl { + ($self:expr, $visit_many:ident) => { + match $self { + Target::SideExit { exit: SideExit { stack, locals, .. }, .. } => { + visit_many!(stack); + visit_many!(locals); + } + Target::Block(edge) => { + visit_many!(edge.args); + } + Target::CodePtr(_) | Target::Label(_) => {} + } + } +} + +/// Macro that enumerates all operands of an Insn, dispatching to caller-provided `$visit_one` +/// macro for a single `Opnd` field and `$visit_many` macro for a slice/`Vec` of `Opnd`s. Used by +/// both `for_each_operand` and `for_each_operand_mut`. +macro_rules! for_each_operand_impl { + ($self:expr, $visit_one:ident, $visit_many:ident $(, $const:expr)?) => { + match $self { + Insn::Jbe(target) | + Insn::Jb(target) | + Insn::Je(target) | + Insn::Jl(target) | + Insn::Jg(target) | + Insn::Jge(target) | + Insn::Jmp(target) | + Insn::Jne(target) | + Insn::Jnz(target) | + Insn::Jo(target) | + Insn::JoMul(target) | + Insn::Jz(target) | + Insn::Label(target) | + Insn::LeaJumpTarget { target, .. } | + Insn::PatchPoint { target, .. } => { + target_for_each_operand_impl!(target, $visit_many); + } + Insn::Joz(opnd, target) | + Insn::Jonz(opnd, target) => { + visit_one!(opnd); + target_for_each_operand_impl!(target, $visit_many); + } + + Insn::BakeString(_) | + Insn::Breakpoint | Insn::Abort | + Insn::Comment(_) | + Insn::CPop { .. } | + Insn::PadPatchPoint | + Insn::PosMarker(_) => {}, + + Insn::CPopInto(opnd) | + Insn::CPush(opnd) | + Insn::CRet(opnd) | + Insn::JmpOpnd(opnd) | + Insn::Lea { opnd, .. } | + Insn::Load { opnd, .. } | + Insn::LoadSExt { opnd, .. } | + Insn::Not { opnd, .. } => { + visit_one!(opnd); + } + Insn::Add { left: opnd0, right: opnd1, .. } | + Insn::And { left: opnd0, right: opnd1, .. } | + Insn::CPushPair(opnd0, opnd1) | + Insn::CPopPairInto(opnd0, opnd1) | + Insn::Cmp { left: opnd0, right: opnd1 } | + Insn::CSelE { truthy: opnd0, falsy: opnd1, .. } | + Insn::CSelG { truthy: opnd0, falsy: opnd1, .. } | + Insn::CSelGE { truthy: opnd0, falsy: opnd1, .. } | + Insn::CSelL { truthy: opnd0, falsy: opnd1, .. } | + Insn::CSelLE { truthy: opnd0, falsy: opnd1, .. } | + Insn::CSelNE { truthy: opnd0, falsy: opnd1, .. } | + Insn::CSelNZ { truthy: opnd0, falsy: opnd1, .. } | + Insn::CSelZ { truthy: opnd0, falsy: opnd1, .. } | + Insn::IncrCounter { mem: opnd0, value: opnd1, .. } | + Insn::LoadInto { dest: opnd0, opnd: opnd1 } | + Insn::LShift { opnd: opnd0, shift: opnd1, .. } | + Insn::Mov { dest: opnd0, src: opnd1 } | + Insn::Or { left: opnd0, right: opnd1, .. } | + Insn::RShift { opnd: opnd0, shift: opnd1, .. } | + Insn::Store { dest: opnd0, src: opnd1 } | + Insn::Sub { left: opnd0, right: opnd1, .. } | + Insn::Mul { left: opnd0, right: opnd1, .. } | + Insn::Test { left: opnd0, right: opnd1 } | + Insn::URShift { opnd: opnd0, shift: opnd1, .. } | + Insn::Xor { left: opnd0, right: opnd1, .. } => { + visit_one!(opnd0); + visit_one!(opnd1); + } + Insn::CCall { opnds, .. } => { + visit_many!(opnds); + } + // only iterate over preserved in the const iterator + #[allow(unused_variables)] + Insn::FrameSetup { preserved, .. } | + Insn::FrameTeardown { preserved } => { + $( + visit_many!(preserved); + $const; + )? + } + } + } +} + impl Insn { - /// Create an iterator that will yield a non-mutable reference to each - /// operand in turn for this instruction. - pub(super) fn opnd_iter(&self) -> InsnOpndIterator<'_> { - InsnOpndIterator::new(self) + pub fn opnd_count(&self) -> usize { + let mut count = 0; + self.for_each_operand(|_| count += 1); + count } - /// Create an iterator that will yield a mutable reference to each operand - /// in turn for this instruction. - pub(super) fn opnd_iter_mut(&mut self) -> InsnOpndMutIterator<'_> { - InsnOpndMutIterator::new(self) + /// Call `f` on each operand (Opnd) of this instruction. + pub fn for_each_operand(&self, mut f: impl FnMut(Opnd)) { + macro_rules! visit_one { ($id:expr) => { f(*$id) }; } + macro_rules! visit_many { ($s:expr) => { for id in ($s).iter() { f(*id) } }; } + // Extra () is a throw-away parameter to avoid iterating over FrameSetup/FrameTeardown + // preserved in the mutable iterator. + for_each_operand_impl!(self, visit_one, visit_many, ()); + } + + /// Call `f` on a mutable reference to each operand (Opnd) of this instruction. + pub fn for_each_operand_mut(&mut self, mut f: impl FnMut(&mut Opnd)) { + macro_rules! visit_one { ($id:expr) => { f($id) }; } + macro_rules! visit_many { ($s:expr) => { for id in ($s).iter_mut() { f(id) } }; } + for_each_operand_impl!(self, visit_one, visit_many); + } + + /// Call `f` on each operand, short-circuiting on the first error. + pub fn try_for_each_operand(&self, mut f: impl FnMut(Opnd) -> Result<(), E>) -> Result<(), E> { + macro_rules! visit_one { ($id:expr) => { f(*$id)? }; } + macro_rules! visit_many { ($s:expr) => { for id in ($s).iter() { f(*id)? } }; } + for_each_operand_impl!(self, visit_one, visit_many, ()); + Ok(()) } /// Get a mutable reference to a Target if it exists. @@ -1081,372 +1204,19 @@ impl Insn { } } -/// An iterator that will yield a non-mutable reference to each operand in turn -/// for the given instruction. -pub(super) struct InsnOpndIterator<'a> { - insn: &'a Insn, - idx: usize, -} - -impl<'a> InsnOpndIterator<'a> { - fn new(insn: &'a Insn) -> Self { - Self { insn, idx: 0 } - } -} - -impl<'a> Iterator for InsnOpndIterator<'a> { - type Item = &'a Opnd; - - fn next(&mut self) -> Option { - match self.insn { - Insn::Jbe(target) | - Insn::Jb(target) | - Insn::Je(target) | - Insn::Jl(target) | - Insn::Jg(target) | - Insn::Jge(target) | - Insn::Jmp(target) | - Insn::Jne(target) | - Insn::Jnz(target) | - Insn::Jo(target) | - Insn::JoMul(target) | - Insn::Jz(target) | - Insn::Label(target) | - Insn::LeaJumpTarget { target, .. } | - Insn::PatchPoint { target, .. } => { - match target { - Target::SideExit { exit: SideExit { stack, locals, .. }, .. } => { - let stack_idx = self.idx; - if stack_idx < stack.len() { - let opnd = &stack[stack_idx]; - self.idx += 1; - return Some(opnd); - } - - let local_idx = self.idx - stack.len(); - if local_idx < locals.len() { - let opnd = &locals[local_idx]; - self.idx += 1; - return Some(opnd); - } - None - } - Target::Block(edge) => { - if self.idx < edge.args.len() { - let opnd = &edge.args[self.idx]; - self.idx += 1; - return Some(opnd); - } - None - } - _ => None - } - } - - Insn::Joz(opnd, target) | - Insn::Jonz(opnd, target) => { - if self.idx == 0 { - self.idx += 1; - return Some(opnd); - } - - match target { - Target::SideExit { exit: SideExit { stack, locals, .. }, .. } => { - let stack_idx = self.idx - 1; - if stack_idx < stack.len() { - let opnd = &stack[stack_idx]; - self.idx += 1; - return Some(opnd); - } - - let local_idx = stack_idx - stack.len(); - if local_idx < locals.len() { - let opnd = &locals[local_idx]; - self.idx += 1; - return Some(opnd); - } - None - } - Target::Block(edge) => { - let arg_idx = self.idx - 1; - if arg_idx < edge.args.len() { - let opnd = &edge.args[arg_idx]; - self.idx += 1; - return Some(opnd); - } - None - } - _ => None - } - } - - Insn::BakeString(_) | - Insn::Breakpoint | Insn::Abort | - Insn::Comment(_) | - Insn::CPop { .. } | - Insn::PadPatchPoint | - Insn::PosMarker(_) => None, - - Insn::CPopInto(opnd) | - Insn::CPush(opnd) | - Insn::CRet(opnd) | - Insn::JmpOpnd(opnd) | - Insn::Lea { opnd, .. } | - Insn::Load { opnd, .. } | - Insn::LoadSExt { opnd, .. } | - Insn::Not { opnd, .. } => { - match self.idx { - 0 => { - self.idx += 1; - Some(opnd) - }, - _ => None - } - }, - Insn::Add { left: opnd0, right: opnd1, .. } | - Insn::And { left: opnd0, right: opnd1, .. } | - Insn::CPushPair(opnd0, opnd1) | - Insn::CPopPairInto(opnd0, opnd1) | - Insn::Cmp { left: opnd0, right: opnd1 } | - Insn::CSelE { truthy: opnd0, falsy: opnd1, .. } | - Insn::CSelG { truthy: opnd0, falsy: opnd1, .. } | - Insn::CSelGE { truthy: opnd0, falsy: opnd1, .. } | - Insn::CSelL { truthy: opnd0, falsy: opnd1, .. } | - Insn::CSelLE { truthy: opnd0, falsy: opnd1, .. } | - Insn::CSelNE { truthy: opnd0, falsy: opnd1, .. } | - Insn::CSelNZ { truthy: opnd0, falsy: opnd1, .. } | - Insn::CSelZ { truthy: opnd0, falsy: opnd1, .. } | - Insn::IncrCounter { mem: opnd0, value: opnd1, .. } | - Insn::LoadInto { dest: opnd0, opnd: opnd1 } | - Insn::LShift { opnd: opnd0, shift: opnd1, .. } | - Insn::Mov { dest: opnd0, src: opnd1 } | - Insn::Or { left: opnd0, right: opnd1, .. } | - Insn::RShift { opnd: opnd0, shift: opnd1, .. } | - Insn::Store { dest: opnd0, src: opnd1 } | - Insn::Sub { left: opnd0, right: opnd1, .. } | - Insn::Mul { left: opnd0, right: opnd1, .. } | - Insn::Test { left: opnd0, right: opnd1 } | - Insn::URShift { opnd: opnd0, shift: opnd1, .. } | - Insn::Xor { left: opnd0, right: opnd1, .. } => { - match self.idx { - 0 => { - self.idx += 1; - Some(opnd0) - } - 1 => { - self.idx += 1; - Some(opnd1) - } - _ => None - } - }, - Insn::CCall { opnds, .. } => { - if self.idx < opnds.len() { - let opnd = &opnds[self.idx]; - self.idx += 1; - Some(opnd) - } else { - None - } - }, - Insn::FrameSetup { preserved, .. } | - Insn::FrameTeardown { preserved } => { - if self.idx < preserved.len() { - let opnd = &preserved[self.idx]; - self.idx += 1; - Some(opnd) - } else { - None - } - } - } - } -} - -/// An iterator that will yield each operand in turn for the given instruction. -pub(super) struct InsnOpndMutIterator<'a> { - insn: &'a mut Insn, - idx: usize, -} - -impl<'a> InsnOpndMutIterator<'a> { - fn new(insn: &'a mut Insn) -> Self { - Self { insn, idx: 0 } - } - - pub(super) fn next(&mut self) -> Option<&mut Opnd> { - match self.insn { - Insn::Jbe(target) | - Insn::Jb(target) | - Insn::Je(target) | - Insn::Jl(target) | - Insn::Jg(target) | - Insn::Jge(target) | - Insn::Jmp(target) | - Insn::Jne(target) | - Insn::Jnz(target) | - Insn::Jo(target) | - Insn::JoMul(target) | - Insn::Jz(target) | - Insn::Label(target) | - Insn::LeaJumpTarget { target, .. } | - Insn::PatchPoint { target, .. } => { - match target { - Target::SideExit { exit: SideExit { stack, locals, .. }, .. } => { - let stack_idx = self.idx; - if stack_idx < stack.len() { - let opnd = &mut stack[stack_idx]; - self.idx += 1; - return Some(opnd); - } - - let local_idx = self.idx - stack.len(); - if local_idx < locals.len() { - let opnd = &mut locals[local_idx]; - self.idx += 1; - return Some(opnd); - } - None - } - Target::Block(edge) => { - if self.idx < edge.args.len() { - let opnd = &mut edge.args[self.idx]; - self.idx += 1; - return Some(opnd); - } - None - } - _ => None - } - } - - Insn::Joz(opnd, target) | - Insn::Jonz(opnd, target) => { - if self.idx == 0 { - self.idx += 1; - return Some(opnd); - } - - match target { - Target::SideExit { exit: SideExit { stack, locals, .. }, .. } => { - let stack_idx = self.idx - 1; - if stack_idx < stack.len() { - let opnd = &mut stack[stack_idx]; - self.idx += 1; - return Some(opnd); - } - - let local_idx = stack_idx - stack.len(); - if local_idx < locals.len() { - let opnd = &mut locals[local_idx]; - self.idx += 1; - return Some(opnd); - } - None - } - Target::Block(edge) => { - let arg_idx = self.idx - 1; - if arg_idx < edge.args.len() { - let opnd = &mut edge.args[arg_idx]; - self.idx += 1; - return Some(opnd); - } - None - } - _ => None - } - } - - Insn::BakeString(_) | - Insn::Breakpoint | Insn::Abort | - Insn::Comment(_) | - Insn::CPop { .. } | - Insn::FrameSetup { .. } | - Insn::FrameTeardown { .. } | - Insn::PadPatchPoint | - Insn::PosMarker(_) => None, - - Insn::CPopInto(opnd) | - Insn::CPush(opnd) | - Insn::CRet(opnd) | - Insn::JmpOpnd(opnd) | - Insn::Lea { opnd, .. } | - Insn::Load { opnd, .. } | - Insn::LoadSExt { opnd, .. } | - Insn::Not { opnd, .. } => { - match self.idx { - 0 => { - self.idx += 1; - Some(opnd) - }, - _ => None - } - }, - Insn::Add { left: opnd0, right: opnd1, .. } | - Insn::And { left: opnd0, right: opnd1, .. } | - Insn::CPushPair(opnd0, opnd1) | - Insn::CPopPairInto(opnd0, opnd1) | - Insn::Cmp { left: opnd0, right: opnd1 } | - Insn::CSelE { truthy: opnd0, falsy: opnd1, .. } | - Insn::CSelG { truthy: opnd0, falsy: opnd1, .. } | - Insn::CSelGE { truthy: opnd0, falsy: opnd1, .. } | - Insn::CSelL { truthy: opnd0, falsy: opnd1, .. } | - Insn::CSelLE { truthy: opnd0, falsy: opnd1, .. } | - Insn::CSelNE { truthy: opnd0, falsy: opnd1, .. } | - Insn::CSelNZ { truthy: opnd0, falsy: opnd1, .. } | - Insn::CSelZ { truthy: opnd0, falsy: opnd1, .. } | - Insn::IncrCounter { mem: opnd0, value: opnd1, .. } | - Insn::LoadInto { dest: opnd0, opnd: opnd1 } | - Insn::LShift { opnd: opnd0, shift: opnd1, .. } | - Insn::Mov { dest: opnd0, src: opnd1 } | - Insn::Or { left: opnd0, right: opnd1, .. } | - Insn::RShift { opnd: opnd0, shift: opnd1, .. } | - Insn::Store { dest: opnd0, src: opnd1 } | - Insn::Sub { left: opnd0, right: opnd1, .. } | - Insn::Mul { left: opnd0, right: opnd1, .. } | - Insn::Test { left: opnd0, right: opnd1 } | - Insn::URShift { opnd: opnd0, shift: opnd1, .. } | - Insn::Xor { left: opnd0, right: opnd1, .. } => { - match self.idx { - 0 => { - self.idx += 1; - Some(opnd0) - } - 1 => { - self.idx += 1; - Some(opnd1) - } - _ => None - } - }, - Insn::CCall { opnds, .. } => { - if self.idx < opnds.len() { - let opnd = &mut opnds[self.idx]; - self.idx += 1; - Some(opnd) - } else { - None - } - }, - } - } -} - impl fmt::Debug for Insn { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { write!(fmt, "{}(", self.op())?; - // Print list of operands - let mut opnd_iter = self.opnd_iter(); if let Insn::FrameSetup { slot_count, .. } = self { write!(fmt, "{slot_count}")?; } - if let Some(first_opnd) = opnd_iter.next() { - write!(fmt, "{first_opnd:?}")?; - } - for opnd in opnd_iter { - write!(fmt, ", {opnd:?}")?; - } + // Print list of operands + let mut sep = ""; + self.for_each_operand(|opnd| { + write!(fmt, "{sep}{opnd:?}").unwrap(); + sep = ", "; + }); write!(fmt, ")")?; // Print text, target, and pos if they are present @@ -1932,10 +1702,9 @@ impl Assembler pub fn push_insn(&mut self, insn: Insn) { // If this Assembler should not accept scratch registers, assert no use of them. if !self.accept_scratch_reg { - let opnd_iter = insn.opnd_iter(); - for opnd in opnd_iter { - assert!(!Self::has_scratch_reg(*opnd), "should not use scratch register: {opnd:?}"); - } + insn.for_each_operand(|opnd| { + assert!(!Self::has_scratch_reg(opnd), "should not use scratch register: {opnd:?}"); + }); } self.idx += 1; @@ -2487,10 +2256,9 @@ impl Assembler fn rewrite_instructions(&mut self, assignments: &[Option]) { for block_id in self.block_order() { for insn in self.basic_blocks[block_id.0].insns.iter_mut() { - let mut iter = insn.opnd_iter_mut(); - while let Some(opnd) = iter.next() { + insn.for_each_operand_mut(|opnd| { Self::rewrite_opnd(opnd, assignments); - } + }); if let Some(out) = insn.out_opnd_mut() { Self::rewrite_opnd(out, assignments); } @@ -2908,12 +2676,12 @@ impl Assembler } // For all input operands that are VRegs (including memory base VRegs), add to gen set - for opnd in insn.opnd_iter() { + insn.for_each_operand(|opnd| { for idx in opnd.vreg_ids() { assert!(!kill_set.get(idx.0)); gen_set.insert(idx.0); } - } + }); } // Add block parameters to kill set @@ -2973,11 +2741,11 @@ impl Assembler } // For each VReg input (including memory base VRegs), add_range from block start to insn - for opnd in insn.opnd_iter() { + insn.for_each_operand(|opnd| { for idx in opnd.vreg_ids() { intervals[idx.0].add_range(block.from.0, insn_id.0); } - } + }); } } @@ -3198,14 +2966,12 @@ fn format_insn_compact(asm: &Assembler, insn: &Insn) -> String { } _ => {} } - } else if insn.opnd_iter().count() > 0 { - for (i, opnd) in insn.opnd_iter().enumerate() { - if i == 0 { - output.push_str(&format!(" {opnd}")); - } else { - output.push_str(&format!(", {opnd}")); - } - } + } else if insn.opnd_count() > 0 { + let mut sep = ""; + insn.for_each_operand(|opnd| { + output.push_str(&format!("{sep}{opnd}")); + sep = ", "; + }); } output @@ -3329,8 +3095,13 @@ impl fmt::Display for Assembler { } _ => {} } - } else if insn.opnd_iter().count() > 0 { - insn.opnd_iter().try_fold(" ", |prefix, opnd| write!(f, "{prefix}{opnd}").and(Ok(", ")))?; + } else if insn.opnd_count() > 0 { + let mut sep = " "; + insn.try_for_each_operand(|opnd| { + let result = write!(f, "{sep}{opnd}"); + sep = ", "; + result + })?; } write!(f, "\n")?; @@ -3838,25 +3609,27 @@ mod tests { } #[test] - fn test_opnd_iter() { + fn test_for_each_operand() { let insn = Insn::Add { left: Opnd::None, right: Opnd::None, out: Opnd::None }; - let mut opnd_iter = insn.opnd_iter(); - assert!(matches!(opnd_iter.next(), Some(Opnd::None))); - assert!(matches!(opnd_iter.next(), Some(Opnd::None))); - - assert!(opnd_iter.next().is_none()); + let mut result = vec![]; + insn.for_each_operand(|opnd| result.push(opnd)); + assert_eq!(result, vec![Opnd::None, Opnd::None]); } #[test] - fn test_opnd_iter_mut() { + fn test_for_each_operand_mut() { let mut insn = Insn::Add { left: Opnd::None, right: Opnd::None, out: Opnd::None }; - let mut opnd_iter = insn.opnd_iter_mut(); - assert!(matches!(opnd_iter.next(), Some(Opnd::None))); - assert!(matches!(opnd_iter.next(), Some(Opnd::None))); - - assert!(opnd_iter.next().is_none()); + let mut counter = 0; + insn.for_each_operand_mut(|opnd| { + *opnd = Opnd::Imm(counter); + counter += 1; + }); + assert!(matches!(insn, Insn::Add { left: Opnd::Imm(0), right: Opnd::Imm(1), out: Opnd::None })); + let mut result = vec![]; + insn.for_each_operand(|opnd| result.push(opnd)); + assert_eq!(result, vec![Opnd::Imm(0), Opnd::Imm(1)]); } #[test] diff --git a/zjit/src/backend/x86_64/mod.rs b/zjit/src/backend/x86_64/mod.rs index bd1e7d7467cac8..d3bf847ab22ae0 100644 --- a/zjit/src/backend/x86_64/mod.rs +++ b/zjit/src/backend/x86_64/mod.rs @@ -154,10 +154,9 @@ impl Assembler { while let Some((_index, mut insn)) = iterator.next(asm) { let is_load = matches!(insn, Insn::Load { .. } | Insn::LoadInto { .. }); let is_jump = insn.is_jump(); - let mut opnd_iter = insn.opnd_iter_mut(); if !is_jump { - while let Some(opnd) = opnd_iter.next() { + insn.for_each_operand_mut(|opnd| { // Lower Opnd::Value to Opnd::VReg or Opnd::UImm if let Opnd::Value(value) = opnd { // If the value is a special constant, and it fits in 32 bits, @@ -176,7 +175,7 @@ impl Assembler { *opnd = asm.load(*opnd); } } - } + }); } // We are replacing instructions here so we know they are already From b070c494acd225979c507b42185d6d88e66e2400 Mon Sep 17 00:00:00 2001 From: Jacob Date: Wed, 27 May 2026 00:06:29 -0400 Subject: [PATCH 3/5] ZJIT: Replace rpo with reverse_post_order (#17122) After a team discussion about naming, we decided that `rpo` should be expanded to `reverse_post_order`. This PR replaces the functions named `rpo` in HIR and LIR. --- zjit/src/backend/lir.rs | 4 +-- zjit/src/codegen.rs | 6 ++--- zjit/src/hir.rs | 58 ++++++++++++++++++++--------------------- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index a3ee8f51f1a91d..b29f832000fec1 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -2572,7 +2572,7 @@ impl Assembler } /// Return a traversal of the block graph in reverse post-order. - pub fn rpo(&self) -> Vec { + pub fn reverse_post_order(&self) -> Vec { let entry_blocks: Vec = self.basic_blocks.iter() .filter(|block| block.is_entry) .map(|block| block.id) @@ -2697,7 +2697,7 @@ impl Assembler } pub fn block_order(&self) -> Vec { - self.rpo() + self.reverse_post_order() } /// Calculate live intervals for each VReg. diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 4eee7693150c20..fbf62a5c81156e 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -383,7 +383,7 @@ fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, version: IseqVersionRef, func // jump targets in LIR (LIR should always jump to the head of an HIR block) let mut hir_to_lir: Vec> = vec![None; function.num_blocks()]; - let reverse_post_order = function.rpo(); + let reverse_post_order = function.reverse_post_order(); // Create all LIR basic blocks corresponding to HIR basic blocks for (rpo_idx, &block_id) in reverse_post_order.iter().enumerate() { @@ -509,7 +509,7 @@ fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, version: IseqVersionRef, func assert!(asm.current_block().insns.last().unwrap().is_terminator()); } - assert!(!asm.rpo().is_empty()); + assert!(!asm.reverse_post_order().is_empty()); // Validate CFG invariants after HIR to LIR lowering asm.validate_jump_positions(); @@ -2988,7 +2988,7 @@ fn build_side_exit(jit: &JITState, state: &FrameState) -> SideExit { /// Returne the maximum number of arguments for a block in a given function fn max_num_params(function: &Function) -> usize { - let reverse_post_order = function.rpo(); + let reverse_post_order = function.reverse_post_order(); reverse_post_order .iter() .filter(|&&block_id| function.is_entry_block(block_id)) diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 93dbbec690ab2d..02ef9e0a1f2c5a 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -3084,7 +3084,7 @@ impl Function { reachable.insert(self.entries_block); // Walk the graph, computing types until fixpoint - let rpo = self.rpo(); + let rpo = self.reverse_post_order(); loop { let mut changed = false; for &block in &rpo { @@ -3564,7 +3564,7 @@ impl Function { /// Also try and inline constant caches, specialize object allocations, and more. /// Calls to C functions are handled separately in optimize_c_calls. fn type_specialize(&mut self) { - for block in self.rpo() { + for block in self.reverse_post_order() { let old_insns = std::mem::take(&mut self.blocks[block.0].insns); assert!(self.blocks[block.0].insns.is_empty()); for insn_id in old_insns { @@ -4211,7 +4211,7 @@ impl Function { } fn inline(&mut self) { - for block in self.rpo() { + for block in self.reverse_post_order() { let old_insns = std::mem::take(&mut self.blocks[block.0].insns); assert!(self.blocks[block.0].insns.is_empty()); for insn_id in old_insns { @@ -4392,7 +4392,7 @@ impl Function { } fn optimize_getivar(&mut self) { - for block in self.rpo() { + for block in self.reverse_post_order() { let old_insns = std::mem::take(&mut self.blocks[block.0].insns); assert!(self.blocks[block.0].insns.is_empty()); for insn_id in old_insns { @@ -4859,7 +4859,7 @@ impl Function { } } - for block in self.rpo() { + for block in self.reverse_post_order() { let old_insns = std::mem::take(&mut self.blocks[block.0].insns); assert!(self.blocks[block.0].insns.is_empty()); for insn_id in old_insns { @@ -4911,7 +4911,7 @@ impl Function { if payload.versions.len() + 1 >= crate::codegen::max_iseq_versions() { return; } - for block in self.rpo() { + for block in self.reverse_post_order() { let old_insns = std::mem::take(&mut self.blocks[block.0].insns); assert!(self.blocks[block.0].insns.is_empty()); for insn_id in old_insns { @@ -4931,7 +4931,7 @@ impl Function { } fn optimize_load_store(&mut self) { - for block in self.rpo() { + for block in self.reverse_post_order() { let mut compile_time_heap: HashMap<(InsnId, i32), InsnId> = HashMap::new(); let old_insns = std::mem::take(&mut self.blocks[block.0].insns); let mut new_insns = vec![]; @@ -5025,7 +5025,7 @@ impl Function { /// (). fn canonicalize(&mut self) { let mut rewrite_map: HashMap = HashMap::new(); - for block in self.rpo() { + for block in self.reverse_post_order() { rewrite_map.clear(); for i in 0..self.blocks[block.0].insns.len() { let insn_id = self.blocks[block.0].insns[i]; @@ -5066,7 +5066,7 @@ impl Function { // // This would require 1) fixpointing, 2) worklist, or 3) (slightly less powerful) calling a // function-level infer_types after each pruned branch. - for block in self.rpo() { + for block in self.reverse_post_order() { let old_insns = std::mem::take(&mut self.blocks[block.0].insns); let mut new_insns = vec![]; for insn_id in old_insns { @@ -5346,7 +5346,7 @@ impl Function { /// Remove instructions that do not have side effects and are not referenced by any other /// instruction. fn eliminate_dead_code(&mut self) { - let rpo = self.rpo(); + let rpo = self.reverse_post_order(); let mut worklist = VecDeque::new(); // Find all of the instructions that have side effects, are control instructions, or are // otherwise necessary to keep around @@ -5407,7 +5407,7 @@ impl Function { // * blocks that get absorbed are not in RPO anymore // * blocks pointed to by blocks that get absorbed retain the same number of in-edges let mut num_in_edges = vec![0; self.blocks.len()]; - for block in self.rpo() { + for block in self.reverse_post_order() { for target in self.successors(block) { num_in_edges[target.0] += 1; } @@ -5415,7 +5415,7 @@ impl Function { let mut changed = false; loop { let mut iter_changed = false; - for block in self.rpo() { + for block in self.reverse_post_order() { // Ignore transient empty blocks if self.blocks[block.0].insns.is_empty() { continue; } loop { @@ -5436,7 +5436,7 @@ impl Function { /// Two PatchPoints are redundant if they assert the same Invariant and no /// intervening instruction could invalidate it (i.e., writes to PatchPoint). fn remove_redundant_patch_points(&mut self) { - for block_id in self.rpo() { + for block_id in self.reverse_post_order() { let mut seen = HashSet::new(); let insns = std::mem::take(&mut self.blocks[block_id.0].insns); let mut new_insns = Vec::with_capacity(insns.len()); @@ -5459,7 +5459,7 @@ impl Function { /// Only the first CheckInterrupts in a block is needed unless an intervening /// instruction writes to InterruptFlag (e.g. a call), which resets tracking. fn remove_duplicate_check_interrupts(&mut self) { - for block_id in self.rpo() { + for block_id in self.reverse_post_order() { let mut seen = false; let insns = std::mem::take(&mut self.blocks[block_id.0].insns); let mut new_insns = Vec::with_capacity(insns.len()); @@ -5496,7 +5496,7 @@ impl Function { } /// Return a traversal of the `Function`'s `BlockId`s in reverse post-order. - pub fn rpo(&self) -> Vec { + pub fn reverse_post_order(&self) -> Vec { let mut result = self.po_from(self.entries_block); result.reverse(); result @@ -5594,7 +5594,7 @@ impl Function { let loop_info = LoopInfo::new(&cfi, &dominators); // Push each block from the iteration in reverse post order to `hir_blocks`. - for block_id in self.rpo() { + for block_id in self.reverse_post_order() { // Create the block with instructions. let block = &self.blocks[block_id.0]; let predecessors = cfi.predecessors(block_id).collect(); @@ -5778,7 +5778,7 @@ impl Function { Ok(()) }; - for block_id in self.rpo() { + for block_id in self.reverse_post_order() { let insns = &self.blocks[block_id.0].insns; for (idx, insn_id) in insns.iter().enumerate() { let insn = self.find(*insn_id); @@ -5819,7 +5819,7 @@ impl Function { // Initialize with all missing values at first, to catch if a jump target points to a // missing location. let mut assigned_in = vec![None; self.num_blocks()]; - let rpo = self.rpo(); + let rpo = self.reverse_post_order(); // Begin with every block having every variable defined, except for entries_block, which // starts with nothing defined. for &block in &rpo { @@ -5894,7 +5894,7 @@ impl Function { /// Checks that each instruction('s representative) appears only once in the CFG. fn validate_insn_uniqueness(&self) -> Result<(), ValidationError> { let mut seen = InsnSet::with_capacity(self.insns.len()); - for block_id in self.rpo() { + for block_id in self.reverse_post_order() { for &insn_id in &self.blocks[block_id.0].insns { let insn_id = self.union_find.borrow().find_const(insn_id); if !seen.insert(insn_id) { @@ -6229,7 +6229,7 @@ impl Function { /// Check that insn types match the expected types for each instruction. fn validate_types(&self) -> Result<(), ValidationError> { - for block_id in self.rpo() { + for block_id in self.reverse_post_order() { for &insn_id in &self.blocks[block_id.0].insns { self.validate_insn_type(insn_id)?; } @@ -6264,7 +6264,7 @@ impl<'a> std::fmt::Display for FunctionPrinter<'a> { iseq_name }; writeln!(f, "fn {iseq_name}:")?; - for block_id in fun.rpo() { + for block_id in fun.reverse_post_order() { if !self.display_snapshot_and_tp_patchpoints && block_id == fun.entries_block { // Unless we're doing --zjit-dump-hir=all, skip the entries superblock -- it's an // internal CFG artifact @@ -8623,7 +8623,7 @@ impl Dominators { /// Cooper, Harvey & Kennedy, "A Simple, Fast Dominance Algorithm" (2001), /// Figure 3: pub fn with_cfi(f: &Function, cfi: &mut ControlFlowInfo) -> Self { - let rpo = f.rpo(); + let rpo = f.reverse_post_order(); let num_blocks = f.blocks.len(); // Map BlockId -> RPO index for O(1) lookup in intersect. @@ -8731,7 +8731,7 @@ impl<'a> ControlFlowInfo<'a> { let mut successor_map: HashMap> = HashMap::new(); let mut predecessor_map: HashMap> = HashMap::new(); - for block_id in function.rpo() { + for block_id in function.reverse_post_order() { let mut successors = function.successors(block_id); successors.dedup(); @@ -8784,7 +8784,7 @@ impl<'a> LoopInfo<'a> { let mut loop_headers: BlockSet = BlockSet::with_capacity(cfi.function.num_blocks()); let mut loop_depths: HashMap = HashMap::new(); let mut back_edge_sources: BlockSet = BlockSet::with_capacity(cfi.function.num_blocks()); - let rpo = cfi.function.rpo(); + let rpo = cfi.function.reverse_post_order(); for &block in &rpo { loop_depths.insert(block, 0); @@ -8909,7 +8909,7 @@ mod rpo_tests { let val = function.push_insn(entry, Insn::Const { val: Const::Value(Qnil) }); function.push_insn(entry, Insn::Return { val }); function.seal_entries(); - assert_eq!(function.rpo(), vec![entries, entry]); + assert_eq!(function.reverse_post_order(), vec![entries, entry]); } #[test] @@ -8922,7 +8922,7 @@ mod rpo_tests { let val = function.push_insn(exit, Insn::Const { val: Const::Value(Qnil) }); function.push_insn(exit, Insn::Return { val }); function.seal_entries(); - assert_eq!(function.rpo(), vec![entries, entry, exit]); + assert_eq!(function.reverse_post_order(), vec![entries, entry, exit]); } #[test] @@ -8942,7 +8942,7 @@ mod rpo_tests { let val = function.push_insn(exit, Insn::Const { val: Const::Value(Qnil) }); function.push_insn(exit, Insn::Return { val }); function.seal_entries(); - assert_eq!(function.rpo(), vec![entries, entry, side, exit]); + assert_eq!(function.reverse_post_order(), vec![entries, entry, side, exit]); } #[test] @@ -8962,7 +8962,7 @@ mod rpo_tests { let val = function.push_insn(exit, Insn::Const { val: Const::Value(Qnil) }); function.push_insn(exit, Insn::Return { val }); function.seal_entries(); - assert_eq!(function.rpo(), vec![entries, entry, side, exit]); + assert_eq!(function.reverse_post_order(), vec![entries, entry, side, exit]); } #[test] @@ -8972,7 +8972,7 @@ mod rpo_tests { let entry = function.entry_block; function.push_insn(entry, Insn::Jump(BranchEdge { target: entry, args: vec![] })); function.seal_entries(); - assert_eq!(function.rpo(), vec![entries, entry]); + assert_eq!(function.reverse_post_order(), vec![entries, entry]); } } From eb70a0d0c4042d63cf2011c64c760ec01ee325e8 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 27 May 2026 04:25:49 +0000 Subject: [PATCH 4/5] Bump taiki-e/install-action Bumps the github-actions group with 1 update in the / directory: [taiki-e/install-action](https://github.com/taiki-e/install-action). Updates `taiki-e/install-action` from 2.79.8 to 2.79.9 - [Release notes](https://github.com/taiki-e/install-action/releases) - [Changelog](https://github.com/taiki-e/install-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/taiki-e/install-action/compare/920ab1831fbf4fb3ef75c8ead83556c918bb7290...8f531eaecd1898bc3da7d104ad91bee98d1b97bd) --- updated-dependencies: - dependency-name: taiki-e/install-action dependency-version: 2.79.9 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions ... Signed-off-by: dependabot[bot] --- .github/workflows/zjit-macos.yml | 2 +- .github/workflows/zjit-ubuntu.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/zjit-macos.yml b/.github/workflows/zjit-macos.yml index b629219bef376a..3011a04cacb636 100644 --- a/.github/workflows/zjit-macos.yml +++ b/.github/workflows/zjit-macos.yml @@ -93,7 +93,7 @@ jobs: rustup install ${{ matrix.rust_version }} --profile minimal rustup default ${{ matrix.rust_version }} - - uses: taiki-e/install-action@920ab1831fbf4fb3ef75c8ead83556c918bb7290 # v2.79.8 + - uses: taiki-e/install-action@8f531eaecd1898bc3da7d104ad91bee98d1b97bd # v2.79.9 with: tool: nextest@0.9 if: ${{ matrix.test_task == 'zjit-check' }} diff --git a/.github/workflows/zjit-ubuntu.yml b/.github/workflows/zjit-ubuntu.yml index 896a74330552ab..d91dfcbeeb7738 100644 --- a/.github/workflows/zjit-ubuntu.yml +++ b/.github/workflows/zjit-ubuntu.yml @@ -119,7 +119,7 @@ jobs: ruby-version: '3.1' bundler: none - - uses: taiki-e/install-action@920ab1831fbf4fb3ef75c8ead83556c918bb7290 # v2.79.8 + - uses: taiki-e/install-action@8f531eaecd1898bc3da7d104ad91bee98d1b97bd # v2.79.9 with: tool: nextest@0.9 if: ${{ matrix.test_task == 'zjit-check' }} From 261eea422a276dbc1fbfeae5b72cf32efd64b076 Mon Sep 17 00:00:00 2001 From: Sampo Kuokkanen Date: Wed, 27 May 2026 13:00:24 +0900 Subject: [PATCH 5/5] Fix tautological assertions in test_each_value in test_hash.rb Both assertions tested `[].length == 0` rather than `res.length`, so the test passed even when `each_value` yielded nothing. --- test/ruby/test_hash.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/ruby/test_hash.rb b/test/ruby/test_hash.rb index 32384f5a5c1fd4..2d1b513c7092c7 100644 --- a/test/ruby/test_hash.rb +++ b/test/ruby/test_hash.rb @@ -465,10 +465,10 @@ def test_each_pair def test_each_value res = [] @cls[].each_value { |v| res << v } - assert_equal(0, [].length) + assert_equal(0, res.length) @h.each_value { |v| res << v } - assert_equal(0, [].length) + assert_equal(@h.size, res.length) expected = [] @h.each { |k, v| expected << v }