From 95936276d3e41b2c48a82922005a3b1429dd9e5a Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Wed, 10 Sep 2025 21:18:46 +0100 Subject: [PATCH 1/8] ZJIT: Revert `self_val` removal and rename it to `recv` (#14504) * Revert "ZJIT: Removed unused self_val from Send" This reverts commit 13c2f8d5c2b41ec78344ae60f9b5ec1564029418. * Revert "ZJIT: Removed unused self_val from InvokeSuper" This reverts commit 877b625922e0f8de4e7ad801dd0306e69b34d263. * ZJIT: Rename self_val in dispatching HIRs to recv * ZJIT: Remove unnecessary constructor param names --- zjit/src/codegen.rs | 2 +- zjit/src/hir.rs | 172 ++++++++++++++++++++++---------------------- 2 files changed, 88 insertions(+), 86 deletions(-) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 8c435cc854f61e..73f0ab5f85d79a 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -367,7 +367,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio // Give up SendWithoutBlockDirect for 6+ args since asm.ccall() doesn't support it. Insn::SendWithoutBlockDirect { cd, state, args, .. } if args.len() + 1 > C_ARG_OPNDS.len() => // +1 for self gen_send_without_block(jit, asm, *cd, &function.frame_state(*state)), - Insn::SendWithoutBlockDirect { cme, iseq, self_val, args, state, .. } => gen_send_without_block_direct(cb, jit, asm, *cme, *iseq, opnd!(self_val), opnds!(args), &function.frame_state(*state)), + Insn::SendWithoutBlockDirect { cme, iseq, recv, args, state, .. } => gen_send_without_block_direct(cb, jit, asm, *cme, *iseq, opnd!(recv), opnds!(args), &function.frame_state(*state)), &Insn::InvokeSuper { cd, blockiseq, state, .. } => gen_invokesuper(jit, asm, cd, blockiseq, &function.frame_state(state)), Insn::InvokeBlock { cd, state, .. } => gen_invoke_block(jit, asm, *cd, &function.frame_state(*state)), // Ensure we have enough room fit ec, self, and arguments diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 27bfe1fc7b92be..932164b550d450 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -582,14 +582,14 @@ pub enum Insn { /// Un-optimized fallback implementation (dynamic dispatch) for send-ish instructions /// Ignoring keyword arguments etc for now - SendWithoutBlock { self_val: InsnId, cd: *const rb_call_data, args: Vec, state: InsnId }, - Send { cd: *const rb_call_data, blockiseq: IseqPtr, args: Vec, state: InsnId }, - InvokeSuper { cd: *const rb_call_data, blockiseq: IseqPtr, args: Vec, state: InsnId }, + SendWithoutBlock { recv: InsnId, cd: *const rb_call_data, args: Vec, state: InsnId }, + Send { recv: InsnId, cd: *const rb_call_data, blockiseq: IseqPtr, args: Vec, state: InsnId }, + InvokeSuper { recv: InsnId, cd: *const rb_call_data, blockiseq: IseqPtr, args: Vec, state: InsnId }, InvokeBlock { cd: *const rb_call_data, args: Vec, state: InsnId }, /// Optimized ISEQ call SendWithoutBlockDirect { - self_val: InsnId, + recv: InsnId, cd: *const rb_call_data, cme: *const rb_callable_method_entry_t, iseq: IseqPtr, @@ -815,32 +815,32 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> { Insn::Jump(target) => { write!(f, "Jump {target}") } Insn::IfTrue { val, target } => { write!(f, "IfTrue {val}, {target}") } Insn::IfFalse { val, target } => { write!(f, "IfFalse {val}, {target}") } - Insn::SendWithoutBlock { self_val, cd, args, .. } => { - write!(f, "SendWithoutBlock {self_val}, :{}", ruby_call_method_name(*cd))?; + Insn::SendWithoutBlock { recv, cd, args, .. } => { + write!(f, "SendWithoutBlock {recv}, :{}", ruby_call_method_name(*cd))?; for arg in args { write!(f, ", {arg}")?; } Ok(()) } - Insn::SendWithoutBlockDirect { self_val, cd, iseq, args, .. } => { - write!(f, "SendWithoutBlockDirect {self_val}, :{} ({:?})", ruby_call_method_name(*cd), self.ptr_map.map_ptr(iseq))?; + Insn::SendWithoutBlockDirect { recv, cd, iseq, args, .. } => { + write!(f, "SendWithoutBlockDirect {recv}, :{} ({:?})", ruby_call_method_name(*cd), self.ptr_map.map_ptr(iseq))?; for arg in args { write!(f, ", {arg}")?; } Ok(()) } - Insn::Send { cd, args, blockiseq, .. } => { + Insn::Send { recv, cd, args, blockiseq, .. } => { // For tests, we want to check HIR snippets textually. Addresses change // between runs, making tests fail. Instead, pick an arbitrary hex value to // use as a "pointer" so we can check the rest of the HIR. - write!(f, "Send {:p}, :{}", self.ptr_map.map_ptr(blockiseq), ruby_call_method_name(*cd))?; + write!(f, "Send {recv}, {:p}, :{}", self.ptr_map.map_ptr(blockiseq), ruby_call_method_name(*cd))?; for arg in args { write!(f, ", {arg}")?; } Ok(()) } - Insn::InvokeSuper { blockiseq, args, .. } => { - write!(f, "InvokeSuper {:p}", self.ptr_map.map_ptr(blockiseq))?; + Insn::InvokeSuper { recv, blockiseq, args, .. } => { + write!(f, "InvokeSuper {recv}, {:p}", self.ptr_map.map_ptr(blockiseq))?; for arg in args { write!(f, ", {arg}")?; } @@ -1329,27 +1329,29 @@ impl Function { str: find!(str), state, }, - &SendWithoutBlock { self_val, cd, ref args, state } => SendWithoutBlock { - self_val: find!(self_val), + &SendWithoutBlock { recv, cd, ref args, state } => SendWithoutBlock { + recv: find!(recv), cd, args: find_vec!(args), state, }, - &SendWithoutBlockDirect { self_val, cd, cme, iseq, ref args, state } => SendWithoutBlockDirect { - self_val: find!(self_val), + &SendWithoutBlockDirect { recv, cd, cme, iseq, ref args, state } => SendWithoutBlockDirect { + recv: find!(recv), cd, cme, iseq, args: find_vec!(args), state, }, - &Send { cd, blockiseq, ref args, state } => Send { + &Send { recv, cd, blockiseq, ref args, state } => Send { + recv: find!(recv), cd, blockiseq, args: find_vec!(args), state, }, - &InvokeSuper { cd, blockiseq, ref args, state } => InvokeSuper { + &InvokeSuper { recv, cd, blockiseq, ref args, state } => InvokeSuper { + recv: find!(recv), cd, blockiseq, args: find_vec!(args), @@ -1707,47 +1709,47 @@ impl Function { assert!(self.blocks[block.0].insns.is_empty()); for insn_id in old_insns { match self.find(insn_id) { - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(plus) && args.len() == 1 => - self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumAdd { left, right, state }, BOP_PLUS, self_val, args[0], state), - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(minus) && args.len() == 1 => - self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumSub { left, right, state }, BOP_MINUS, self_val, args[0], state), - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(mult) && args.len() == 1 => - self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumMult { left, right, state }, BOP_MULT, self_val, args[0], state), - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(div) && args.len() == 1 => - self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumDiv { left, right, state }, BOP_DIV, self_val, args[0], state), - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(modulo) && args.len() == 1 => - self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumMod { left, right, state }, BOP_MOD, self_val, args[0], state), - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(eq) && args.len() == 1 => - self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumEq { left, right }, BOP_EQ, self_val, args[0], state), - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(neq) && args.len() == 1 => - self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumNeq { left, right }, BOP_NEQ, self_val, args[0], state), - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(lt) && args.len() == 1 => - self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumLt { left, right }, BOP_LT, self_val, args[0], state), - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(le) && args.len() == 1 => - self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumLe { left, right }, BOP_LE, self_val, args[0], state), - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(gt) && args.len() == 1 => - self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumGt { left, right }, BOP_GT, self_val, args[0], state), - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(ge) && args.len() == 1 => - self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumGe { left, right }, BOP_GE, self_val, args[0], state), - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(and) && args.len() == 1 => - self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumAnd { left, right }, BOP_AND, self_val, args[0], state), - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(or) && args.len() == 1 => - self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumOr { left, right }, BOP_OR, self_val, args[0], state), - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(freeze) && args.is_empty() => - self.try_rewrite_freeze(block, insn_id, self_val, state), - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(minusat) && args.is_empty() => - self.try_rewrite_uminus(block, insn_id, self_val, state), - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(aref) && args.len() == 1 => - self.try_rewrite_aref(block, insn_id, self_val, args[0], state), - Insn::SendWithoutBlock { mut self_val, cd, args, state } => { + Insn::SendWithoutBlock { recv, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(plus) && args.len() == 1 => + self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumAdd { left, right, state }, BOP_PLUS, recv, args[0], state), + Insn::SendWithoutBlock { recv, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(minus) && args.len() == 1 => + self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumSub { left, right, state }, BOP_MINUS, recv, args[0], state), + Insn::SendWithoutBlock { recv, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(mult) && args.len() == 1 => + self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumMult { left, right, state }, BOP_MULT, recv, args[0], state), + Insn::SendWithoutBlock { recv, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(div) && args.len() == 1 => + self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumDiv { left, right, state }, BOP_DIV, recv, args[0], state), + Insn::SendWithoutBlock { recv, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(modulo) && args.len() == 1 => + self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumMod { left, right, state }, BOP_MOD, recv, args[0], state), + Insn::SendWithoutBlock { recv, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(eq) && args.len() == 1 => + self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumEq { left, right }, BOP_EQ, recv, args[0], state), + Insn::SendWithoutBlock { recv, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(neq) && args.len() == 1 => + self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumNeq { left, right }, BOP_NEQ, recv, args[0], state), + Insn::SendWithoutBlock { recv, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(lt) && args.len() == 1 => + self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumLt { left, right }, BOP_LT, recv, args[0], state), + Insn::SendWithoutBlock { recv, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(le) && args.len() == 1 => + self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumLe { left, right }, BOP_LE, recv, args[0], state), + Insn::SendWithoutBlock { recv, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(gt) && args.len() == 1 => + self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumGt { left, right }, BOP_GT, recv, args[0], state), + Insn::SendWithoutBlock { recv, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(ge) && args.len() == 1 => + self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumGe { left, right }, BOP_GE, recv, args[0], state), + Insn::SendWithoutBlock { recv, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(and) && args.len() == 1 => + self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumAnd { left, right }, BOP_AND, recv, args[0], state), + Insn::SendWithoutBlock { recv, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(or) && args.len() == 1 => + self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumOr { left, right }, BOP_OR, recv, args[0], state), + Insn::SendWithoutBlock { recv, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(freeze) && args.is_empty() => + self.try_rewrite_freeze(block, insn_id, recv, state), + Insn::SendWithoutBlock { recv, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(minusat) && args.is_empty() => + self.try_rewrite_uminus(block, insn_id, recv, state), + Insn::SendWithoutBlock { recv, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(aref) && args.len() == 1 => + self.try_rewrite_aref(block, insn_id, recv, args[0], state), + Insn::SendWithoutBlock { mut recv, cd, args, state } => { let frame_state = self.frame_state(state); - let (klass, profiled_type) = if let Some(klass) = self.type_of(self_val).runtime_exact_ruby_class() { + let (klass, profiled_type) = if let Some(klass) = self.type_of(recv).runtime_exact_ruby_class() { // If we know the class statically, use it to fold the lookup at compile-time. (klass, None) } else { // If we know that self is reasonably monomorphic from profile information, guard and use it to fold the lookup at compile-time. // TODO(max): Figure out how to handle top self? - let Some(recv_type) = self.profiled_type_of_at(self_val, frame_state.insn_idx) else { + let Some(recv_type) = self.profiled_type_of_at(recv, frame_state.insn_idx) else { self.push_insn_id(block, insn_id); continue; }; (recv_type.class(), Some(recv_type)) @@ -1773,14 +1775,14 @@ impl Function { } self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state }); if let Some(profiled_type) = profiled_type { - self_val = self.push_insn(block, Insn::GuardType { val: self_val, guard_type: Type::from_profiled_type(profiled_type), state }); + recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state }); } - let send_direct = self.push_insn(block, Insn::SendWithoutBlockDirect { self_val, cd, cme, iseq, args, state }); + let send_direct = self.push_insn(block, Insn::SendWithoutBlockDirect { recv, cd, cme, iseq, args, state }); self.make_equal_to(insn_id, send_direct); } else if def_type == VM_METHOD_TYPE_IVAR && args.is_empty() { self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state }); if let Some(profiled_type) = profiled_type { - self_val = self.push_insn(block, Insn::GuardType { val: self_val, guard_type: Type::from_profiled_type(profiled_type), state }); + recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state }); } let id = unsafe { get_cme_def_body_attr_id(cme) }; @@ -1792,7 +1794,7 @@ impl Function { self.push_insn(block, Insn::PatchPoint { invariant: Invariant::SingleRactorMode, state }); } } - let getivar = self.push_insn(block, Insn::GetIvar { self_val, id, state }); + let getivar = self.push_insn(block, Insn::GetIvar { self_val: recv, id, state }); self.make_equal_to(insn_id, getivar); } else { self.push_insn_id(block, insn_id); continue; @@ -1830,13 +1832,13 @@ impl Function { }; if recv_type.is_string() { - let guard = self.push_insn(block, Insn::GuardType { val: val, guard_type: types::String, state: state }); + let guard = self.push_insn(block, Insn::GuardType { val, guard_type: types::String, state }); // Infer type so AnyToString can fold off this self.insn_types[guard.0] = self.infer_type(guard); self.make_equal_to(insn_id, guard); } else { - self.push_insn(block, Insn::GuardTypeNot { val: val, guard_type: types::String, state: state}); - let send_to_s = self.push_insn(block, Insn::SendWithoutBlock { self_val: val, cd: cd, args: vec![], state: state}); + self.push_insn(block, Insn::GuardTypeNot { val, guard_type: types::String, state}); + let send_to_s = self.push_insn(block, Insn::SendWithoutBlock { recv: val, cd, args: vec![], state}); self.make_equal_to(insn_id, send_to_s); } } @@ -1913,7 +1915,7 @@ impl Function { send: Insn, send_insn_id: InsnId, ) -> Result<(), ()> { - let Insn::SendWithoutBlock { mut self_val, cd, mut args, state, .. } = send else { + let Insn::SendWithoutBlock { mut recv, cd, mut args, state, .. } = send else { return Err(()); }; @@ -1926,7 +1928,7 @@ impl Function { (class, None) } else { let iseq_insn_idx = fun.frame_state(state).insn_idx; - let Some(recv_type) = fun.profiled_type_of_at(self_val, iseq_insn_idx) else { return Err(()) }; + let Some(recv_type) = fun.profiled_type_of_at(recv, iseq_insn_idx) else { return Err(()) }; (recv_type.class(), Some(recv_type)) }; @@ -1969,10 +1971,10 @@ impl Function { fun.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass: recv_class, method: method_id, cme: method }, state }); if let Some(profiled_type) = profiled_type { // Guard receiver class - self_val = fun.push_insn(block, Insn::GuardType { val: self_val, guard_type: Type::from_profiled_type(profiled_type), state }); + recv = fun.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state }); } let cfun = unsafe { get_mct_func(cfunc) }.cast(); - let mut cfunc_args = vec![self_val]; + let mut cfunc_args = vec![recv]; cfunc_args.append(&mut args); let ccall = fun.push_insn(block, Insn::CCall { cfun, args: cfunc_args, name: method_id, return_type, elidable }); fun.make_equal_to(send_insn_id, ccall); @@ -1997,9 +1999,9 @@ impl Function { 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 { - if let send @ Insn::SendWithoutBlock { self_val, .. } = self.find(insn_id) { - let self_type = self.type_of(self_val); - if reduce_to_ccall(self, block, self_type, send, insn_id).is_ok() { + if let send @ Insn::SendWithoutBlock { recv, .. } = self.find(insn_id) { + let recv_type = self.type_of(recv); + if reduce_to_ccall(self, block, recv_type, send, insn_id).is_ok() { continue; } } @@ -2280,15 +2282,15 @@ impl Function { worklist.push_back(val); worklist.push_back(state); } - | &Insn::SendWithoutBlock { self_val, ref args, state, .. } - | &Insn::SendWithoutBlockDirect { self_val, ref args, state, .. } => { - worklist.push_back(self_val); + &Insn::Send { recv, ref args, state, .. } + | &Insn::SendWithoutBlock { recv, ref args, state, .. } + | &Insn::SendWithoutBlockDirect { recv, ref args, state, .. } + | &Insn::InvokeSuper { recv, ref args, state, .. } => { + worklist.push_back(recv); worklist.extend(args); worklist.push_back(state); } - &Insn::InvokeSuper { ref args, state, .. } - | &Insn::Send { ref args, state, .. } - | &Insn::InvokeBuiltin { ref args, state, .. } + &Insn::InvokeBuiltin { ref args, state, .. } | &Insn::InvokeBlock { ref args, state, .. } => { worklist.extend(args); worklist.push_back(state) @@ -3527,7 +3529,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let args = state.stack_pop_n(argc as usize)?; let recv = state.stack_pop()?; let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - let send = fun.push_insn(block, Insn::SendWithoutBlock { self_val: recv, cd, args, state: exit_id }); + let send = fun.push_insn(block, Insn::SendWithoutBlock { recv, cd, args, state: exit_id }); state.stack_push(send); } YARVINSN_opt_hash_freeze | @@ -3550,7 +3552,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); let recv = fun.push_insn(block, Insn::Const { val: Const::Value(get_arg(pc, 0)) }); - let send = fun.push_insn(block, Insn::SendWithoutBlock { self_val: recv, cd, args, state: exit_id }); + let send = fun.push_insn(block, Insn::SendWithoutBlock { recv, cd, args, state: exit_id }); state.stack_push(send); } @@ -3606,7 +3608,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let args = state.stack_pop_n(argc as usize)?; let recv = state.stack_pop()?; let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - let send = fun.push_insn(block, Insn::SendWithoutBlock { self_val: recv, cd, args, state: exit_id }); + let send = fun.push_insn(block, Insn::SendWithoutBlock { recv, cd, args, state: exit_id }); state.stack_push(send); } YARVINSN_send => { @@ -3622,9 +3624,9 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let argc = unsafe { vm_ci_argc((*cd).ci) }; let args = state.stack_pop_n(argc as usize)?; - let _recv = state.stack_pop()?; + let recv = state.stack_pop()?; let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - let send = fun.push_insn(block, Insn::Send { cd, blockiseq, args, state: exit_id }); + let send = fun.push_insn(block, Insn::Send { recv, cd, blockiseq, args, state: exit_id }); state.stack_push(send); // Reload locals that may have been modified by the blockiseq. @@ -3647,10 +3649,10 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { } let argc = unsafe { vm_ci_argc((*cd).ci) }; let args = state.stack_pop_n(argc as usize)?; - let _recv = state.stack_pop()?; + let recv = state.stack_pop()?; let blockiseq: IseqPtr = get_arg(pc, 1).as_ptr(); let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - let result = fun.push_insn(block, Insn::InvokeSuper { cd, blockiseq, args, state: exit_id }); + let result = fun.push_insn(block, Insn::InvokeSuper { recv, cd, blockiseq, args, state: exit_id }); state.stack_push(result); if !blockiseq.is_null() { @@ -5063,7 +5065,7 @@ mod tests { fn test@:3: bb0(v0:BasicObject, v1:BasicObject): v5:BasicObject = GetLocal l0, EP@3 - v7:BasicObject = Send 0x1000, :each + v7:BasicObject = Send v5, 0x1000, :each v8:BasicObject = GetLocal l0, EP@3 CheckInterrupts Return v7 @@ -5174,7 +5176,7 @@ mod tests { assert_snapshot!(hir_string("test"), @r" fn test@:2: bb0(v0:BasicObject): - v5:BasicObject = InvokeSuper 0x1000 + v5:BasicObject = InvokeSuper v0, 0x1000 CheckInterrupts Return v5 "); @@ -5188,7 +5190,7 @@ mod tests { assert_snapshot!(hir_string("test"), @r" fn test@:2: bb0(v0:BasicObject): - v5:BasicObject = InvokeSuper 0x1000 + v5:BasicObject = InvokeSuper v0, 0x1000 CheckInterrupts Return v5 "); @@ -7798,7 +7800,7 @@ mod opt_tests { assert_snapshot!(hir_string("test"), @r" fn test@:3: bb0(v0:BasicObject): - v5:BasicObject = Send 0x1000, :foo + v5:BasicObject = Send v0, 0x1000, :foo CheckInterrupts Return v5 "); @@ -7822,7 +7824,7 @@ mod opt_tests { v1:NilClass = Const Value(nil) v5:Fixnum[1] = Const Value(1) SetLocal l0, EP@3, v5 - v10:BasicObject = Send 0x1000, :foo + v10:BasicObject = Send v0, 0x1000, :foo v11:BasicObject = GetLocal l0, EP@3 v14:BasicObject = GetLocal l0, EP@3 CheckInterrupts From 399e2abc4380857134af9f0b037a4723157d14de Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 10 Sep 2025 09:09:13 -0700 Subject: [PATCH 2/8] Allow concurrent_set to be collected in minor GC When testing we've found that the concurrent_set objects used for fstrings can grow quite large, and because they reach oldgen quickly end up not being collected. This commit is a bit of a hack but aims to improve that by moving the objects to not be WB_PROTECTED. "Unprotected" objects do not age and can't become oldgen, so this allows them to be collected in a minor GC. --- concurrent_set.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/concurrent_set.c b/concurrent_set.c index ffbd028a2c0294..19b4b6c373559f 100644 --- a/concurrent_set.c +++ b/concurrent_set.c @@ -40,14 +40,26 @@ concurrent_set_size(const void *ptr) (set->capacity * sizeof(struct concurrent_set_entry)); } +/* Hack: Though it would be trivial, we're intentionally avoiding WB-protecting + * this object. This prevents the object from aging and ensures it can always be + * collected in a minor GC. + * Longer term this deserves a better way to reclaim memory promptly. + */ +static void +concurrent_set_mark(void *ptr) +{ + (void)ptr; +} + static const rb_data_type_t concurrent_set_type = { .wrap_struct_name = "VM/concurrent_set", .function = { - .dmark = NULL, + .dmark = concurrent_set_mark, .dfree = concurrent_set_free, .dsize = concurrent_set_size, }, - .flags = RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED | RUBY_TYPED_EMBEDDABLE + /* Hack: NOT WB_PROTECTED on purpose (see above) */ + .flags = RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_EMBEDDABLE }; VALUE From 23c60e185ea7e74f0eebc27119184fb1ed844856 Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Wed, 10 Sep 2025 21:37:17 +0100 Subject: [PATCH 3/8] YJIT: Tiny refactors (#14505) Addressed some suggestions from clippy that made sense to me. --- yjit/src/asm/mod.rs | 4 ++-- yjit/src/backend/ir.rs | 2 +- yjit/src/core.rs | 2 +- yjit/src/utils.rs | 5 +---- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/yjit/src/asm/mod.rs b/yjit/src/asm/mod.rs index ebdc205da96518..9ef675b34dfa82 100644 --- a/yjit/src/asm/mod.rs +++ b/yjit/src/asm/mod.rs @@ -441,12 +441,12 @@ impl CodeBlock { // Ignore empty code ranges if start_addr == end_addr { - return (0..0).into_iter(); + return 0..0; } let start_page = (start_addr.raw_addr(self) - mem_start) / self.page_size; let end_page = (end_addr.raw_addr(self) - mem_start - 1) / self.page_size; - (start_page..end_page + 1).into_iter() + start_page..end_page + 1 } /// Get a (possibly dangling) direct pointer to the current write position diff --git a/yjit/src/backend/ir.rs b/yjit/src/backend/ir.rs index b704a249853ceb..40df3ae4d5830b 100644 --- a/yjit/src/backend/ir.rs +++ b/yjit/src/backend/ir.rs @@ -1602,7 +1602,7 @@ impl Assembler if c_args.len() > 0 { // Resolve C argument dependencies let c_args_len = c_args.len() as isize; - let moves = Self::reorder_reg_moves(&c_args.drain(..).into_iter().collect()); + let moves = Self::reorder_reg_moves(&c_args.drain(..).collect()); shift_live_ranges(&mut shifted_live_ranges, asm.insns.len(), moves.len() as isize - c_args_len); // Push batched C arguments diff --git a/yjit/src/core.rs b/yjit/src/core.rs index f8c80c0c860130..cfe55b8c767374 100644 --- a/yjit/src/core.rs +++ b/yjit/src/core.rs @@ -1099,7 +1099,7 @@ impl Context { MapToLocal(local_idx) => { bits.push_op(CtxOp::MapTempLocal); bits.push_u3(stack_idx as u8); - bits.push_u3(local_idx as u8); + bits.push_u3(local_idx); } MapToSelf => { diff --git a/yjit/src/utils.rs b/yjit/src/utils.rs index 8c4133546d54c8..43fd90142f6e5e 100644 --- a/yjit/src/utils.rs +++ b/yjit/src/utils.rs @@ -92,10 +92,7 @@ pub fn ruby_str_to_rust(v: VALUE) -> String { let str_ptr = unsafe { rb_RSTRING_PTR(v) } as *mut u8; let str_len: usize = unsafe { rb_RSTRING_LEN(v) }.try_into().unwrap(); let str_slice: &[u8] = unsafe { slice::from_raw_parts(str_ptr, str_len) }; - match String::from_utf8(str_slice.to_vec()) { - Ok(utf8) => utf8, - Err(_) => String::new(), - } + String::from_utf8(str_slice.to_vec()).unwrap_or_default() } // Location is the file defining the method, colon, method name. From 0cec87399df1cdff47f5b7bcfa6a804a2ebd7868 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Wed, 10 Sep 2025 14:04:33 -0700 Subject: [PATCH 4/8] Rename VM instruction classes to singular names (#14507) --- template/Makefile.in | 10 +++++----- ...{bare_instructions.rb => bare_instruction.rb} | 6 +++--- tool/ruby_vm/models/instructions.rb | 16 ++++++++-------- ...ifications.rb => instructions_unification.rb} | 8 ++++---- ...s_unifications.rb => operands_unification.rb} | 10 +++++----- ...race_instructions.rb => trace_instruction.rb} | 8 ++++---- ...{zjit_instructions.rb => zjit_instruction.rb} | 8 ++++---- tool/ruby_vm/views/_insn_leaf_info.erb | 2 +- tool/ruby_vm/views/_zjit_helpers.erb | 4 ++-- tool/ruby_vm/views/optinsn.inc.erb | 4 ++-- tool/ruby_vm/views/vm.inc.erb | 10 +++++----- 11 files changed, 43 insertions(+), 43 deletions(-) rename tool/ruby_vm/models/{bare_instructions.rb => bare_instruction.rb} (99%) rename tool/ruby_vm/models/{instructions_unifications.rb => instructions_unification.rb} (85%) rename tool/ruby_vm/models/{operands_unifications.rb => operands_unification.rb} (93%) rename tool/ruby_vm/models/{trace_instructions.rb => trace_instruction.rb} (90%) rename tool/ruby_vm/models/{zjit_instructions.rb => zjit_instruction.rb} (86%) diff --git a/template/Makefile.in b/template/Makefile.in index 67e09789561a28..68d4240c4b56fa 100644 --- a/template/Makefile.in +++ b/template/Makefile.in @@ -664,14 +664,14 @@ $(INSNS): $(srcdir)/insns.def vm_opts.h \ $(tooldir)/ruby_vm/loaders/opt_operand_def.rb \ $(tooldir)/ruby_vm/loaders/vm_opts_h.rb \ $(tooldir)/ruby_vm/models/attribute.rb \ - $(tooldir)/ruby_vm/models/bare_instructions.rb \ + $(tooldir)/ruby_vm/models/bare_instruction.rb \ $(tooldir)/ruby_vm/models/c_expr.rb \ $(tooldir)/ruby_vm/models/instructions.rb \ - $(tooldir)/ruby_vm/models/instructions_unifications.rb \ - $(tooldir)/ruby_vm/models/operands_unifications.rb \ - $(tooldir)/ruby_vm/models/trace_instructions.rb \ + $(tooldir)/ruby_vm/models/instructions_unification.rb \ + $(tooldir)/ruby_vm/models/operands_unification.rb \ + $(tooldir)/ruby_vm/models/trace_instruction.rb \ $(tooldir)/ruby_vm/models/typemap.rb \ - $(tooldir)/ruby_vm/models/zjit_instructions.rb \ + $(tooldir)/ruby_vm/models/zjit_instruction.rb \ $(tooldir)/ruby_vm/scripts/converter.rb \ $(tooldir)/ruby_vm/scripts/insns2vm.rb \ $(tooldir)/ruby_vm/views/_attributes.erb \ diff --git a/tool/ruby_vm/models/bare_instructions.rb b/tool/ruby_vm/models/bare_instruction.rb similarity index 99% rename from tool/ruby_vm/models/bare_instructions.rb rename to tool/ruby_vm/models/bare_instruction.rb index 8ec7c9245cc293..f87dd741797be8 100644 --- a/tool/ruby_vm/models/bare_instructions.rb +++ b/tool/ruby_vm/models/bare_instruction.rb @@ -14,7 +14,7 @@ require_relative 'typemap' require_relative 'attribute' -class RubyVM::BareInstructions +class RubyVM::BareInstruction attr_reader :template, :name, :operands, :pops, :rets, :decls, :expr def initialize opts = {} @@ -224,13 +224,13 @@ def typesplit a new h.merge(:template => h) } - def self.fetch name + def self.find(name) @instances.find do |insn| insn.name == name end or raise IndexError, "instruction not found: #{name}" end - def self.to_a + def self.all @instances end end diff --git a/tool/ruby_vm/models/instructions.rb b/tool/ruby_vm/models/instructions.rb index e3da2ba5ac3525..f2c6562d37ae2a 100644 --- a/tool/ruby_vm/models/instructions.rb +++ b/tool/ruby_vm/models/instructions.rb @@ -9,14 +9,14 @@ # conditions mentioned in the file COPYING are met. Consult the file for # details. -require_relative 'bare_instructions' -require_relative 'operands_unifications' -require_relative 'instructions_unifications' +require_relative 'bare_instruction' +require_relative 'operands_unification' +require_relative 'instructions_unification' -RubyVM::Instructions = RubyVM::BareInstructions.to_a + \ - RubyVM::OperandsUnifications.to_a + \ - RubyVM::InstructionsUnifications.to_a +RubyVM::Instructions = RubyVM::BareInstruction.all + + RubyVM::OperandsUnification.all + + RubyVM::InstructionsUnification.all -require_relative 'trace_instructions' -require_relative 'zjit_instructions' +require_relative 'trace_instruction' +require_relative 'zjit_instruction' RubyVM::Instructions.freeze diff --git a/tool/ruby_vm/models/instructions_unifications.rb b/tool/ruby_vm/models/instructions_unification.rb similarity index 85% rename from tool/ruby_vm/models/instructions_unifications.rb rename to tool/ruby_vm/models/instructions_unification.rb index 2932ec57b264f2..5c798e6d544e8e 100644 --- a/tool/ruby_vm/models/instructions_unifications.rb +++ b/tool/ruby_vm/models/instructions_unification.rb @@ -11,9 +11,9 @@ require_relative '../helpers/c_escape' require_relative '../loaders/opt_insn_unif_def' -require_relative 'bare_instructions' +require_relative 'bare_instruction' -class RubyVM::InstructionsUnifications +class RubyVM::InstructionsUnification include RubyVM::CEscape attr_reader :name @@ -22,7 +22,7 @@ def initialize opts = {} @location = opts[:location] @name = namegen opts[:signature] @series = opts[:signature].map do |i| - RubyVM::BareInstructions.fetch i # Misshit is fatal + RubyVM::BareInstruction.find(i) # Misshit is fatal end end @@ -36,7 +36,7 @@ def namegen signature new h end - def self.to_a + def self.all @instances end end diff --git a/tool/ruby_vm/models/operands_unifications.rb b/tool/ruby_vm/models/operands_unification.rb similarity index 93% rename from tool/ruby_vm/models/operands_unifications.rb rename to tool/ruby_vm/models/operands_unification.rb index ff84abb3c3e0fb..ce118648ca00c7 100644 --- a/tool/ruby_vm/models/operands_unifications.rb +++ b/tool/ruby_vm/models/operands_unification.rb @@ -11,16 +11,16 @@ require_relative '../helpers/c_escape' require_relative '../loaders/opt_operand_def' -require_relative 'bare_instructions' +require_relative 'bare_instruction' -class RubyVM::OperandsUnifications < RubyVM::BareInstructions +class RubyVM::OperandsUnification < RubyVM::BareInstruction include RubyVM::CEscape attr_reader :preamble, :original, :spec def initialize opts = {} name = opts[:signature][0] - @original = RubyVM::BareInstructions.fetch name + @original = RubyVM::BareInstruction.find(name) template = @original.template parts = compose opts[:location], opts[:signature], template[:signature] json = template.dup @@ -129,12 +129,12 @@ def compose location, spec, template new h end - def self.to_a + def self.all @instances end def self.each_group - to_a.group_by(&:original).each_pair do |k, v| + all.group_by(&:original).each_pair do |k, v| yield k, v end end diff --git a/tool/ruby_vm/models/trace_instructions.rb b/tool/ruby_vm/models/trace_instruction.rb similarity index 90% rename from tool/ruby_vm/models/trace_instructions.rb rename to tool/ruby_vm/models/trace_instruction.rb index 5ffff3f63de1ce..2a5b67eaa2af01 100644 --- a/tool/ruby_vm/models/trace_instructions.rb +++ b/tool/ruby_vm/models/trace_instruction.rb @@ -10,9 +10,9 @@ # details. require_relative '../helpers/c_escape' -require_relative 'bare_instructions' +require_relative 'bare_instruction' -class RubyVM::TraceInstructions +class RubyVM::TraceInstruction include RubyVM::CEscape attr_reader :name @@ -66,9 +66,9 @@ def zjit_profile? @instances = RubyVM::Instructions.map {|i| new i } - def self.to_a + def self.all @instances end - RubyVM::Instructions.push(*to_a) + RubyVM::Instructions.push(*all) end diff --git a/tool/ruby_vm/models/zjit_instructions.rb b/tool/ruby_vm/models/zjit_instruction.rb similarity index 86% rename from tool/ruby_vm/models/zjit_instructions.rb rename to tool/ruby_vm/models/zjit_instruction.rb index 2bf190e3f744d6..dc1925b095d585 100644 --- a/tool/ruby_vm/models/zjit_instructions.rb +++ b/tool/ruby_vm/models/zjit_instruction.rb @@ -1,8 +1,8 @@ require_relative '../helpers/c_escape' -require_relative 'bare_instructions' +require_relative 'bare_instruction' # Profile YARV instructions to optimize code generated by ZJIT -class RubyVM::ZJITInstructions +class RubyVM::ZJITInstruction include RubyVM::CEscape attr_reader :name @@ -50,9 +50,9 @@ def has_attribute?(*) @instances = RubyVM::Instructions.filter(&:zjit_profile?).map {|i| new(i) } - def self.to_a + def self.all @instances end - RubyVM::Instructions.push(*to_a) + RubyVM::Instructions.push(*all) end diff --git a/tool/ruby_vm/views/_insn_leaf_info.erb b/tool/ruby_vm/views/_insn_leaf_info.erb index 79642b8f666cf5..f30366ffdac1ad 100644 --- a/tool/ruby_vm/views/_insn_leaf_info.erb +++ b/tool/ruby_vm/views/_insn_leaf_info.erb @@ -4,7 +4,7 @@ insn_leaf(int insn, const VALUE *opes) { switch (insn) { % RubyVM::Instructions.each do |insn| -% next if insn.is_a?(RubyVM::TraceInstructions) || insn.is_a?(RubyVM::ZJITInstructions) +% next if insn.is_a?(RubyVM::TraceInstruction) || insn.is_a?(RubyVM::ZJITInstruction) case <%= insn.bin %>: return attr_leaf_<%= insn.name %>(<%= insn.operands.map.with_index do |ope, i| diff --git a/tool/ruby_vm/views/_zjit_helpers.erb b/tool/ruby_vm/views/_zjit_helpers.erb index 97b9feffbc3106..1185dbd9d8ce9b 100644 --- a/tool/ruby_vm/views/_zjit_helpers.erb +++ b/tool/ruby_vm/views/_zjit_helpers.erb @@ -5,7 +5,7 @@ static int vm_bare_insn_to_zjit_insn(int insn) { switch (insn) { -% RubyVM::ZJITInstructions.to_a.each do |insn| +% RubyVM::ZJITInstruction.all.each do |insn| case BIN(<%= insn.jump_destination %>): return <%= insn.bin %>; % end @@ -19,7 +19,7 @@ static int vm_zjit_insn_to_bare_insn(int insn) { switch (insn) { -% RubyVM::ZJITInstructions.to_a.each do |insn| +% RubyVM::ZJITInstruction.all.each do |insn| case <%= insn.bin %>: return BIN(<%= insn.jump_destination %>); % end diff --git a/tool/ruby_vm/views/optinsn.inc.erb b/tool/ruby_vm/views/optinsn.inc.erb index de7bb210eae37d..9d9cf0a43aafe1 100644 --- a/tool/ruby_vm/views/optinsn.inc.erb +++ b/tool/ruby_vm/views/optinsn.inc.erb @@ -23,7 +23,7 @@ insn_operands_unification(INSN *iobj) /* do nothing */; break; -% RubyVM::OperandsUnifications.each_group do |orig, unifs| +% RubyVM::OperandsUnification.each_group do |orig, unifs| case <%= orig.bin %>: % unifs.each do |insn| @@ -56,7 +56,7 @@ rb_insn_unified_local_var_level(VALUE insn) switch (insn) { default: return -1; /* do nothing */; -% RubyVM::OperandsUnifications.each_group do |orig, unifs| +% RubyVM::OperandsUnification.each_group do |orig, unifs| % unifs.each do|insn| case <%= insn.bin %>: % insn.spec.map{|(var,val)|val}.reject{|i| i == '*' }.each do |val| diff --git a/tool/ruby_vm/views/vm.inc.erb b/tool/ruby_vm/views/vm.inc.erb index b92d6d31bcc562..38bf5f05ae7922 100644 --- a/tool/ruby_vm/views/vm.inc.erb +++ b/tool/ruby_vm/views/vm.inc.erb @@ -13,22 +13,22 @@ } -%> #include "vm_insnhelper.h" -% RubyVM::BareInstructions.to_a.each do |insn| +% RubyVM::BareInstruction.all.each do |insn| <%= render 'insn_entry', locals: { insn: insn } -%> % end % -% RubyVM::OperandsUnifications.to_a.each do |insn| +% RubyVM::OperandsUnification.all.each do |insn| <%= render 'insn_entry', locals: { insn: insn } -%> % end % -% RubyVM::InstructionsUnifications.to_a.each do |insn| +% RubyVM::InstructionsUnification.all.each do |insn| <%= render 'insn_entry', locals: { insn: insn } -%> % end % -% RubyVM::ZJITInstructions.to_a.each do |insn| +% RubyVM::ZJITInstruction.all.each do |insn| <%= render 'zjit_instruction', locals: { insn: insn } -%> % end % -% RubyVM::TraceInstructions.to_a.each do |insn| +% RubyVM::TraceInstruction.all.each do |insn| <%= render 'trace_instruction', locals: { insn: insn } -%> % end From 9401f4e7f28af7cb9e1476c4e513e67ca2a782cd Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Wed, 10 Sep 2025 21:35:17 +0100 Subject: [PATCH 5/8] Skip `vm_cc_invalidate`'s `cc->klass` assertion for multi-ractor for now It's failing more frequently on CI in Ractor tests lately. To make allow other PRs to land more smoothly, we can skip the assertion for now. --- vm_callinfo.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vm_callinfo.h b/vm_callinfo.h index e52b2f9b1ab8b3..80b5b9a0966c1e 100644 --- a/vm_callinfo.h +++ b/vm_callinfo.h @@ -565,7 +565,8 @@ vm_cc_invalidate(const struct rb_callcache *cc) { VM_ASSERT(IMEMO_TYPE_P(cc, imemo_callcache)); VM_ASSERT(cc != vm_cc_empty()); - VM_ASSERT(cc->klass != Qundef); // should be enable + // TODO: rb_multi_ractor_p() is a workaround to stabilize CI + VM_ASSERT(cc->klass != Qundef || rb_multi_ractor_p()); // should be enable *(VALUE *)&cc->klass = Qundef; RB_DEBUG_COUNTER_INC(cc_ent_invalidate); From bd4de1245f9899463a817d43863bcee520741e50 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Wed, 10 Sep 2025 14:18:44 -0700 Subject: [PATCH 6/8] Explicitly declare VM instruction dependencies (#14509) instead of mutating RubyVM::Instructions and letting the order of `require` impact its behavior. Now that we have both RubyVM::TraceInstruction and RubyVM::ZJITInstruction, it feels too much of a tight coupling to rely on `require` to be ordered properly. --- tool/ruby_vm/models/instructions.rb | 9 +++++---- tool/ruby_vm/models/trace_instruction.rb | 10 +++------- tool/ruby_vm/models/zjit_instruction.rb | 4 +--- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/tool/ruby_vm/models/instructions.rb b/tool/ruby_vm/models/instructions.rb index f2c6562d37ae2a..7be7064b1419f7 100644 --- a/tool/ruby_vm/models/instructions.rb +++ b/tool/ruby_vm/models/instructions.rb @@ -12,11 +12,12 @@ require_relative 'bare_instruction' require_relative 'operands_unification' require_relative 'instructions_unification' +require_relative 'trace_instruction' +require_relative 'zjit_instruction' RubyVM::Instructions = RubyVM::BareInstruction.all + RubyVM::OperandsUnification.all + - RubyVM::InstructionsUnification.all - -require_relative 'trace_instruction' -require_relative 'zjit_instruction' + RubyVM::InstructionsUnification.all + + RubyVM::TraceInstruction.all + + RubyVM::ZJITInstruction.all RubyVM::Instructions.freeze diff --git a/tool/ruby_vm/models/trace_instruction.rb b/tool/ruby_vm/models/trace_instruction.rb index 2a5b67eaa2af01..6a3ad53c44bf0d 100644 --- a/tool/ruby_vm/models/trace_instruction.rb +++ b/tool/ruby_vm/models/trace_instruction.rb @@ -58,17 +58,13 @@ def has_attribute? *; return false end - def zjit_profile? - return false - end - private - @instances = RubyVM::Instructions.map {|i| new i } + @instances = (RubyVM::BareInstruction.all + + RubyVM::OperandsUnification.all + + RubyVM::InstructionsUnification.all).map {|i| new(i) } def self.all @instances end - - RubyVM::Instructions.push(*all) end diff --git a/tool/ruby_vm/models/zjit_instruction.rb b/tool/ruby_vm/models/zjit_instruction.rb index dc1925b095d585..04764e4c613248 100644 --- a/tool/ruby_vm/models/zjit_instruction.rb +++ b/tool/ruby_vm/models/zjit_instruction.rb @@ -48,11 +48,9 @@ def has_attribute?(*) return false end - @instances = RubyVM::Instructions.filter(&:zjit_profile?).map {|i| new(i) } + @instances = RubyVM::BareInstruction.all.filter(&:zjit_profile?).map {|i| new(i) } def self.all @instances end - - RubyVM::Instructions.push(*all) end From 1b9fc426bbf6b6ca83ef6301f7a11f3fb7cc4f84 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Wed, 10 Sep 2025 17:04:11 -0400 Subject: [PATCH 7/8] YJIT: Remove dead code: `asm_comment!` checks `--yjit-dump-disasm` --- yjit/src/codegen.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index cd057359ac2427..2ab45f41dbf0b5 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -1094,11 +1094,7 @@ pub fn gen_entry_prologue( let code_ptr = cb.get_write_ptr(); let mut asm = Assembler::new(unsafe { get_iseq_body_local_table_size(iseq) }); - if get_option_ref!(dump_disasm).is_some() { - asm_comment!(asm, "YJIT entry point: {}", iseq_get_location(iseq, 0)); - } else { - asm_comment!(asm, "YJIT entry"); - } + asm_comment!(asm, "YJIT entry point: {}", iseq_get_location(iseq, 0)); asm.frame_setup(); From 8d05669bafc06717814b508e433f77836cf375dd Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Wed, 10 Sep 2025 17:02:21 -0400 Subject: [PATCH 8/8] YJIT: Print more disassembly in release builds These `#[cfg(feature = "disasm")]` were unnecessary and we can provide the information like ruby source location regardless of the availability of capstone. --- yjit/src/codegen.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 2ab45f41dbf0b5..2bac85e62b57af 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -821,11 +821,11 @@ fn gen_stub_exit(ocb: &mut OutlinedCb) -> Option { /// Generate an exit to return to the interpreter fn gen_exit(exit_pc: *mut VALUE, asm: &mut Assembler) { - #[cfg(all(feature = "disasm", not(test)))] - { + #[cfg(not(test))] + asm_comment!(asm, "exit to interpreter on {}", { let opcode = unsafe { rb_vm_insn_addr2opcode((*exit_pc).as_ptr()) }; - asm_comment!(asm, "exit to interpreter on {}", insn_name(opcode as usize)); - } + insn_name(opcode as usize) + }); if asm.ctx.is_return_landing() { asm.mov(SP, Opnd::mem(64, CFP, RUBY_OFFSET_CFP_SP)); @@ -1292,7 +1292,6 @@ pub fn gen_single_block( let mut asm = Assembler::new(jit.num_locals()); asm.ctx = ctx; - #[cfg(feature = "disasm")] if get_option_ref!(dump_disasm).is_some() { let blockid_idx = blockid.idx; let chain_depth = if asm.ctx.get_chain_depth() > 0 { format!("(chain_depth: {})", asm.ctx.get_chain_depth()) } else { "".to_string() }; @@ -9047,7 +9046,6 @@ fn gen_send_general( let recv_opnd: YARVOpnd = recv.into(); // Log the name of the method we're calling to - #[cfg(feature = "disasm")] asm_comment!(asm, "call to {}", get_method_name(Some(comptime_recv_klass), mid)); // Gather some statistics about sends