From ee3aebec41089cd1163e63eeaf0cfb9e5eb1e768 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Fri, 10 Jul 2026 17:19:12 -0700 Subject: [PATCH 1/8] ZJIT: Optimize polymorphic setivar --- zjit/src/codegen_tests.rs | 23 ++++ zjit/src/hir.rs | 192 +++++++++++++++++++++++++++------ zjit/src/hir/opt_tests.rs | 221 ++++++++++++++++++++++++++++++-------- zjit/src/stats.rs | 2 + 4 files changed, 360 insertions(+), 78 deletions(-) diff --git a/zjit/src/codegen_tests.rs b/zjit/src/codegen_tests.rs index 0d93b31e232090..a3fb2a8a916e78 100644 --- a/zjit/src/codegen_tests.rs +++ b/zjit/src/codegen_tests.rs @@ -4169,6 +4169,29 @@ fn test_setinstancevariable() { "), @"1"); } +#[test] +fn test_polymorphic_setinstancevariable_with_shape_transitions() { + set_call_threshold(3); + assert_snapshot!(inspect(r#" + class C + def set(value) = @a = value + end + + normal = C.new + with_b = C.new + with_b.instance_variable_set(:@b, true) + normal.set(:profile_normal) + with_b.set(:profile_with_b) + + normal = C.new + with_b = C.new + with_b.instance_variable_set(:@b, true) + results = [normal.set(:normal), with_b.set(:with_b)] + results << normal.instance_variable_get(:@a) + results << with_b.instance_variable_get(:@a) + "#), @"[:normal, :with_b, :normal, :with_b]"); +} + #[test] fn test_getclassvariable() { assert_snapshot!(inspect(" diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index cbc302320f6d61..cfa223a4d6bf98 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -573,6 +573,7 @@ pub enum SideExitReason { SendWhileTracing, NoProfileSend, NoProfileGetIvar, + NoProfileSetIvar, InvokeBlockNotIfunc, } @@ -2611,6 +2612,13 @@ struct CompilePolicy { no_side_exits: bool, } +#[derive(Debug, Clone, Copy)] +struct SetIvarSpec { + profiled_type: ProfiledType, + ivar_index: attr_index_t, + next_shape: ShapeId, +} + impl CompilePolicy { fn new(iseq: *const rb_iseq_t) -> Self { // When a previous version was invalidated and we've reached the version @@ -5250,9 +5258,9 @@ impl Function { Ok(self.load_ivar(block, self_val, profiled_type, id)) } - fn try_emit_optimized_setivar(&mut self, block: BlockId, self_val: InsnId, id: ID, val: InsnId, profiled_type: ProfiledType, state: InsnId, recompile: Option) -> Result<(), Counter> { + fn prepare_optimized_setivar(&mut self, id: ID, profiled_type: ProfiledType) -> Result { if profiled_type.flags().is_immediate() { - // Instance variable lookups on immediate values are always nil + // Instance variable writes on immediate values raise. return Err(Counter::setivar_fallback_immediate); } if !profiled_type.flags().is_t_object() { @@ -5268,63 +5276,69 @@ impl Function { // too-complex shapes can't use index access return Err(Counter::setivar_fallback_complex); } - if self.policy.no_side_exits { - // On the final version, skip GetIvar shape specialization. - // iseq_to_hir already generates polymorphic branches with a - // GetIvar C call fallback for getinstancevariable, so we don't - // need to wrap it again here. - return Err(Counter::setivar_fallback_no_side_exits); - } - let mut ivar_index: attr_index_t = 0; - let mut next_shape_id = profiled_type.shape(); + let mut next_shape = profiled_type.shape(); if !unsafe { rb_shape_get_iv_index(profiled_type.shape().0, id, &mut ivar_index) } { // Current shape does not contain this ivar; do a shape transition. let current_shape_id = profiled_type.shape(); let class = profiled_type.class(); // We're only looking at T_OBJECT so ignore all of the imemo stuff. assert!(profiled_type.flags().is_t_object()); - next_shape_id = ShapeId(unsafe { rb_shape_transition_add_ivar_no_warnings(current_shape_id.0, id, class) }); + next_shape = ShapeId(unsafe { rb_shape_transition_add_ivar_no_warnings(current_shape_id.0, id, class) }); // If the VM ran out of shapes, or this class generated too many leaf, // it may be de-optimized into OBJ_COMPLEX_SHAPE (hash-table). - let new_shape_complex = unsafe { rb_jit_shape_complex_p(next_shape_id.0) }; + let new_shape_complex = unsafe { rb_jit_shape_complex_p(next_shape.0) }; // TODO(max): Is it OK to bail out here after making a shape transition? if new_shape_complex { return Err(Counter::setivar_fallback_new_shape_complex); } - let ivar_result = unsafe { rb_shape_get_iv_index(next_shape_id.0, id, &mut ivar_index) }; + let ivar_result = unsafe { rb_shape_get_iv_index(next_shape.0, id, &mut ivar_index) }; assert!(ivar_result, "New shape must have the ivar index"); let current_capacity = unsafe { rb_jit_shape_capacity(current_shape_id.0) }; - let next_capacity = unsafe { rb_jit_shape_capacity(next_shape_id.0) }; + let next_capacity = unsafe { rb_jit_shape_capacity(next_shape.0) }; // If the new shape has a different capacity, or is COMPLEX, we'll have to // reallocate it. let needs_extension = next_capacity != current_capacity; if needs_extension { return Err(Counter::setivar_fallback_new_shape_needs_extension); } - // Fall through to emitting the ivar write + // Fall through to preparing the ivar write. } - let self_val = self.guard_heap(block, self_val, state); - let shape = self.load_shape(block, self_val); - self.guard_shape(block, shape, profiled_type.shape(), state, recompile); + + Ok(SetIvarSpec { profiled_type, ivar_index, next_shape }) + } + + fn emit_optimized_setivar(&mut self, block: BlockId, self_val: InsnId, id: ID, val: InsnId, spec: SetIvarSpec) { // Current shape contains this ivar - let (ivar_storage, offset) = if profiled_type.flags().is_embedded() { + let (ivar_storage, offset) = if spec.profiled_type.flags().is_embedded() { // See ROBJECT_FIELDS() from include/ruby/internal/core/robject.h - let offset = ROBJECT_OFFSET_AS_ARY + (SIZEOF_VALUE * ivar_index.to_usize()) as i32; + let offset = ROBJECT_OFFSET_AS_ARY + (SIZEOF_VALUE * spec.ivar_index.to_usize()) as i32; (self_val, offset) } else { let as_heap = self.load_field(block, self_val, FieldName::as_heap, ROBJECT_OFFSET_AS_HEAP_FIELDS, types::CPtr); - let offset = SIZEOF_VALUE_I32 * ivar_index as i32; + let offset = SIZEOF_VALUE_I32 * spec.ivar_index as i32; (as_heap, offset) }; self.push_insn(block, Insn::StoreField { recv: ivar_storage, id: id.into(), offset, val, num_bits: types::BasicObject.num_bits() }); self.push_insn(block, Insn::WriteBarrier { recv: self_val, val }); - if next_shape_id != profiled_type.shape() { + if spec.next_shape != spec.profiled_type.shape() { // Write the new shape ID - let shape_id = self.push_insn(block, Insn::Const { val: Const::CShape(next_shape_id) }); + let shape_id = self.push_insn(block, Insn::Const { val: Const::CShape(spec.next_shape) }); let shape_id_offset = unsafe { rb_shape_id_offset() }; self.push_insn(block, Insn::StoreField { recv: self_val, id: FieldName::shape_id, offset: shape_id_offset, val: shape_id, num_bits: types::CShape.num_bits() }); } + } + + fn try_emit_optimized_setivar(&mut self, block: BlockId, self_val: InsnId, id: ID, val: InsnId, profiled_type: ProfiledType, state: InsnId, recompile: Option) -> Result<(), Counter> { + if self.policy.no_side_exits { + // On the final version, don't add a shape guard without a fallback. + return Err(Counter::setivar_fallback_no_side_exits); + } + let spec = self.prepare_optimized_setivar(id, profiled_type)?; + let self_val = self.guard_heap(block, self_val, state); + let shape = self.load_shape(block, self_val); + self.guard_shape(block, shape, profiled_type.shape(), state, recompile); + self.emit_optimized_setivar(block, self_val, id, val, spec); Ok(()) } @@ -6933,6 +6947,82 @@ impl Function { } Some((join_block, result)) } + + /// Return Some(BlockId) if we generated a setivar or None if we only generated an + /// unconditional SideExit (in which case we should end the block). + fn dispatch_setivar( + &mut self, + specs: &[SetIvarSpec], + unoptimized_reason: Option, + mut block: BlockId, + insn_idx: u32, + self_param: InsnId, + id: ID, + ic: *const iseq_inline_iv_cache_entry, + val: InsnId, + exit_id: InsnId, + ) -> Option { + // 0 specs: Generate a recompile exit or a fallback. No need for new HIR blocks. + if specs.is_empty() { + if let Some(counter) = unoptimized_reason { + self.count(block, counter); + self.push_insn(block, Insn::SetIvar { self_val: self_param, id, ic, val, state: exit_id }); + return Some(block); + } else if self.policy.no_side_exits { + self.count(block, Counter::setivar_fallback_no_side_exits); + self.push_insn(block, Insn::SetIvar { self_val: self_param, id, ic, val, state: exit_id }); + return Some(block); + } else { + self.push_insn(block, Insn::SideExit { state: exit_id, reason: Box::new(SideExitReason::NoProfileSetIvar), recompile: Some(Recompile) }); + return None; + } + } + // 1 spec: Generate a monomorphic setivar with a guard if allowed by policy. No need for new HIR blocks. + if specs.len() == 1 && !self.policy.no_side_exits { + let actual = self.load_shape(block, self_param); + self.guard_shape(block, actual, specs[0].profiled_type.shape(), exit_id, Some(Recompile)); + self.emit_optimized_setivar(block, self_param, id, val, specs[0]); + return Some(block); + } + + // Otherwise, make HIR blocks to handle different shapes or a fallback, and let them jump to join_block. + let edge = |target: BlockId| BranchEdge { target, args: vec![] }; + let branch = |cond: InsnId, if_true: BlockId, if_false: BlockId| Insn::CondBranch { val: cond, if_true: edge(if_true), if_false: edge(if_false) }; + let actual = self.load_shape(block, self_param); + let last_shape_index = specs.len() - 1; + let join_block = self.new_block(insn_idx); + for (i, &spec) in specs.iter().enumerate() { + let store_optimized_block = self.new_block(insn_idx); + if i == last_shape_index { + if self.policy.no_side_exits { + // If the policy doesn't allow exits, make a fallback block and jump to it if the shape doesn't match. + let expected = self.push_insn(block, Insn::Const { val: Const::CShape(spec.profiled_type.shape()) }); + let matches = self.push_insn(block, Insn::IsBitEqual { left: actual, right: expected }); + let store_fallback_block = self.new_block(insn_idx); + self.push_insn(block, branch(matches, store_optimized_block, store_fallback_block)); + self.count(store_fallback_block, Counter::setivar_fallback_no_side_exits); + self.push_insn(store_fallback_block, Insn::SetIvar { self_val: self_param, id, ic, val, state: exit_id }); + self.push_insn(store_fallback_block, Insn::Jump(edge(join_block))); + } else { + // If the policy allows exits, exit to the interpreter if the shape doesn't match. + self.guard_shape(block, actual, spec.profiled_type.shape(), exit_id, Some(Recompile)); + // TODO(max): Don't make a new block in this case + self.push_insn(block, Insn::Jump(edge(store_optimized_block))); + } + } else { + // If this is not the last profiled shape, let the guard jump to the next block. + let expected = self.push_insn(block, Insn::Const { val: Const::CShape(spec.profiled_type.shape()) }); + let matches = self.push_insn(block, Insn::IsBitEqual { left: actual, right: expected }); + let next_block = self.new_block(insn_idx); + self.push_insn(block, branch(matches, store_optimized_block, next_block)); + block = next_block; + } + // Generate the optimized setivar for the profiled shape and jump to join_block. + self.emit_optimized_setivar(store_optimized_block, self_param, id, val, spec); + self.push_insn(store_optimized_block, Insn::Jump(edge(join_block))); + } + Some(join_block) + } } impl<'a> std::fmt::Display for FunctionPrinter<'a> { @@ -9161,19 +9251,53 @@ fn add_iseq_to_hir( break; // End the block } let val = state.stack_pop()?; - if let Some(profiled_type) = fun.monomorphic_summary(&profiles, self_param, exit_id) { - // TODO(max): Assert ic is never null - let recompile = (!ic.is_null()).then_some(Recompile); - fun.try_emit_optimized_setivar(block, self_param, id, val, profiled_type, exit_id, recompile).unwrap_or_else(|counter| { - fun.count(block, counter); - fun.push_insn(block, Insn::SetIvar { self_val: self_param, id, ic, val, state: exit_id }); - }); - } else { - fun.push_insn(block, Insn::SetIvar { self_val: self_param, id, ic, val, state: exit_id }); + let unrefined_self_param = self_param; + let summary = fun.profile_summary(&profiles, self_param, exit_id); + // Filter out profiled types we don't know how to optimize and de-duplicate shapes. + let mut seen_shapes = Vec::with_capacity(summary.buckets().len()); + let mut specs = Vec::with_capacity(summary.buckets().len()); + let mut unoptimized_reason = None; + for &profiled_type in summary.buckets() { + if profiled_type.is_empty() { + continue; + } + let shape = if profiled_type.flags().is_immediate() { + INVALID_SHAPE_ID + } else { + profiled_type.shape() + }; + if seen_shapes.contains(&shape) { + continue; + } + seen_shapes.push(shape); + match fun.prepare_optimized_setivar(id, profiled_type) { + Ok(spec) => specs.push(spec), + Err(counter) => { + unoptimized_reason.get_or_insert(counter); + }, + } + } + if !specs.is_empty() || unoptimized_reason.is_none() { + self_param = fun.guard_heap(block, self_param, exit_id); } + let Some(new_block) = fun.dispatch_setivar( + &specs, + unoptimized_reason, + block, + insn_idx, + self_param, + id, + ic, + val, + exit_id, + ) else { + // Side-exiting unconditionally; end the block. + break; + }; + block = new_block; // SetIvar will raise if self is an immediate. If it raises, we will have // exited JIT code. So upgrade the type within JIT code to a heap object. - self_param = fun.push_insn(block, Insn::RefineType { val: self_param, new_type: types::HeapBasicObject }); + self_param = fun.push_insn(block, Insn::RefineType { val: unrefined_self_param, new_type: types::HeapBasicObject }); } YARVINSN_getclassvariable => { let id = ID(get_arg(pc, 0).as_u64()); diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index b723d720812906..cabbc251f33f01 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -5752,9 +5752,8 @@ mod hir_opt_tests { bb3(v6:BasicObject): v10:Fixnum[1] = Const Value(1) PatchPoint SingleRactorMode - SetIvar v6, :@foo, v10 - CheckInterrupts - Return v10 + v14:HeapBasicObject = GuardType v6, HeapBasicObject + SideExit NoProfileSetIvar recompile "); } @@ -6455,6 +6454,30 @@ mod hir_opt_tests { "); } + #[test] + fn test_specialize_monomorphic_setivar_on_final_version() { + set_max_versions(2); + set_inline_threshold(0); + eval(" + class FinalSetIvar + def test(x) + @foo = 5 + x + 1 + end + end + + obj = FinalSetIvar.new + 30.times { obj.test(1) } + 30.times { obj.test(1.5) } + "); + + let hir = hir_string_proc("FinalSetIvar.new.method(:test)"); + assert!(hir.contains("CondBranch"), "{hir}"); + assert!(hir.contains("StoreField"), "{hir}"); + assert!(hir.contains("SetIvar"), "{hir}"); + assert!(!hir.contains("GuardBitEquals"), "{hir}"); + } + #[test] fn test_specialize_multiple_monomorphic_setivar_with_shape_transition() { eval(r#" @@ -6537,7 +6560,7 @@ mod hir_opt_tests { } #[test] - fn test_dont_specialize_polymorphic_setivar() { + fn test_specialize_polymorphic_setivar() { set_call_threshold(3); eval(" class C @@ -6564,7 +6587,23 @@ mod hir_opt_tests { bb3(v6:BasicObject): v10:Fixnum[5] = Const Value(5) PatchPoint SingleRactorMode - SetIvar v6, :@a, v10 + v14:HeapBasicObject = GuardType v6, HeapBasicObject + v15:CShape = LoadField v14, :shape_id@0x1000 + v16:CShape[0x1001] = Const CShape(0x1001) + v17:CBool = IsBitEqual v15, v16 + CondBranch v17, bb5(), bb6() + bb5(): + StoreField v14, :@a@0x1002, v10 + WriteBarrier v14, v10 + v21:CShape[0x1003] = Const CShape(0x1003) + StoreField v14, :shape_id@0x1000, v21 + Jump bb4() + bb6(): + v24:CShape[0x1004] = GuardBitEquals v15, CShape(0x1004) recompile + StoreField v14, :@a@0x1005, v10 + WriteBarrier v14, v10 + Jump bb4() + bb4(): CheckInterrupts Return v10 "); @@ -8735,9 +8774,10 @@ mod hir_opt_tests { #[test] fn test_setivar_shape_guard_recompile() { + set_max_versions(2); // Call with one shape to compile, then call with a different shape to - // trigger shape guard exits and recompilation. On the recompiled version, - // SetIvar stays as a C call fallback to avoid more shape guard exits. + // trigger shape guard exits and recompilation. The recompiled version + // specializes both profiled shapes. eval(" class C def initialize(extra = false) @@ -8767,7 +8807,20 @@ mod hir_opt_tests { bb3(v6:HeapBasicObject): v10:Fixnum[5] = Const Value(5) PatchPoint SingleRactorMode - SetIvar v6, :@foo, v10 + v15:CShape = LoadField v6, :shape_id@0x1000 + v16:CShape[0x1001] = Const CShape(0x1001) + v17:CBool = IsBitEqual v15, v16 + CondBranch v17, bb5(), bb6() + bb5(): + StoreField v6, :@foo@0x1002, v10 + WriteBarrier v6, v10 + Jump bb4() + bb6(): + v22:CShape[0x1003] = GuardBitEquals v15, CShape(0x1003) recompile + StoreField v6, :@foo@0x1004, v10 + WriteBarrier v6, v10 + Jump bb4() + bb4(): CheckInterrupts Return v10 "); @@ -14777,17 +14830,33 @@ mod hir_opt_tests { CheckInterrupts SetLocal :formatted, l0, EP@3, v21 PatchPoint SingleRactorMode - SetIvar v20, :@formatted, v21 - v52:ClassSubclass[VMFrozenCore] = Const Value(VALUE(0x1008)) - PatchPoint MethodRedefined(Class@0x1010, lambda@0x1018, cme:0x1020) - v68:BasicObject = CCallWithFrame v52, :RubyVM::FrozenCore.lambda@0x1048, block=0x1050 - v55:CPtr = GetEP 0 - v56:BasicObject = LoadField v55, :a@0x1001 - v57:BasicObject = LoadField v55, :_b@0x1002 - v58:BasicObject = LoadField v55, :_c@0x1003 - v59:BasicObject = LoadField v55, :formatted@0x1004 + v48:HeapBasicObject = GuardType v20, HeapBasicObject + v49:CShape = LoadField v48, :shape_id@0x1005 + v50:CShape[0x1006] = Const CShape(0x1006) + v51:CBool = IsBitEqual v49, v50 + CondBranch v51, bb7(), bb8() + bb7(): + StoreField v48, :@formatted@0x1007, v21 + WriteBarrier v48, v21 + Jump bb6() + bb8(): + v56:CShape[0x1008] = GuardBitEquals v49, CShape(0x1008) recompile + StoreField v48, :@formatted@0x1007, v21 + WriteBarrier v48, v21 + v60:CShape[0x1006] = Const CShape(0x1006) + StoreField v48, :shape_id@0x1005, v60 + Jump bb6() + bb6(): + v66:ClassSubclass[VMFrozenCore] = Const Value(VALUE(0x1010)) + PatchPoint MethodRedefined(Class@0x1018, lambda@0x1020, cme:0x1028) + v82:BasicObject = CCallWithFrame v66, :RubyVM::FrozenCore.lambda@0x1050, block=0x1058 + v69:CPtr = GetEP 0 + v70:BasicObject = LoadField v69, :a@0x1001 + v71:BasicObject = LoadField v69, :_b@0x1002 + v72:BasicObject = LoadField v69, :_c@0x1003 + v73:BasicObject = LoadField v69, :formatted@0x1004 CheckInterrupts - Return v68 + Return v82 "); } @@ -18141,16 +18210,48 @@ mod hir_opt_tests { bb3(v11:HeapBasicObject, v12:BasicObject, v13:NilClass): v17:Fixnum[1] = Const Value(1) PatchPoint SingleRactorMode + v21:CShape = LoadField v11, :shape_id@0x1001 + v22:CShape[0x1002] = Const CShape(0x1002) + v23:CBool = IsBitEqual v21, v22 + CondBranch v23, bb5(), bb6() + bb5(): + StoreField v11, :@a@0x1003, v17 + WriteBarrier v11, v17 + Jump bb4() + bb6(): + v28:CShape[0x1004] = Const CShape(0x1004) + v29:CBool = IsBitEqual v21, v28 + CondBranch v29, bb7(), bb8() + bb7(): + StoreField v11, :@a@0x1003, v17 + WriteBarrier v11, v17 + v35:CShape[0x1002] = Const CShape(0x1002) + StoreField v11, :shape_id@0x1001, v35 + Jump bb4() + bb8(): SetIvar v11, :@a, v17 + Jump bb4() + bb4(): PatchPoint NoEPEscape(f) - v27:Fixnum[1] = Const Value(1) + v44:Fixnum[1] = Const Value(1) PatchPoint MethodRedefined(Integer@0x1008, +@0x1010, cme:0x1018) - v46:Fixnum = GuardType v12, Fixnum recompile - v47:Fixnum = FixnumAdd v46, v27 + v72:Fixnum = GuardType v12, Fixnum recompile + v73:Fixnum = FixnumAdd v72, v44 PatchPoint SingleRactorMode - SetIvar v11, :@a, v47 + v55:CShape = LoadField v11, :shape_id@0x1001 + v56:CShape[0x1002] = Const CShape(0x1002) + v57:CBool = IsBitEqual v55, v56 + CondBranch v57, bb10(), bb11() + bb10(): + StoreField v11, :@a@0x1003, v73 + WriteBarrier v11, v73 + Jump bb9() + bb11(): + SetIvar v11, :@a, v73 + Jump bb9() + bb9(): CheckInterrupts - Return v47 + Return v73 fn f@:4: bb1(): @@ -18169,34 +18270,66 @@ mod hir_opt_tests { bb3(v11:HeapBasicObject, v12:BasicObject, v13:NilClass): v17:Fixnum[1] = Const Value(1) PatchPoint SingleRactorMode - SetIvar v11, :@a, v17 - PatchPoint NoEPEscape(f) - v27:Fixnum[1] = Const Value(1) - v31:CBool = HasType v12, Flonum - CondBranch v31, bb5(), bb6() + v21:CShape = LoadField v11, :shape_id@0x1001 + v22:CShape[0x1002] = Const CShape(0x1002) + v23:CBool = IsBitEqual v21, v22 + CondBranch v23, bb5(), bb6() bb5(): - v34:Flonum = RefineType v12, Flonum - PatchPoint MethodRedefined(Float@0x1008, +@0x1010, cme:0x1018) - v60:Float = FloatAdd v34, v27 - Jump bb4(v60) + StoreField v11, :@a@0x1003, v17 + WriteBarrier v11, v17 + Jump bb4() bb6(): - v37:CBool = HasType v12, Fixnum - CondBranch v37, bb7(), bb8() + v28:CShape[0x1004] = Const CShape(0x1004) + v29:CBool = IsBitEqual v21, v28 + CondBranch v29, bb7(), bb8() bb7(): - v40:Fixnum = RefineType v12, Fixnum - PatchPoint MethodRedefined(Integer@0x1040, +@0x1010, cme:0x1048) - v63:Fixnum = FixnumAdd v40, v27 - Jump bb4(v63) + StoreField v11, :@a@0x1003, v17 + WriteBarrier v11, v17 + v35:CShape[0x1002] = Const CShape(0x1002) + StoreField v11, :shape_id@0x1001, v35 + Jump bb4() bb8(): + SetIvar v11, :@a, v17 + Jump bb4() + bb4(): + PatchPoint NoEPEscape(f) + v44:Fixnum[1] = Const Value(1) + v48:CBool = HasType v12, Flonum + CondBranch v48, bb10(), bb11() + bb10(): + v51:Flonum = RefineType v12, Flonum PatchPoint MethodRedefined(Float@0x1008, +@0x1010, cme:0x1018) - v66:Flonum = GuardType v12, Flonum recompile - v67:Float = FloatAdd v66, v27 - Jump bb4(v67) - bb4(v30:Float|Fixnum): + v86:Float = FloatAdd v51, v44 + Jump bb9(v86) + bb11(): + v54:CBool = HasType v12, Fixnum + CondBranch v54, bb12(), bb13() + bb12(): + v57:Fixnum = RefineType v12, Fixnum + PatchPoint MethodRedefined(Integer@0x1040, +@0x1010, cme:0x1048) + v89:Fixnum = FixnumAdd v57, v44 + Jump bb9(v89) + bb13(): + PatchPoint MethodRedefined(Float@0x1008, +@0x1010, cme:0x1018) + v92:Flonum = GuardType v12, Flonum recompile + v93:Float = FloatAdd v92, v44 + Jump bb9(v93) + bb9(v47:Float|Fixnum): PatchPoint SingleRactorMode - SetIvar v11, :@a, v30 + v69:CShape = LoadField v11, :shape_id@0x1001 + v70:CShape[0x1002] = Const CShape(0x1002) + v71:CBool = IsBitEqual v69, v70 + CondBranch v71, bb15(), bb16() + bb15(): + StoreField v11, :@a@0x1003, v47 + WriteBarrier v11, v47 + Jump bb14() + bb16(): + SetIvar v11, :@a, v47 + Jump bb14() + bb14(): CheckInterrupts - Return v30 + Return v47 "); } diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 7468f96a5eab5f..51327153ae19b6 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -238,6 +238,7 @@ make_counters! { exit_too_many_args_for_lir, exit_no_profile_send, exit_no_profile_getivar, + exit_no_profile_setivar, exit_splatkw_not_nil_or_hash, exit_splatkw_polymorphic, exit_splatkw_not_profiled, @@ -666,6 +667,7 @@ pub fn side_exit_counter(reason: crate::hir::SideExitReason) -> Counter { SendWhileTracing => exit_send_while_tracing, NoProfileSend => exit_no_profile_send, NoProfileGetIvar => exit_no_profile_getivar, + NoProfileSetIvar => exit_no_profile_setivar, InvokeBlockNotIfunc => exit_invokeblock_not_ifunc, } } From 10a5e5d370cb53f9baaf8a50993d624f066f685a Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Fri, 10 Jul 2026 17:24:18 -0700 Subject: [PATCH 2/8] ZJIT: Share getivar and setivar dispatch --- zjit/src/hir.rs | 184 +++++++++++++++++++++++++----------------------- 1 file changed, 94 insertions(+), 90 deletions(-) diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index cfa223a4d6bf98..4a4b386d48ab9d 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -6876,85 +6876,121 @@ impl Function { Ok(()) } - /// Return Some(InsnId) if we generated any code to load an ivar and None if we only generated - /// an unconditional SideExit (in which case we should end the block). - fn dispatch_getivar( + /// Dispatch an ivar access to profiled shapes. Callbacks generate the optimized access and + /// generic fallback, optionally returning a value to pass through the join block. + fn dispatch_ivar( &mut self, - profiled_types: &Vec, + profiles: &[T], mut block: BlockId, insn_idx: u32, self_param: InsnId, - id: ID, - ic: *const iseq_inline_iv_cache_entry, exit_id: InsnId, - ) -> Option<(BlockId, InsnId)> { - // 0 profiled_types: Generate a recompile exit or a fallback. No need for new HIR blocks. - if profiled_types.is_empty() { + no_profile_reason: SideExitReason, + fallback_counter: Counter, + has_result: bool, + profile_shape: impl Fn(T) -> ShapeId, + mut emit_optimized: impl FnMut(&mut Function, BlockId, T) -> Option, + mut emit_fallback: impl FnMut(&mut Function, BlockId) -> Option, + ) -> Option<(BlockId, Option)> { + // 0 profiles: Generate a recompile exit or a fallback. No need for new HIR blocks. + if profiles.is_empty() { if self.policy.no_side_exits { - self.count(block, Counter::getivar_fallback_no_side_exits); - let result = self.push_insn(block, Insn::GetIvar { self_val: self_param, id, ic, state: exit_id }); + self.count(block, fallback_counter); + let result = emit_fallback(self, block); + assert_eq!(has_result, result.is_some()); return Some((block, result)); } else { - self.push_insn(block, Insn::SideExit { state: exit_id, reason: Box::new(SideExitReason::NoProfileGetIvar), recompile: Some(Recompile) }); + self.push_insn(block, Insn::SideExit { state: exit_id, reason: Box::new(no_profile_reason), recompile: Some(Recompile) }); return None; } } - // 1 profiled_types: Generate a monomorphic getivar with a guard if allowed by policy. No need for new HIR blocks. - if profiled_types.len() == 1 && !self.policy.no_side_exits { - // 0 profiled_types: Generate a recompile exit or a fallback. + // 1 profile: Generate a monomorphic ivar access with a guard if allowed by policy. No need for new HIR blocks. + if profiles.len() == 1 && !self.policy.no_side_exits { let actual = self.load_shape(block, self_param); - self.guard_shape(block, actual, profiled_types[0].shape(), exit_id, Some(Recompile)); - let result = self.load_ivar(block, self_param, profiled_types[0], id); + self.guard_shape(block, actual, profile_shape(profiles[0]), exit_id, Some(Recompile)); + let result = emit_optimized(self, block, profiles[0]); + assert_eq!(has_result, result.is_some()); return Some((block, result)); } // Otherwise, make HIR blocks to handle different shapes or a fallback, and let them jump to join_block. let edge = |target: BlockId| BranchEdge { target, args: vec![] }; let branch = |cond: InsnId, if_true: BlockId, if_false: BlockId| Insn::CondBranch { val: cond, if_true: edge(if_true), if_false: edge(if_false) }; + let result_edge = |target: BlockId, result: Option| { + assert_eq!(has_result, result.is_some()); + BranchEdge { target, args: result.into_iter().collect() } + }; let actual = self.load_shape(block, self_param); - let last_shape_index = profiled_types.len() - 1; + let last_shape_index = profiles.len() - 1; let join_block = self.new_block(insn_idx); - let result = self.push_insn(join_block, Insn::Param); - for (i, &profiled_type) in profiled_types.iter().enumerate() { - let load_optimized_block = self.new_block(insn_idx); + let result = has_result.then(|| self.push_insn(join_block, Insn::Param)); + for (i, &profile) in profiles.iter().enumerate() { + let optimized_block = self.new_block(insn_idx); if i == last_shape_index { if self.policy.no_side_exits { // If the policy doesn't allow exits, make a fallback block and jump to it if the shape doesn't match. - let expected = self.push_insn(block, Insn::Const { val: Const::CShape(profiled_type.shape()) }); - let val = self.push_insn(block, Insn::IsBitEqual { left: actual, right: expected }); - let load_fallback_block = self.new_block(insn_idx); - self.push_insn(block, branch(val, load_optimized_block, load_fallback_block)); - self.count(load_fallback_block, Counter::getivar_fallback_no_side_exits); - let result = self.push_insn(load_fallback_block, Insn::GetIvar { self_val: self_param, id, ic, state: exit_id }); - self.push_insn(load_fallback_block, Insn::Jump(BranchEdge { target: join_block, args: vec![result] })); + let expected = self.push_insn(block, Insn::Const { val: Const::CShape(profile_shape(profile)) }); + let matches = self.push_insn(block, Insn::IsBitEqual { left: actual, right: expected }); + let fallback_block = self.new_block(insn_idx); + self.push_insn(block, branch(matches, optimized_block, fallback_block)); + self.count(fallback_block, fallback_counter); + let fallback_result = emit_fallback(self, fallback_block); + self.push_insn(fallback_block, Insn::Jump(result_edge(join_block, fallback_result))); } else { // If the policy allows exits, exit to the interpreter if the shape doesn't match. - self.guard_shape(block, actual, profiled_type.shape(), exit_id, Some(Recompile)); + self.guard_shape(block, actual, profile_shape(profile), exit_id, Some(Recompile)); // TODO(max): Don't make a new block in this case - self.push_insn(block, Insn::Jump(BranchEdge { target: load_optimized_block, args: vec![] })); + self.push_insn(block, Insn::Jump(edge(optimized_block))); } } else { // If this is not the last profiled shape, let the guard jump to the next block. - let expected = self.push_insn(block, Insn::Const { val: Const::CShape(profiled_type.shape()) }); - let val = self.push_insn(block, Insn::IsBitEqual { left: actual, right: expected}); + let expected = self.push_insn(block, Insn::Const { val: Const::CShape(profile_shape(profile)) }); + let matches = self.push_insn(block, Insn::IsBitEqual { left: actual, right: expected }); let next_block = self.new_block(insn_idx); - self.push_insn(block, branch(val, load_optimized_block, next_block)); + self.push_insn(block, branch(matches, optimized_block, next_block)); block = next_block; } - // Generate the optimized getivar for the profiled_type and jump to join_block - let result = self.load_ivar(load_optimized_block, self_param, profiled_type, id); - self.push_insn(load_optimized_block, Insn::Jump(BranchEdge { target: join_block, args: vec![result] })); + let optimized_result = emit_optimized(self, optimized_block, profile); + self.push_insn(optimized_block, Insn::Jump(result_edge(join_block, optimized_result))); } Some((join_block, result)) } + /// Return Some(InsnId) if we generated any code to load an ivar and None if we only generated + /// an unconditional SideExit (in which case we should end the block). + fn dispatch_getivar( + &mut self, + profiled_types: &[ProfiledType], + block: BlockId, + insn_idx: u32, + self_param: InsnId, + id: ID, + ic: *const iseq_inline_iv_cache_entry, + exit_id: InsnId, + ) -> Option<(BlockId, InsnId)> { + let (block, result) = self.dispatch_ivar( + profiled_types, + block, + insn_idx, + self_param, + exit_id, + SideExitReason::NoProfileGetIvar, + Counter::getivar_fallback_no_side_exits, + true, + |profiled_type| profiled_type.shape(), + |fun, block, profiled_type| Some(fun.load_ivar(block, self_param, profiled_type, id)), + |fun, block| Some(fun.push_insn(block, Insn::GetIvar { self_val: self_param, id, ic, state: exit_id })), + )?; + Some((block, result.unwrap())) + } + /// Return Some(BlockId) if we generated a setivar or None if we only generated an /// unconditional SideExit (in which case we should end the block). fn dispatch_setivar( &mut self, specs: &[SetIvarSpec], unoptimized_reason: Option, - mut block: BlockId, + block: BlockId, insn_idx: u32, self_param: InsnId, id: ID, @@ -6962,66 +6998,34 @@ impl Function { val: InsnId, exit_id: InsnId, ) -> Option { - // 0 specs: Generate a recompile exit or a fallback. No need for new HIR blocks. if specs.is_empty() { if let Some(counter) = unoptimized_reason { self.count(block, counter); self.push_insn(block, Insn::SetIvar { self_val: self_param, id, ic, val, state: exit_id }); return Some(block); - } else if self.policy.no_side_exits { - self.count(block, Counter::setivar_fallback_no_side_exits); - self.push_insn(block, Insn::SetIvar { self_val: self_param, id, ic, val, state: exit_id }); - return Some(block); - } else { - self.push_insn(block, Insn::SideExit { state: exit_id, reason: Box::new(SideExitReason::NoProfileSetIvar), recompile: Some(Recompile) }); - return None; } } - // 1 spec: Generate a monomorphic setivar with a guard if allowed by policy. No need for new HIR blocks. - if specs.len() == 1 && !self.policy.no_side_exits { - let actual = self.load_shape(block, self_param); - self.guard_shape(block, actual, specs[0].profiled_type.shape(), exit_id, Some(Recompile)); - self.emit_optimized_setivar(block, self_param, id, val, specs[0]); - return Some(block); - } - - // Otherwise, make HIR blocks to handle different shapes or a fallback, and let them jump to join_block. - let edge = |target: BlockId| BranchEdge { target, args: vec![] }; - let branch = |cond: InsnId, if_true: BlockId, if_false: BlockId| Insn::CondBranch { val: cond, if_true: edge(if_true), if_false: edge(if_false) }; - let actual = self.load_shape(block, self_param); - let last_shape_index = specs.len() - 1; - let join_block = self.new_block(insn_idx); - for (i, &spec) in specs.iter().enumerate() { - let store_optimized_block = self.new_block(insn_idx); - if i == last_shape_index { - if self.policy.no_side_exits { - // If the policy doesn't allow exits, make a fallback block and jump to it if the shape doesn't match. - let expected = self.push_insn(block, Insn::Const { val: Const::CShape(spec.profiled_type.shape()) }); - let matches = self.push_insn(block, Insn::IsBitEqual { left: actual, right: expected }); - let store_fallback_block = self.new_block(insn_idx); - self.push_insn(block, branch(matches, store_optimized_block, store_fallback_block)); - self.count(store_fallback_block, Counter::setivar_fallback_no_side_exits); - self.push_insn(store_fallback_block, Insn::SetIvar { self_val: self_param, id, ic, val, state: exit_id }); - self.push_insn(store_fallback_block, Insn::Jump(edge(join_block))); - } else { - // If the policy allows exits, exit to the interpreter if the shape doesn't match. - self.guard_shape(block, actual, spec.profiled_type.shape(), exit_id, Some(Recompile)); - // TODO(max): Don't make a new block in this case - self.push_insn(block, Insn::Jump(edge(store_optimized_block))); - } - } else { - // If this is not the last profiled shape, let the guard jump to the next block. - let expected = self.push_insn(block, Insn::Const { val: Const::CShape(spec.profiled_type.shape()) }); - let matches = self.push_insn(block, Insn::IsBitEqual { left: actual, right: expected }); - let next_block = self.new_block(insn_idx); - self.push_insn(block, branch(matches, store_optimized_block, next_block)); - block = next_block; - } - // Generate the optimized setivar for the profiled shape and jump to join_block. - self.emit_optimized_setivar(store_optimized_block, self_param, id, val, spec); - self.push_insn(store_optimized_block, Insn::Jump(edge(join_block))); - } - Some(join_block) + let (block, result) = self.dispatch_ivar( + specs, + block, + insn_idx, + self_param, + exit_id, + SideExitReason::NoProfileSetIvar, + Counter::setivar_fallback_no_side_exits, + false, + |spec| spec.profiled_type.shape(), + |fun, block, spec| { + fun.emit_optimized_setivar(block, self_param, id, val, spec); + None + }, + |fun, block| { + fun.push_insn(block, Insn::SetIvar { self_val: self_param, id, ic, val, state: exit_id }); + None + }, + )?; + assert!(result.is_none()); + Some(block) } } From fc8b340e3c89db7c5e7ad6b0d890c98baac916f5 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Sun, 12 Jul 2026 19:02:09 +0900 Subject: [PATCH 3/8] Fix mk_builtin_loader.rb for CRLF source checkouts dump_ast reports prism byte offsets against the raw file bytes, which are authoritative, but the loader read the source in text mode, so on Windows a CRLF checkout shifted every offset and produced wrong line numbers. The broken line numbers missed the LOCALS_DB lookup, cexpr! locals were silently dropped, and the generated inline functions failed to compile (error C2065). Read the source in binary mode and normalize CRLF in extracted C snippets so that CRLF and LF checkouts generate identical rbinc files. Co-Authored-By: Claude Fable 5 --- tool/mk_builtin_loader.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tool/mk_builtin_loader.rb b/tool/mk_builtin_loader.rb index a84f322e84df8f..51cccc0abb85f5 100644 --- a/tool/mk_builtin_loader.rb +++ b/tool/mk_builtin_loader.rb @@ -116,7 +116,9 @@ def visit_call_node(source, node, name, locals, requires, bs, inlines) # an inline function, and then continue the visit. raise "argc (#{argc}) of inline! should be 1" if argc != 1 - text = extract_string_literal(args[0]).rstrip + # The string literal keeps the line endings of the source file; normalize + # CRLF so the generated code does not depend on how it was checked out. + text = extract_string_literal(args[0]).gsub(/\r\n/, "\n").rstrip lineno = line_number(source, node) case primitive_macro @@ -184,7 +186,10 @@ def collect_builtins(dump_ast, file) exit(1) end - source = File.read(file) + # dump_ast reports byte offsets against the raw file bytes, so the source + # must be read in binary mode; text mode would convert CRLF to LF on Windows + # and shift every offset. + source = File.binread(file) root = JSON.parse(stdout) visit_node(source, root, "top", nil, requires = [], builtins = [], inlines = []) From 3590e1457849e26190d99559faf508c51dbbd073 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Mon, 13 Jul 2026 13:59:44 +0900 Subject: [PATCH 4/8] Use String#encode(universal_newline:) for line-ending normalization The universal newline decorator states the intent more clearly than a regexp substitution and also folds a lone CR. --- tool/mk_builtin_loader.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tool/mk_builtin_loader.rb b/tool/mk_builtin_loader.rb index 51cccc0abb85f5..7c93cccd6a10df 100644 --- a/tool/mk_builtin_loader.rb +++ b/tool/mk_builtin_loader.rb @@ -117,8 +117,8 @@ def visit_call_node(source, node, name, locals, requires, bs, inlines) raise "argc (#{argc}) of inline! should be 1" if argc != 1 # The string literal keeps the line endings of the source file; normalize - # CRLF so the generated code does not depend on how it was checked out. - text = extract_string_literal(args[0]).gsub(/\r\n/, "\n").rstrip + # them so the generated code does not depend on how it was checked out. + text = extract_string_literal(args[0]).encode(universal_newline: true).rstrip lineno = line_number(source, node) case primitive_macro From b5b586ba16d4f3cbc90d71510c942542f6911417 Mon Sep 17 00:00:00 2001 From: nozomemein Date: Sat, 4 Jul 2026 09:13:05 +0900 Subject: [PATCH 5/8] ZJIT: Specialize send with param_rest As a first step toward optimizing sends with complex argument passing, allow SendDirect for callees with *rest by packing the positional arguments into a rest array before the call. Track the selected optional argument JIT entry as jit_entry_idx explicitly so packed rest arguments do not confuse entry-point selection for optional, post, and keyword parameter layouts. Rest array allocation elision is left for a further follow-up, since it requires proving that the rest array does not escape. --- zjit/src/codegen.rs | 24 +--- zjit/src/codegen_tests.rs | 90 ++++++++++++ zjit/src/hir.rs | 171 ++++++++++++++++++----- zjit/src/hir/opt_tests.rs | 284 ++++++++++++++++++++++++++++++++++++-- zjit/src/stats.rs | 4 +- 5 files changed, 502 insertions(+), 71 deletions(-) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 970f6103061d6c..20ff39339b7352 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -648,11 +648,11 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio &Insn::Send { cd, block: Some(BlockHandler::BlockArg), state, reason, .. } => gen_send(jit, asm, function, cd, std::ptr::null(), &function.frame_state(state), reason), &Insn::SendForward { cd, blockiseq, state, reason, .. } => gen_send_forward(jit, asm, function, cd, blockiseq, &function.frame_state(state), reason), Insn::SendDirect(insn) => { - let SendDirectData { cme, iseq, recv, args, kw_bits, block, state, .. } = &**insn; + let SendDirectData { cme, iseq, recv, args, kw_bits, jit_entry_idx, block, state, .. } = &**insn; gen_send_iseq_direct( cb, jit, asm, function, *cme, *iseq, opnd!(recv), opnds!(args), - *kw_bits, &function.frame_state(*state), *block, + *kw_bits, *jit_entry_idx, &function.frame_state(*state), *block, ) } Insn::PushInlineFrame { cme, iseq, recv, args, blockiseq, state, .. } => { @@ -1685,6 +1685,7 @@ fn gen_send_iseq_direct( recv: Opnd, args: Vec, kw_bits: u32, + jit_entry_idx: u16, state: &FrameState, block: Option, ) -> lir::Opnd { @@ -1788,25 +1789,8 @@ fn gen_send_iseq_direct( } } - let num_optionals_passed = if params.flags.has_opt() != 0 { - // See vm_call_iseq_setup_normal_opt_start in vm_inshelper.c - let lead_num = params.lead_num as u32; - let opt_num = params.opt_num as u32; - let post_num = params.post_num as u32; - let keyword = params.keyword; - let kw_total_num = if keyword.is_null() { 0 } else { unsafe { (*keyword).num } } as u32; - assert!(args.len() as u32 <= lead_num + opt_num + post_num + kw_total_num); - // For computing optional positional entry point, only count positional args - // and exclude the always-present lead and post slots. - let positional_argc = args.len() as u32 - kw_total_num; - let num_optionals_passed = positional_argc.saturating_sub(lead_num + post_num); - num_optionals_passed - } else { - 0 - }; - // Make a method call. The target address will be rewritten once compiled. - let iseq_call = IseqCall::new(iseq, num_optionals_passed.try_into().expect("checked in HIR"), args.len().try_into().expect("checked in HIR")); + let iseq_call = IseqCall::new(iseq, jit_entry_idx, args.len().try_into().expect("checked in HIR")); let dummy_ptr = cb.get_write_ptr().raw_ptr(cb); jit.iseq_calls.push(iseq_call.clone()); let ret = asm.ccall_with_iseq_call(dummy_ptr, c_args, &iseq_call); diff --git a/zjit/src/codegen_tests.rs b/zjit/src/codegen_tests.rs index a3fb2a8a916e78..09faf5814daa96 100644 --- a/zjit/src/codegen_tests.rs +++ b/zjit/src/codegen_tests.rs @@ -974,6 +974,96 @@ fn test_send_optional_arguments() { "), @"[[1, 2], [3, 4]]"); } +#[test] +fn test_send_rest_arguments() { + eval(" + def test(*args) = args + def entry = test(1, 2, 3) + entry + "); + assert_snapshot!(assert_compiles("entry"), @"[1, 2, 3]"); +} + +#[test] +fn test_send_many_rest_arguments() { + eval(" + def test(*args) = args.length + def entry = test(1, 2, 3, 4, 5, 6, 7) + entry + "); + assert_snapshot!(assert_compiles("entry"), @"7"); +} + +#[test] +fn test_send_rest_arguments_with_post() { + eval(" + def test(a, *args, z) = [a, args, z] + def entry = test(1, 2, 3, 4) + entry + "); + assert_snapshot!(assert_compiles("entry"), @"[1, [2, 3], 4]"); +} + +#[test] +fn test_send_rest_arguments_with_keyword() { + eval(" + def test(*args, k:) = [args, k] + def entry = test(1, 2, k: 40) + entry + "); + assert_snapshot!(assert_compiles("entry"), @"[[1, 2], 40]"); +} + +#[test] +fn test_send_rest_arguments_with_optional_keyword_default() { + eval(" + def test(*args, k: 40) = [args, k] + def entry = test(1, 2) + entry + "); + assert_snapshot!(assert_compiles("entry"), @"[[1, 2], 40]"); +} + +#[test] +fn test_send_optional_and_rest_arguments() { + eval(" + def test(a, b = 2, *rest) = [a, b, rest] + def entry = [test(1), test(3, 4), test(5, 6, 7, 8)] + entry + "); + assert_snapshot!(assert_compiles("entry"), @"[[1, 2, []], [3, 4, []], [5, 6, [7, 8]]]"); +} + +#[test] +fn test_send_rest_arguments_with_keyword_to_positional_hash() { + eval(" + def test(*args) = args + def entry = test(k: 1) + entry + "); + assert_snapshot!(assert_compiles("entry"), @"[{k: 1}]"); +} + +#[test] +fn test_send_rest_arguments_with_block_literal() { + eval(" + def test(*args) = yield args.length + def entry = test(1, 2, 3) { |n| n + 4 } + entry + "); + assert_snapshot!(assert_compiles("entry"), @"7"); +} + +#[test] +fn test_send_rest_arguments_with_block_param() { + eval(" + def test(*args, &block) = block.call(args.length) + def entry = test(1, 2, 3) { |n| n + 5 } + entry + "); + assert_snapshot!(assert_compiles("entry"), @"8"); +} + #[test] fn test_send_nil_block_arg() { assert_snapshot!(inspect(" diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 4a4b386d48ab9d..95d9d05a389123 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -876,6 +876,7 @@ pub struct SendDirectData { pub iseq: IseqPtr, pub args: Vec, pub kw_bits: u32, + pub jit_entry_idx: u16, pub block: Option, pub state: InsnId, } @@ -2079,10 +2080,13 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> { Insn::Jump(target) => { write!(f, "Jump {target}") } Insn::CondBranch { val, if_true, if_false } => { write!(f, "CondBranch {val}, {if_true}, {if_false}") }, Insn::SendDirect(insn) => { - let SendDirectData { recv, cme, iseq, args, block, .. } = &**insn; + let SendDirectData { recv, cme, iseq, args, block, jit_entry_idx, .. } = &**insn; let blockiseq = block.map(|bh| match bh { BlockHandler::BlockIseq(iseq) => iseq, BlockHandler::BlockArg => unreachable!() }); let method_name = unsafe { (**cme).called_id }; write!(f, "SendDirect {recv}, {:p}, :{} ({:?})", self.ptr_map.map_ptr(&blockiseq), method_name, self.ptr_map.map_ptr(iseq))?; + if *jit_entry_idx != 0 { + write!(f, ", jit_entry_idx={jit_entry_idx}")?; + } write_separated!(f, ", ", ", ", args); Ok(()) } @@ -2530,7 +2534,6 @@ fn can_direct_send(function: &mut Function, block: BlockId, iseq: *const rb_iseq let caller_passes_block_arg = (unsafe { rb_vm_ci_flag(ci) } & VM_CALL_ARGS_BLOCKARG) != 0; use Counter::*; - if 0 != params.flags.has_rest() { count_failure(complex_arg_pass_param_rest) } if 0 != params.flags.forwardable() { count_failure(complex_arg_pass_param_forwardable) } if callee_has_block_param && caller_passes_block_arg { count_failure(complex_arg_pass_param_block) } @@ -2559,6 +2562,7 @@ fn can_direct_send(function: &mut Function, block: BlockId, iseq: *const rb_iseq let kw_total_num = if keyword.is_null() { 0 } else { unsafe { (*keyword).num } }; let kwarg = unsafe { rb_vm_ci_kwarg(ci) }; let caller_kw_count = if kwarg.is_null() { 0 } else { (unsafe { get_cikw_keyword_len(kwarg) }) as usize }; + let has_rest = 0 != params.flags.has_rest(); let caller_positional = match args.len().checked_sub(caller_kw_count) { Some(count) => count, None => { @@ -2567,10 +2571,28 @@ fn can_direct_send(function: &mut Function, block: BlockId, iseq: *const rb_iseq } }; - let positional_ok = c_int::try_from(caller_positional) - .as_ref() - .map(|argc| (lead_num + post_num..=lead_num + opt_num + post_num).contains(argc)) - .unwrap_or(false); + // When the callee has no keyword parameter table, VM dispatch may convert + // caller keywords into a positional hash. DirectSend only models explicit + // keyword slots for now, so leave that conversion to VM dispatch. + if caller_kw_count != 0 && keyword.is_null() { + function.count(block, complex_arg_pass_keyword_to_positional_hash); + function.set_dynamic_send_reason(send_insn, ComplexArgPass); + return false; + } + + let caller_positional_i32 = match c_int::try_from(caller_positional) { + Ok(argc) => argc, + Err(_) => { + function.set_dynamic_send_reason(send_insn, ArgcParamMismatch); + return false; + } + }; + let min_positional = lead_num + post_num; + let positional_ok = if has_rest { + (min_positional..).contains(&caller_positional_i32) + } else { + (min_positional..=min_positional + opt_num).contains(&caller_positional_i32) + }; let keyword_ok = c_int::try_from(caller_kw_count) .as_ref() .map(|argc| (kw_req_num..=kw_total_num).contains(argc)) @@ -2580,21 +2602,27 @@ fn can_direct_send(function: &mut Function, block: BlockId, iseq: *const rb_iseq return false } - // asm.ccall() doesn't support 6+ args. Compute the final argc after keyword setup: - // final_argc = caller's positional args + callee's total keywords (all kw slots are filled). + // asm.ccall() doesn't support 6+ args. Compute the final argc after keyword setup + // and rest packing: + // send_argc = packed positional args + callee's total keywords (all kw slots are filled). + // c_argc = self + send_argc + synthetic block handler arg. // Right now, the JIT entrypoint accepts the block as an param // We may remove it, remove the block_arg addition to match // See: https://github.com/ruby/ruby/pull/15911#discussion_r2710544982 let block_arg = if 0 != params.flags.has_block() { 1 } else { 0 }; - let final_argc = caller_positional + kw_total_num as usize + block_arg; + let passed_opt_num = (caller_positional_i32 - min_positional).min(opt_num) as usize; + let send_positional_argc = if has_rest { min_positional as usize + passed_opt_num + 1 } else { caller_positional }; + let send_argc = send_positional_argc + kw_total_num as usize; + let c_argc = 1 + send_argc + block_arg; // +1 for self + // TODO: Support passing arguments on the stack in C calls - if final_argc + 1 > C_ARG_OPNDS.len() { // +1 for self + if c_argc > C_ARG_OPNDS.len() { function.set_dynamic_send_reason(send_insn, TooManyArgsForLir); return false; } - // IseqCall stores num_optionals_passed and argc as u16 - if u16::try_from(args.len()).is_err() { + // IseqCall stores the JIT entry index and argc as u16. + if u16::try_from(send_argc).is_err() { function.set_dynamic_send_reason(send_insn, TooManyArgsForLir); return false; } @@ -2678,6 +2706,14 @@ enum IseqReturn { InvokeLeafBuiltin(*const rb_builtin_function, Type), } +/// Arguments and metadata prepared for lowering an ISEQ call to `SendDirect`. +struct SendDirectArgs { + state: InsnId, + args: Vec, + kw_bits: u32, + jit_entry_idx: u16, +} + unsafe extern "C" { fn rb_simple_iseq_p(iseq: IseqPtr) -> bool; } @@ -3408,8 +3444,9 @@ impl Function { } } - /// Prepare arguments for a direct send, handling keyword argument reordering and default synthesis. - /// Returns the (state, processed_args, kw_bits) to use for the SendDirect instruction, + /// Prepare arguments for a direct send, handling keyword argument reordering, + /// default synthesis, and rest parameter packing. + /// Returns the arguments to use for the SendDirect instruction, /// or Err with the fallback reason if direct send isn't possible. fn prepare_direct_send_args( &mut self, @@ -3418,9 +3455,10 @@ impl Function { ci: *const rb_callinfo, iseq: IseqPtr, state: InsnId, - ) -> Result<(InsnId, Vec, u32), SendFallbackReason> { + ) -> Result { let kwarg = unsafe { rb_vm_ci_kwarg(ci) }; let (processed_args, caller_argc, kw_bits) = self.setup_keyword_arguments(block, args, kwarg, iseq)?; + let (processed_args, jit_entry_idx) = self.setup_rest_parameter(block, processed_args, iseq, state)?; // If args were reordered or synthesized, create a new snapshot with the updated stack let send_state = if processed_args != args { @@ -3430,7 +3468,12 @@ impl Function { state }; - Ok((send_state, processed_args, kw_bits)) + Ok(SendDirectArgs { + state: send_state, + args: processed_args, + kw_bits, + jit_entry_idx, + }) } /// Reorder keyword arguments to match the callee's expected order, and synthesize @@ -3561,6 +3604,67 @@ impl Function { Ok((processed_args, caller_argc, kw_bits)) } + /// Compute the positional optional entry index and pack positional arguments + /// into a rest array when the callee has a *rest parameter. + /// + /// The input args must already have keyword arguments normalized to the callee's + /// keyword table order by setup_keyword_arguments. This function only reshapes + /// the positional section before those keyword slots. + /// + /// Returns Ok with (processed_args, jit_entry_idx) if successful, or Err with + /// the fallback reason if direct send isn't possible. + /// - processed_args: arguments to use for SendDirect after optional rest packing + /// - jit_entry_idx: number of positional optional parameters provided by the caller + fn setup_rest_parameter( + &mut self, + block: BlockId, + args: Vec, + iseq: IseqPtr, + state: InsnId, + ) -> Result<(Vec, u16), SendFallbackReason> { + let params = unsafe { iseq.params() }; + let lead_num = params.lead_num as usize; + let opt_num = params.opt_num as usize; + let post_num = params.post_num as usize; + let kw_num = callee_kw_num(iseq); + + let positional_argc = args.len().checked_sub(kw_num).ok_or(ArgcParamMismatch)?; + let min_positional_argc = lead_num + post_num; + if positional_argc < min_positional_argc { return Err(ArgcParamMismatch); } + + // See vm_call_iseq_setup_normal_opt_start in vm_inshelper.c. + // For computing the optional positional entry point, only count positional args + // and exclude the always-present lead and post slots. + let passed_opt_num = (positional_argc - min_positional_argc).min(opt_num); + let jit_entry_idx = passed_opt_num.try_into().map_err(|_| TooManyArgsForLir)?; + + // Methods without *rest still need the jit_entry_idx computed above, + // but their positional args do not need repacking. + if params.flags.has_rest() == 0 { + return Ok((args, jit_entry_idx)); + } + + // Rebuild [lead, filled opts, rest elements..., post, kw...] into + // [lead, filled opts, rest array, post, kw...]. Keyword args were + // already normalized to the callee's keyword table order. + let rest_start = lead_num + passed_opt_num; + let rest_end = positional_argc - post_num; + let (prefix, rest_and_suffix) = args.split_at(rest_start); + let (rest_elements, suffix) = rest_and_suffix.split_at(rest_end - rest_start); + + let rest = self.push_insn(block, Insn::NewArray { + elements: rest_elements.to_vec(), + state, + }); + + let mut packed_args = Vec::with_capacity(prefix.len() + 1 + suffix.len()); + packed_args.extend_from_slice(prefix); + packed_args.push(rest); + packed_args.extend_from_slice(suffix); + + Ok((packed_args, jit_entry_idx)) + } + /// Resolve the receiver type for method dispatch optimization. /// /// Takes the receiver's Type, receiver HIR instruction, and ISEQ instruction index. @@ -4004,7 +4108,7 @@ impl Function { } // Check if the args are compatible before emitting any assmptions - let Ok((send_state, processed_args, kw_bits)) = self.prepare_direct_send_args(block, &args, ci, iseq, state) + let Ok(SendDirectArgs { state: send_state, args: send_args, kw_bits, jit_entry_idx }) = self.prepare_direct_send_args(block, &args, ci, iseq, state) .inspect_err(|&reason| self.set_dynamic_send_reason(insn_id, reason)) else { self.push_insn_id(block, insn_id); continue; }; @@ -4024,7 +4128,7 @@ impl Function { self.insn_types[recv.0] = self.infer_type(recv); } - let replacement = self.try_inline_send_direct(block, Insn::SendDirect(Box::new(SendDirectData { recv, cd, cme, iseq, args: processed_args, kw_bits, state: send_state, block: send_block }))); + let replacement = self.try_inline_send_direct(block, Insn::SendDirect(Box::new(SendDirectData { recv, cd, cme, iseq, args: send_args, kw_bits, jit_entry_idx, state: send_state, block: send_block }))); self.make_equal_to(insn_id, replacement); } else if !has_block && def_type == VM_METHOD_TYPE_BMETHOD { let procv = unsafe { rb_get_def_bmethod_proc((*cme).def) }; @@ -4044,7 +4148,7 @@ impl Function { } // Check if the args are compatible before emitting any assmptions - let Ok((send_state, processed_args, kw_bits)) = self.prepare_direct_send_args(block, &args, ci, iseq, state) + let Ok(SendDirectArgs { state: send_state, args: send_args, kw_bits, jit_entry_idx }) = self.prepare_direct_send_args(block, &args, ci, iseq, state) .inspect_err(|&reason| self.set_dynamic_send_reason(insn_id, reason)) else { self.push_insn_id(block, insn_id); continue; }; @@ -4067,7 +4171,7 @@ impl Function { recv = self.guard_type_recompile(block, recv, Type::from_profiled_type(profiled_type), state, Recompile); } - let replacement = self.try_inline_send_direct(block, Insn::SendDirect(Box::new(SendDirectData { recv, cd, cme, iseq, args: processed_args, kw_bits, state: send_state, block: None }))); + let replacement = self.try_inline_send_direct(block, Insn::SendDirect(Box::new(SendDirectData { recv, cd, cme, iseq, args: send_args, kw_bits, jit_entry_idx, state: send_state, block: None }))); self.make_equal_to(insn_id, replacement); } else if !has_block && def_type == VM_METHOD_TYPE_IVAR && args.is_empty() { // Check if we're accessing ivars of a Class or Module object as they require single-ractor mode. @@ -4597,7 +4701,7 @@ impl Function { } // Check if the args are compatible before emitting any assmptions - let Ok((send_state, processed_args, kw_bits)) = self.prepare_direct_send_args(block, &args, ci, super_iseq, state) + let Ok(SendDirectArgs { state: send_state, args: send_args, kw_bits, jit_entry_idx }) = self.prepare_direct_send_args(block, &args, ci, super_iseq, state) .inspect_err(|&reason| self.set_dynamic_send_reason(insn_id, reason)) else { self.push_insn_id(block, insn_id); continue; }; @@ -4610,8 +4714,9 @@ impl Function { cd, cme: super_cme, iseq: super_iseq, - args: processed_args, + args: send_args, kw_bits, + jit_entry_idx, state: send_state, block: None, }))); @@ -4768,8 +4873,8 @@ impl Function { fn can_inline(callee_iseq: IseqPtr) -> bool { // Inline callees with required, optional, post-required positional, keyword, and // block parameters, including callees that dispatch to a passed block with `yield`. - // Rest params, double-splat (kwrest), and forwardable params are rejected because - // `can_direct_send` rejects them -- we only inline direct sends. + // Rest params, double-splat (kwrest), and forwardable params stay out of the + // general inliner for now; rest-aware direct sends are still handled by codegen. let params = unsafe { callee_iseq.params() }; if params.flags.has_rest() != 0 || params.flags.forwardable() != 0 @@ -4895,7 +5000,7 @@ impl Function { else { unreachable!("position {send_insn_id} is not a SendDirect"); }; - let SendDirectData { recv, cme, iseq, args, kw_bits, block: call_block, state, .. } = *data; + let SendDirectData { recv, cme, iseq, args, kw_bits, jit_entry_idx, block: call_block, state, .. } = *data; // SendDirect invariant: block is either None or BlockIseq. // BlockArg is rejected upstream during type specialization. let blockiseq: Option = call_block.map(|bh| match bh { @@ -4931,20 +5036,16 @@ impl Function { // passed: it runs the default-init code for the remaining // `opt_num - k` optionals (if any) before falling through into the // post-default body. SendDirect emission already guarantees - // args.len() lies in `lead_num + post_num + kw_num..=lead_num + - // opt_num + post_num + kw_num`, with `kw_num` being the callee's - // full keyword count (zero when the callee has no keywords). After - // `prepare_direct_send_args` runs, the caller's arg list has a slot - // for every callee keyword (filled in callee table order, padded - // with defaults for omitted optional keywords), so the keyword tail - // is a fixed-size addition we subtract off before recovering the - // optional-positional count. + // `prepare_direct_send_args` records the matching `jit_entry_idx` + // when it packs the caller args, so do not recover the optional + // positional count from args.len() here. let callee_params = unsafe { iseq.params() }; let lead_num = callee_params.lead_num as usize; let opt_num = callee_params.opt_num as usize; let post_num = callee_params.post_num as usize; let kw_num = callee_kw_num(iseq); - let passed_opt_num = args.len() - lead_num - post_num - kw_num; + let rest_slots = usize::from(callee_params.flags.has_rest() != 0); + let passed_opt_num = jit_entry_idx as usize; // Create the continuation block before translating the callee so it // can serve as the return_block argument; the callee's leaves become @@ -5000,7 +5101,7 @@ impl Function { debug_assert!(matches!(self.find(tail[0]), Insn::SendDirect(..))); let omitted_opt_num = opt_num - passed_opt_num; - let positional_kw_end = lead_num + opt_num + post_num + kw_num; + let positional_kw_end = lead_num + opt_num + rest_slots + post_num + kw_num; let kw_bits_local_idx = callee_kw_bits_local_idx(iseq); let callee_entry_body_block = add_result.body_entry_block .expect("inlined compilation always produces a body entry block"); diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index cabbc251f33f01..d49f688ed083bd 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -4009,11 +4009,10 @@ mod hir_opt_tests { } #[test] - fn dont_specialize_call_to_iseq_with_rest() { - enable_zjit_stats(); + fn specialize_call_to_iseq_with_rest() { eval(" - def foo(*args) = 1 - def test = foo 1 + def foo(*args) = args.length + def test = foo 1, 2, 3 test test "); @@ -4026,18 +4025,240 @@ mod hir_opt_tests { bb2(): EntryPoint JIT(0) v4:BasicObject = LoadArg :self@0 - IncrCounterPtr Jump bb3(v4) - bb3(v7:BasicObject): - IncrCounter zjit_insn_count - IncrCounter zjit_insn_count - v14:Fixnum[1] = Const Value(1) - IncrCounter zjit_insn_count - IncrCounter complex_arg_pass_param_rest - v17:BasicObject = Send v7, :foo, v14 # SendFallbackReason: Complex argument passing - IncrCounter zjit_insn_count + bb3(v6:BasicObject): + v11:Fixnum[1] = Const Value(1) + v13:Fixnum[2] = Const Value(2) + v15:Fixnum[3] = Const Value(3) + v23:ArrayExact = NewArray v11, v13, v15 + PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) + v26:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile + v27:BasicObject = SendDirect v26, 0x1038, :foo (0x1048), v23 CheckInterrupts - Return v17 + Return v27 + "); + } + + #[test] + fn specialize_call_to_iseq_with_many_rest_arguments() { + eval(" + def foo(*args) = args.length + def test = foo 1, 2, 3, 4, 5, 6, 7 + test + test + "); + assert_snapshot!(hir_string("test"), @" + fn test@:3: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v4:BasicObject = LoadArg :self@0 + Jump bb3(v4) + bb3(v6:BasicObject): + v11:Fixnum[1] = Const Value(1) + v13:Fixnum[2] = Const Value(2) + v15:Fixnum[3] = Const Value(3) + v17:Fixnum[4] = Const Value(4) + v19:Fixnum[5] = Const Value(5) + v21:Fixnum[6] = Const Value(6) + v23:Fixnum[7] = Const Value(7) + v31:ArrayExact = NewArray v11, v13, v15, v17, v19, v21, v23 + PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) + v34:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile + v35:BasicObject = SendDirect v34, 0x1038, :foo (0x1048), v31 + CheckInterrupts + Return v35 + "); + } + + #[test] + fn specialize_call_to_iseq_with_rest_and_block_literal() { + eval(" + def foo(*args) = yield args.length + def test = foo(1, 2, 3) { |n| n + 1 } + test + test + "); + assert_snapshot!(hir_string("test"), @" + fn test@:3: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v4:BasicObject = LoadArg :self@0 + Jump bb3(v4) + bb3(v6:BasicObject): + v11:Fixnum[1] = Const Value(1) + v13:Fixnum[2] = Const Value(2) + v15:Fixnum[3] = Const Value(3) + v23:ArrayExact = NewArray v11, v13, v15 + PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) + v26:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile + v27:BasicObject = SendDirect v26, 0x1038, :foo (0x1048), v23 + CheckInterrupts + Return v27 + "); + } + + #[test] + fn specialize_call_to_iseq_with_rest_and_block_param() { + eval(" + def foo(*args, &block) = block.call(args.length) + def test = foo(1, 2, 3) { |n| n + 1 } + test + test + "); + assert_snapshot!(hir_string("test"), @" + fn test@:3: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v4:BasicObject = LoadArg :self@0 + Jump bb3(v4) + bb3(v6:BasicObject): + v11:Fixnum[1] = Const Value(1) + v13:Fixnum[2] = Const Value(2) + v15:Fixnum[3] = Const Value(3) + v23:ArrayExact = NewArray v11, v13, v15 + PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) + v26:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile + v27:BasicObject = SendDirect v26, 0x1038, :foo (0x1048), v23 + CheckInterrupts + Return v27 + "); + } + + #[test] + fn specialize_call_to_iseq_with_rest_and_post() { + eval(" + def foo(a, *args, z) = args.length + a + z + def test = foo 1, 2, 3, 4 + test + test + "); + assert_snapshot!(hir_string("test"), @" + fn test@:3: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v4:BasicObject = LoadArg :self@0 + Jump bb3(v4) + bb3(v6:BasicObject): + v11:Fixnum[1] = Const Value(1) + v13:Fixnum[2] = Const Value(2) + v15:Fixnum[3] = Const Value(3) + v17:Fixnum[4] = Const Value(4) + v25:ArrayExact = NewArray v13, v15 + PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) + v28:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile + v29:BasicObject = SendDirect v28, 0x1038, :foo (0x1048), v11, v25, v17 + CheckInterrupts + Return v29 + "); + } + + #[test] + fn specialize_call_to_iseq_with_rest_and_keyword() { + eval(" + def foo(*args, k:) = args.length + k + def test = foo 1, 2, k: 40 + test + test + "); + assert_snapshot!(hir_string("test"), @" + fn test@:3: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v4:BasicObject = LoadArg :self@0 + Jump bb3(v4) + bb3(v6:BasicObject): + v11:Fixnum[1] = Const Value(1) + v13:Fixnum[2] = Const Value(2) + v15:Fixnum[40] = Const Value(40) + v23:ArrayExact = NewArray v11, v13 + PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) + v26:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile + v27:BasicObject = SendDirect v26, 0x1038, :foo (0x1048), v23, v15 + CheckInterrupts + Return v27 + "); + } + + #[test] + fn specialize_call_to_iseq_with_rest_and_optional_keyword_default() { + eval(" + def foo(*args, k: 40) = args.length + k + def test = foo 1, 2 + test + test + "); + assert_snapshot!(hir_string("test"), @" + fn test@:3: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v4:BasicObject = LoadArg :self@0 + Jump bb3(v4) + bb3(v6:BasicObject): + v11:Fixnum[1] = Const Value(1) + v13:Fixnum[2] = Const Value(2) + v21:Fixnum[40] = Const Value(40) + v22:ArrayExact = NewArray v11, v13 + PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) + v25:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile + v26:BasicObject = SendDirect v25, 0x1038, :foo (0x1048), v22, v21 + CheckInterrupts + Return v26 + "); + } + + #[test] + fn specialize_call_to_iseq_with_optional_and_rest() { + eval(" + def foo(a, b = 1, *rest) = [a, b, rest] + def test = foo(10, 20, 30, 40) + test + test + "); + assert_snapshot!(hir_string("test"), @" + fn test@:3: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v4:BasicObject = LoadArg :self@0 + Jump bb3(v4) + bb3(v6:BasicObject): + v11:Fixnum[10] = Const Value(10) + v13:Fixnum[20] = Const Value(20) + v15:Fixnum[30] = Const Value(30) + v17:Fixnum[40] = Const Value(40) + v25:ArrayExact = NewArray v15, v17 + PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) + v28:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile + v29:BasicObject = SendDirect v28, 0x1038, :foo (0x1048), jit_entry_idx=1, v11, v13, v25 + CheckInterrupts + Return v29 "); } @@ -4349,7 +4570,7 @@ mod hir_opt_tests { v20:Fixnum[30] = Const Value(30) v22:Fixnum[6] = Const Value(6) PatchPoint MethodRedefined(Object@0x1000, target@0x1008, cme:0x1010) - v52:BasicObject = SendDirect v48, 0x1038, :target (0x1048), v16, v18, v20, v22 + v52:BasicObject = SendDirect v48, 0x1038, :target (0x1048), jit_entry_idx=3, v16, v18, v20, v22 v27:Fixnum[10] = Const Value(10) v29:Fixnum[20] = Const Value(20) v31:Fixnum[30] = Const Value(30) @@ -4363,6 +4584,39 @@ mod hir_opt_tests { "); } + #[test] + fn dont_specialize_call_to_rest_with_keyword_to_positional_hash() { + enable_zjit_stats(); + eval(" + def foo(*args) = args + def test = foo(k: 1) + test + test + "); + assert_snapshot!(hir_string("test"), @" + fn test@:3: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v4:BasicObject = LoadArg :self@0 + IncrCounterPtr + Jump bb3(v4) + bb3(v7:BasicObject): + IncrCounter zjit_insn_count + IncrCounter zjit_insn_count + v14:Fixnum[1] = Const Value(1) + IncrCounter zjit_insn_count + IncrCounter complex_arg_pass_keyword_to_positional_hash + v17:BasicObject = Send v7, :foo, v14 # SendFallbackReason: Complex argument passing + IncrCounter zjit_insn_count + CheckInterrupts + Return v17 + "); + } + #[test] fn test_send_call_to_iseq_with_optional_kw() { eval(" diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 51327153ae19b6..e9be7ee0c7c9ab 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -435,7 +435,6 @@ make_counters! { unspecialized_super_def_type_null, // Unsupported parameter features - complex_arg_pass_param_rest, complex_arg_pass_param_post, complex_arg_pass_param_kwrest, complex_arg_pass_param_block, @@ -453,6 +452,9 @@ make_counters! { complex_arg_pass_caller_zsuper, complex_arg_pass_caller_forwarding, + // Unsupported argument conversions + complex_arg_pass_keyword_to_positional_hash, + // Writes to the VM frame vm_write_jit_frame_count, vm_write_sp_count, From 5b8f6a2265a6ebfb0921472c717b1512043ff13f Mon Sep 17 00:00:00 2001 From: nozomemein Date: Tue, 7 Jul 2026 08:27:27 +0900 Subject: [PATCH 6/8] ZJIT: Fix keyword-to-hash argument count fallback Count keyword-to-positional-hash cases as ComplexArgPass only when the converted positional argument count is accepted by the callee. Otherwise, keep the fallback reason as ArgcParamMismatch. Add an opt test covering the arity mismatch case. --- zjit/src/hir.rs | 32 +++++++++++++++++++++----------- zjit/src/hir/opt_tests.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 11 deletions(-) diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 95d9d05a389123..81dd9ae6aea432 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -2571,16 +2571,12 @@ fn can_direct_send(function: &mut Function, block: BlockId, iseq: *const rb_iseq } }; - // When the callee has no keyword parameter table, VM dispatch may convert - // caller keywords into a positional hash. DirectSend only models explicit - // keyword slots for now, so leave that conversion to VM dispatch. - if caller_kw_count != 0 && keyword.is_null() { - function.count(block, complex_arg_pass_keyword_to_positional_hash); - function.set_dynamic_send_reason(send_insn, ComplexArgPass); - return false; - } - - let caller_positional_i32 = match c_int::try_from(caller_positional) { + // Match vm_args.c's setup_parameters_complex via args_kw_argv_to_hash: + // keywords passed to a method with no keyword parameters can become one + // positional hash before the argument count check. + let keywords_as_positional_hash = caller_kw_count != 0 && keyword.is_null(); + let effective_positional = caller_positional + usize::from(keywords_as_positional_hash); + let caller_positional_i32 = match c_int::try_from(effective_positional) { Ok(argc) => argc, Err(_) => { function.set_dynamic_send_reason(send_insn, ArgcParamMismatch); @@ -2593,11 +2589,25 @@ fn can_direct_send(function: &mut Function, block: BlockId, iseq: *const rb_iseq } else { (min_positional..=min_positional + opt_num).contains(&caller_positional_i32) }; + if !positional_ok { + function.set_dynamic_send_reason(send_insn, ArgcParamMismatch); + return false + } + + // When the callee has no keyword parameter table, VM dispatch may convert + // caller keywords into a positional hash. SendDirect only models explicit + // keyword slots for now, so leave that conversion to VM dispatch. + if keywords_as_positional_hash { + function.count(block, complex_arg_pass_keyword_to_positional_hash); + function.set_dynamic_send_reason(send_insn, ComplexArgPass); + return false; + } + let keyword_ok = c_int::try_from(caller_kw_count) .as_ref() .map(|argc| (kw_req_num..=kw_total_num).contains(argc)) .unwrap_or(false); - if !positional_ok || !keyword_ok { + if !keyword_ok { function.set_dynamic_send_reason(send_insn, ArgcParamMismatch); return false } diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index d49f688ed083bd..fe405557e0439c 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -4617,6 +4617,32 @@ mod hir_opt_tests { "); } + #[test] + fn dont_classify_keyword_to_positional_hash_argc_mismatch_as_complex_arg_pass() { + eval(" + def foo(a, b) = a + def test = foo(k: 1) + begin; test; rescue ArgumentError; end + begin; test; rescue ArgumentError; end + "); + assert_snapshot!(hir_string("test"), @" + fn test@:3: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v4:BasicObject = LoadArg :self@0 + Jump bb3(v4) + bb3(v6:BasicObject): + v11:Fixnum[1] = Const Value(1) + v13:BasicObject = Send v6, :foo, v11 # SendFallbackReason: Argument count does not match parameter count + CheckInterrupts + Return v13 + "); + } + #[test] fn test_send_call_to_iseq_with_optional_kw() { eval(" From 657ce7c7ce33485822977ca1007c67e310ef8567 Mon Sep 17 00:00:00 2001 From: nozomemein Date: Tue, 7 Jul 2026 09:31:58 +0900 Subject: [PATCH 7/8] ZJIT: Clarify SendDirect argument setup comments Reference the VM argument setup functions that SendDirect mirrors for keyword-to-positional-hash handling, rest parameter packing, and optional argument entry selection. --- zjit/src/hir.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 81dd9ae6aea432..dd8424384944de 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -2594,9 +2594,8 @@ fn can_direct_send(function: &mut Function, block: BlockId, iseq: *const rb_iseq return false } - // When the callee has no keyword parameter table, VM dispatch may convert - // caller keywords into a positional hash. SendDirect only models explicit - // keyword slots for now, so leave that conversion to VM dispatch. + // SendDirect only models explicit keyword slots for now, so leave this + // conversion to VM dispatch. if keywords_as_positional_hash { function.count(block, complex_arg_pass_keyword_to_positional_hash); function.set_dynamic_send_reason(send_insn, ComplexArgPass); @@ -2620,6 +2619,8 @@ fn can_direct_send(function: &mut Function, block: BlockId, iseq: *const rb_iseq // We may remove it, remove the block_arg addition to match // See: https://github.com/ruby/ruby/pull/15911#discussion_r2710544982 let block_arg = if 0 != params.flags.has_block() { 1 } else { 0 }; + // With *rest, SendDirect receives one rest-array slot instead of each rest + // element, so cap positional argc at required/post + filled opts + rest slot. let passed_opt_num = (caller_positional_i32 - min_positional).min(opt_num) as usize; let send_positional_argc = if has_rest { min_positional as usize + passed_opt_num + 1 } else { caller_positional }; let send_argc = send_positional_argc + kw_total_num as usize; @@ -3616,6 +3617,9 @@ impl Function { /// Compute the positional optional entry index and pack positional arguments /// into a rest array when the callee has a *rest parameter. + /// Mirrors vm_args.c's setup_parameters_complex / args_setup_rest_parameter: + /// positional arguments between required/optional and post parameters become + /// the callee's *rest array before entering the method body. /// /// The input args must already have keyword arguments normalized to the callee's /// keyword table order by setup_keyword_arguments. This function only reshapes @@ -3642,9 +3646,10 @@ impl Function { let min_positional_argc = lead_num + post_num; if positional_argc < min_positional_argc { return Err(ArgcParamMismatch); } - // See vm_call_iseq_setup_normal_opt_start in vm_inshelper.c. - // For computing the optional positional entry point, only count positional args - // and exclude the always-present lead and post slots. + // For computing the optional positional entry point, only count positional + // args and exclude the always-present lead and post slots. Do this before + // rest packing changes the SendDirect argument count. + // See: vm_args.c's setup_parameters_complex and args_setup_opt_parameters. let passed_opt_num = (positional_argc - min_positional_argc).min(opt_num); let jit_entry_idx = passed_opt_num.try_into().map_err(|_| TooManyArgsForLir)?; @@ -3654,9 +3659,9 @@ impl Function { return Ok((args, jit_entry_idx)); } - // Rebuild [lead, filled opts, rest elements..., post, kw...] into - // [lead, filled opts, rest array, post, kw...]. Keyword args were - // already normalized to the callee's keyword table order. + // Rebuild [lead, filled opts, rest elements..., post, kw...] into the + // argument shape expected by SendDirect: + // [lead, filled opts, rest array, post, kw...]. let rest_start = lead_num + passed_opt_num; let rest_end = positional_argc - post_num; let (prefix, rest_and_suffix) = args.split_at(rest_start); From 3513044298b498b5c5c0127f5ff85d6384b29328 Mon Sep 17 00:00:00 2001 From: Randy Stauner Date: Mon, 13 Jul 2026 17:34:01 -0700 Subject: [PATCH 8/8] ZJIT: Optimize send with a nil block to SendWithoutBlockDirect (#16621) * ZJIT: Add HIR tests for block arg nil and non-nil * ZJIT: Optimize send with a nil block to SendWithoutBlockDirect A call like `foo(&block)` sets VM_CALL_ARGS_BLOCKARG, which previously forced a dynamic dispatch even when the block argument is nil and the callee is an ISEQ method that could otherwise be reduced to a SendDirect. Passing a nil block argument is equivalent to calling without a block, so when we can prove the block argument is nil we strip it and treat the call as blockless. This happens when the block argument is either: * statically known to be nil, or * profiled as monomorphically nil, in which case we emit a GuardBitEquals against nil (side exit reason BlockArgNotNil) before stripping it. The guard recompiles on exit, so a site that starts seeing non-nil blocks re-profiles and drops the speculation, falling back to a dynamic send instead of taking the guard exit repeatedly. When the block argument can't be proven nil, we fall back to a dynamic Send and record the SendBlockArgNotNil fallback reason. The block-stripped frame state is used only to set up the callee frame for the direct send. The pre-call guards (the nil-block GuardBitEquals, the receiver GuardType, and the method-redefinition PatchPoint) keep using the original frame state, which still has the block argument on the stack. This matters because a guard side exit re-executes the `send` in the interpreter, which pops the block argument off the stack; reconstructing the interpreter stack without it would leave the saved SP off by one and corrupt the pushed frame. --- test/ruby/test_zjit_cli.rb | 50 +++++++++++++ zjit/src/hir.rs | 68 ++++++++++++++++-- zjit/src/hir/opt_tests.rs | 141 +++++++++++++++++++++++++++++++++---- zjit/src/stats.rs | 4 ++ 4 files changed, 244 insertions(+), 19 deletions(-) diff --git a/test/ruby/test_zjit_cli.rb b/test/ruby/test_zjit_cli.rb index ad7e7a72d9d3c2..877d345fb32f9c 100644 --- a/test/ruby/test_zjit_cli.rb +++ b/test/ruby/test_zjit_cli.rb @@ -304,6 +304,56 @@ def array.itself = :not_itself RUBY end + def test_send_forwarded_block_arg_nil_then_non_nil + # Regression test: when a forwarded &block arg is profiled as nil, the nil + # block optimization must update the frame state to match the stripped args. + # Otherwise the saved SP is off by one, causing a stack consistency error + # when the guard side-exits for a non-nil block. + assert_runs ':ok', <<~RUBY, call_threshold: 2 + def inner(callable = nil, &block) + callable || block + end + + def outer(&block) + inner(&block) + end + + 100.times { outer } + result = outer { |x| x } + result.is_a?(Proc) ? :ok : :fail + RUBY + end + + def test_send_forwarded_nil_block_arg_with_polymorphic_receiver + # Regression test: the nil block optimization strips the block arg from the + # frame state used to set up the callee frame, but the pre-call guards + # (receiver GuardType, method-redefinition PatchPoint) must keep using the + # original frame state that still has the block arg on the stack. Otherwise a + # guard side-exit re-executes the send with a stack that is missing the block + # arg slot, corrupting the pushed frame's EP (VM_ENV_FLAGS assertion failure). + # A polymorphic receiver forces the receiver GuardType to side-exit. + assert_runs ':ok', <<~RUBY, call_threshold: 2 + class Base + def self.inner(model, name, &block) + block ? block.call : model + end + def self.outer(model, name, &block) + inner(model, name, &block) + end + end + class A < Base; end + class B < Base; end + class C < Base; end + class D < Base; end + + 1000.times do |i| + klass = [A, B, C, D][i % 4] + klass.outer(i, :n) + end + :ok + RUBY + end + private # Assert that every method call in `test_script` can be compiled by ZJIT diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index dd8424384944de..e16e7df836d66b 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -537,6 +537,7 @@ pub enum SideExitReason { UnhandledYARVInsn(u32), UnhandledCallType(CallType), UnhandledBlockArg, + BlockArgNotNil, TooManyKeywordParameters, TooManyArgsForLir, FixnumAddOverflow, @@ -704,6 +705,8 @@ pub enum SendFallbackReason { SendCfuncArrayVariadic, SendNotOptimizedMethodType(MethodType), SendNotOptimizedNeedPermission, + /// The block argument is not nil, so we can't optimize to SendWithoutBlockDirect + SendBlockArgNotNil, CCallWithFrameTooManyArgs, ObjToStringNotString, TooManyArgsForLir, @@ -778,6 +781,7 @@ impl Display for SendFallbackReason { SendCfuncVariadic => write!(f, "Send: C function is variadic"), SendCfuncArrayVariadic => write!(f, "Send: C function expects array variadic"), SendNotOptimizedMethodType(method_type) => write!(f, "Send: unsupported method type {:?}", method_type), + SendBlockArgNotNil => write!(f, "Send: block argument is not nil"), CCallWithFrameTooManyArgs => write!(f, "CCallWithFrame: too many arguments"), ObjToStringNotString => write!(f, "ObjToString: result is not a string"), TooManyArgsForLir => write!(f, "Too many arguments for LIR"), @@ -2531,7 +2535,7 @@ fn can_direct_send(function: &mut Function, block: BlockId, iseq: *const rb_iseq let params = unsafe { iseq.params() }; let callee_has_block_param = 0 != params.flags.has_block(); - let caller_passes_block_arg = (unsafe { rb_vm_ci_flag(ci) } & VM_CALL_ARGS_BLOCKARG) != 0; + let caller_passes_block_arg = has_block && (unsafe { rb_vm_ci_flag(ci) } & VM_CALL_ARGS_BLOCKARG) != 0; use Counter::*; if 0 != params.flags.forwardable() { count_failure(complex_arg_pass_param_forwardable) } @@ -4044,7 +4048,7 @@ impl Function { Insn::Send { recv, block: None, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(minusat) && args.is_empty() => self.try_rewrite_uminus(block, insn_id, recv, state), ref send@Insn::Send { mut recv, cd, state, block: send_block, ref args, .. } => { - let has_block = send_block.is_some(); + let mut has_block = send_block.is_some(); let (klass, profiled_type) = match self.resolve_receiver_type(recv, self.type_of(recv), state) { ReceiverTypeResolution::StaticallyKnown { class } => (class, None), ReceiverTypeResolution::Monomorphic { profiled_type } @@ -4105,9 +4109,63 @@ impl Function { def_type = unsafe { get_cme_def_type(cme) }; } + // Check if we can optimize `foo(&block)` where block is nil to a send without block. + // `state` keeps referring to the pre-send frame state (block arg still on the + // stack). Any guard that side-exits before the call re-executes the `send` in + // the interpreter, so it must reconstruct the stack with the block arg present. + // Only the direct-send frame setup uses `send_frame_state`, which has the nil + // block arg stripped from the stack. + let mut send_block = send_block; + let mut send_frame_state = state; + let mut args = args.to_vec(); + let mut stripped_nil_block = false; + if send_block == Some(BlockHandler::BlockArg) && def_type == VM_METHOD_TYPE_ISEQ { + // The block arg is the last element in args + if let Some(&block_arg) = args.last() { + let statically_nil = self.is_a(block_arg, types::NilClass); + let profiled_nil = self.profiled_type_of_at(block_arg, state) + .map_or(false, |pt| pt.is_nil()); + if statically_nil || profiled_nil { + if !statically_nil { + // Guard needed when relying on profiled type. Uses the original + // `state` so a side exit re-executes the send with the block + // arg still on the VM stack. + // + // Recompile on exit so a site that starts seeing non-nil + // blocks re-profiles the block arg and drops this speculation + // (falling back to a dynamic send) instead of paying the guard + // side exit repeatedly. This matches the receiver GuardType + // below and the getblockparamproxy BlockParamProxyNotNil guard. + self.push_insn(block, Insn::GuardBitEquals { + val: block_arg, + expected: Const::Value(Qnil), + reason: Box::new(SideExitReason::BlockArgNotNil), + state, + recompile: Some(Recompile), + }); + } + // Strip nil block arg and treat as no block + args = args[..args.len() - 1].to_vec(); + send_block = None; + has_block = false; + stripped_nil_block = true; + // Frame state for the direct send only: the block arg is removed + // from the stack so the callee frame is laid out correctly. + let new_state = self.frame_state(state).with_replaced_args(&args, args.len() + 1); + send_frame_state = self.push_insn(block, Insn::Snapshot { state: Box::new(new_state) }); + } else { + // Can't prove block arg is nil + self.set_dynamic_send_reason(insn_id, SendBlockArgNotNil); + self.push_insn_id(block, insn_id); continue; + } + } + } + // If the call site info indicates that the `Function` has overly complex arguments, then do not optimize into a `SendDirect`. // Optimized methods(`VM_METHOD_TYPE_OPTIMIZED`) and C methods handle their own argument constraints (e.g., kw_splat for Proc call). - if def_type != VM_METHOD_TYPE_OPTIMIZED && def_type != VM_METHOD_TYPE_CFUNC && unspecializable_call_type(flags) { + // Mask out ARGS_BLOCKARG only if we've already handled the nil block arg case above. + let flags_for_check = if stripped_nil_block { flags & !VM_CALL_ARGS_BLOCKARG } else { flags }; + if def_type != VM_METHOD_TYPE_OPTIMIZED && def_type != VM_METHOD_TYPE_CFUNC && unspecializable_call_type(flags_for_check) { self.count_complex_call_features(block, flags); self.set_dynamic_send_reason(insn_id, ComplexArgPass); self.push_insn_id(block, insn_id); continue; @@ -4123,7 +4181,7 @@ impl Function { } // Check if the args are compatible before emitting any assmptions - let Ok(SendDirectArgs { state: send_state, args: send_args, kw_bits, jit_entry_idx }) = self.prepare_direct_send_args(block, &args, ci, iseq, state) + let Ok(SendDirectArgs { state: send_state, args: send_args, kw_bits, jit_entry_idx }) = self.prepare_direct_send_args(block, &args, ci, iseq, send_frame_state) .inspect_err(|&reason| self.set_dynamic_send_reason(insn_id, reason)) else { self.push_insn_id(block, insn_id); continue; }; @@ -4163,7 +4221,7 @@ impl Function { } // Check if the args are compatible before emitting any assmptions - let Ok(SendDirectArgs { state: send_state, args: send_args, kw_bits, jit_entry_idx }) = self.prepare_direct_send_args(block, &args, ci, iseq, state) + let Ok(SendDirectArgs { state: send_state, args: send_args, kw_bits, jit_entry_idx }) = self.prepare_direct_send_args(block, &args, ci, iseq, send_frame_state) .inspect_err(|&reason| self.set_dynamic_send_reason(insn_id, reason)) else { self.push_insn_id(block, insn_id); continue; }; diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index fe405557e0439c..8023368039055f 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -3922,7 +3922,7 @@ mod hir_opt_tests { v7:BasicObject = LoadArg :l@1 Jump bb3(v6, v7) bb3(v9:BasicObject, v10:BasicObject): - v16:BasicObject = Send v9, &block, :foo, v10 # SendFallbackReason: Complex argument passing + v16:BasicObject = Send v9, &block, :foo, v10 # SendFallbackReason: Send: block argument is not nil CheckInterrupts Return v16 "); @@ -5558,7 +5558,7 @@ mod hir_opt_tests { v26:TrueClass = GuardBitEquals v25, Value(true) recompile Jump bb6(v24, v10) bb6(v16:BasicObject, v17:BasicObject): - v29:BasicObject = Send v14, &block, :then, v16 # SendFallbackReason: Complex argument passing + v29:BasicObject = Send v14, &block, :then, v16 # SendFallbackReason: Send: block argument is not nil CheckInterrupts Return v29 "); @@ -5614,7 +5614,7 @@ mod hir_opt_tests { v37:NilClass = Const Value(nil) Jump bb8(v37, v13) bb8(v27:BasicObject, v28:BasicObject): - v40:BasicObject = Send v25, &block, :then, v27 # SendFallbackReason: Complex argument passing + v40:BasicObject = Send v25, &block, :then, v27 # SendFallbackReason: Send: block argument is not nil CheckInterrupts Return v40 bb4(v45:BasicObject, v46:Falsy, v47:BasicObject): @@ -5779,7 +5779,7 @@ mod hir_opt_tests { v34:NilClass = Const Value(nil) Jump bb6(v34, v10) bb6(v16:BasicObject, v17:BasicObject): - v38:BasicObject = Send v14, &block, :then, v16 # SendFallbackReason: Complex argument passing + v38:BasicObject = Send v14, &block, :then, v16 # SendFallbackReason: Send: block argument is not nil CheckInterrupts Return v38 bb10(): @@ -5847,7 +5847,7 @@ mod hir_opt_tests { v41:ObjectSubclass[BlockParamProxy] = Const Value(VALUE(0x1010)) Jump bb6(v41, v10) bb6(v16:BasicObject, v17:BasicObject): - v45:BasicObject = Send v14, &block, :then, v16 # SendFallbackReason: Complex argument passing + v45:BasicObject = Send v14, &block, :then, v16 # SendFallbackReason: Send: block argument is not nil CheckInterrupts Return v45 bb13(): @@ -9883,7 +9883,7 @@ mod hir_opt_tests { v26:ObjectSubclass[BlockParamProxy] = Const Value(VALUE(0x1008)) Jump bb6(v26, v10) bb6(v16:BasicObject, v17:BasicObject): - v29:BasicObject = Send v14, &block, :map, v16 # SendFallbackReason: Complex argument passing + v29:BasicObject = Send v14, &block, :map, v16 # SendFallbackReason: Send: block argument is not nil CheckInterrupts Return v29 "); @@ -9923,9 +9923,12 @@ mod hir_opt_tests { v26:NilClass = Const Value(nil) Jump bb6(v26, v10) bb6(v16:BasicObject, v17:BasicObject): - v29:BasicObject = Send v14, &block, :map, v16 # SendFallbackReason: Complex argument passing + v35:NilClass = GuardBitEquals v16, Value(nil) recompile + PatchPoint NoSingletonClass(Array@0x1008) + PatchPoint MethodRedefined(Array@0x1008, map@0x1010, cme:0x1018) + v40:BasicObject = SendDirect v14, 0x1040, :map (0x1050) CheckInterrupts - Return v29 + Return v40 "); } @@ -9964,7 +9967,7 @@ mod hir_opt_tests { v21:ObjectSubclass[BlockParamProxy] = Const Value(VALUE(0x1008)) Jump bb6(v21) bb6(v12:BasicObject): - v24:BasicObject = Send v10, &block, :map, v12 # SendFallbackReason: Complex argument passing + v24:BasicObject = Send v10, &block, :map, v12 # SendFallbackReason: Send: block argument is not nil CheckInterrupts Return v24 "); @@ -10112,6 +10115,116 @@ mod hir_opt_tests { "); } + #[test] + fn test_send_with_non_nil_block_arg() { + eval(r#" + def foo = 42 + + def test + block = :to_s + foo(&block) + end + test; test + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:5: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:NilClass = Const Value(nil) + Jump bb3(v1, v2) + bb2(): + EntryPoint JIT(0) + v5:BasicObject = LoadArg :self@0 + v6:NilClass = Const Value(nil) + Jump bb3(v5, v6) + bb3(v8:BasicObject, v9:NilClass): + v13:StaticSymbol[:to_s] = Const Value(VALUE(0x1000)) + v19:BasicObject = Send v8, &block, :foo, v13 # SendFallbackReason: Send: block argument is not nil + CheckInterrupts + Return v19 + "); + } + + #[test] + fn test_send_with_statically_nil_block_arg() { + eval(r#" + def foo = 42 + + def test + block = nil + foo(&block) + end + test; test + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:5: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:NilClass = Const Value(nil) + Jump bb3(v1, v2) + bb2(): + EntryPoint JIT(0) + v5:BasicObject = LoadArg :self@0 + v6:NilClass = Const Value(nil) + Jump bb3(v5, v6) + bb3(v8:BasicObject, v9:NilClass): + v13:NilClass = Const Value(nil) + PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) + v27:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v8, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile + v28:Fixnum[42] = Const Value(42) + CheckInterrupts + Return v28 + "); + } + + #[test] + fn test_send_with_monomorphically_nil_block_arg() { + eval(r#" + def foo = 42 + + def test(&block) + foo(&block) + end + test; test + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:5: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :block@0x1000 + Jump bb3(v1, v3) + bb2(): + EntryPoint JIT(0) + v6:BasicObject = LoadArg :self@0 + v7:BasicObject = LoadArg :block@1 + Jump bb3(v6, v7) + bb3(v9:BasicObject, v10:BasicObject): + v17:CPtr = GetEP 0 + v18:CUInt64 = LoadField v17, :VM_ENV_DATA_INDEX_FLAGS@0x1001 + v19:CBool = IsBlockParamModified v18 + CondBranch v19, bb4(), bb5() + bb4(): + v21:BasicObject = LoadField v17, :block@0x1002 + Jump bb6(v21, v21) + bb5(): + v23:CInt64 = LoadField v17, :VM_ENV_DATA_INDEX_SPECVAL@0x1003 + v24:CInt64[0] = GuardBitEquals v23, CInt64(0) recompile + v25:NilClass = Const Value(nil) + Jump bb6(v25, v10) + bb6(v15:BasicObject, v16:BasicObject): + v34:NilClass = GuardBitEquals v15, Value(nil) recompile + PatchPoint MethodRedefined(Object@0x1008, foo@0x1010, cme:0x1018) + v37:ObjectSubclass[class_exact*:Object@VALUE(0x1008)] = GuardType v9, ObjectSubclass[class_exact*:Object@VALUE(0x1008)] recompile + v38:Fixnum[42] = Const Value(42) + CheckInterrupts + Return v38 + "); + } + #[test] fn test_inline_attr_reader_constant() { eval(" @@ -13469,7 +13582,7 @@ mod hir_opt_tests { Jump bb3(v4) bb3(v6:BasicObject): v11:StaticSymbol[:the_block] = Const Value(VALUE(0x1000)) - v13:BasicObject = Send v6, &block, :callee, v11 # SendFallbackReason: Complex argument passing + v13:BasicObject = Send v6, &block, :callee, v11 # SendFallbackReason: Send: block argument is not nil CheckInterrupts Return v13 "); @@ -13514,7 +13627,7 @@ mod hir_opt_tests { v26:ObjectSubclass[BlockParamProxy] = Const Value(VALUE(0x1008)) Jump bb6(v26, v10) bb6(v16:BasicObject, v17:BasicObject): - v29:BasicObject = Send v14, &block, :map, v16 # SendFallbackReason: Complex argument passing + v29:BasicObject = Send v14, &block, :map, v16 # SendFallbackReason: Send: block argument is not nil CheckInterrupts Return v29 "); @@ -17386,7 +17499,7 @@ mod hir_opt_tests { test(true, block) "); - assert_snapshot!(hir_string("test"), @r" + assert_snapshot!(hir_string("test"), @" fn test@:7: bb1(): EntryPoint interpreter @@ -17409,7 +17522,7 @@ mod hir_opt_tests { bb5(): v22:Truthy = RefineType v12, Truthy v26:Fixnum[42] = Const Value(42) - v29:BasicObject = Send v11, &block, :passthrough_recompile_blockarg, v26, v13 # SendFallbackReason: Complex argument passing + v29:BasicObject = Send v11, &block, :passthrough_recompile_blockarg, v26, v13 # SendFallbackReason: Send: block argument is not nil CheckInterrupts Return v29 bb4(v34:BasicObject, v35:Falsy, v36:BasicObject): @@ -19826,7 +19939,7 @@ mod hir_opt_tests { v46:ObjectSubclass[BlockParamProxy] = Const Value(VALUE(0x1050)) Jump bb8(v46, v54) bb8(v36:BasicObject, v37:BasicObject): - v49:BasicObject = Send v25, &block, :inner, v10, v36 # SendFallbackReason: Complex argument passing + v49:BasicObject = Send v25, &block, :inner, v10, v36 # SendFallbackReason: Send: block argument is not nil CheckInterrupts PopInlineFrame PatchPoint NoEPEscape(test) diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index e9be7ee0c7c9ab..4c95e9f61d2f83 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -195,6 +195,7 @@ make_counters! { exit_unhandled_splat, exit_unhandled_kwarg, exit_unhandled_block_arg, + exit_block_arg_not_nil, exit_unknown_special_variable, exit_unhandled_hir_insn, exit_unhandled_yarv_insn, @@ -271,6 +272,7 @@ make_counters! { send_fallback_send_no_profiles, send_fallback_send_not_optimized_method_type, send_fallback_send_not_optimized_need_permission, + send_fallback_send_block_arg_not_nil, send_fallback_ccall_with_frame_too_many_args, send_fallback_argc_param_mismatch, // The call has at least one feature on the caller or callee side @@ -618,6 +620,7 @@ pub fn side_exit_counter(reason: crate::hir::SideExitReason) -> Counter { UnhandledHIRUnknown(_) => exit_unhandled_hir_insn, UnhandledYARVInsn(_) => exit_unhandled_yarv_insn, UnhandledBlockArg => exit_unhandled_block_arg, + BlockArgNotNil => exit_block_arg_not_nil, FixnumAddOverflow => exit_fixnum_add_overflow, FixnumSubOverflow => exit_fixnum_sub_overflow, FixnumMultOverflow => exit_fixnum_mult_overflow, @@ -713,6 +716,7 @@ pub fn send_fallback_counter(reason: crate::hir::SendFallbackReason) -> Counter BmethodNonIseqProc => send_fallback_bmethod_non_iseq_proc, SendNotOptimizedMethodType(_) => send_fallback_send_not_optimized_method_type, SendNotOptimizedNeedPermission => send_fallback_send_not_optimized_need_permission, + SendBlockArgNotNil => send_fallback_send_block_arg_not_nil, CCallWithFrameTooManyArgs => send_fallback_ccall_with_frame_too_many_args, ObjToStringNotString => send_fallback_obj_to_string_not_string, SuperCallWithBlock => send_fallback_super_call_with_block,