From df1334421f555a9d2ad9961d234eb3b88418b058 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Fri, 29 Aug 2025 07:22:40 -0700 Subject: [PATCH 1/8] ZJIT: Repurpose ExitCounters for unhandled YARV insns (#14392) --- zjit.rb | 5 +- zjit/src/backend/lir.rs | 14 +++-- zjit/src/codegen.rs | 2 +- zjit/src/hir.rs | 44 +++++++------- zjit/src/stats.rs | 131 ++++++++++++++++++++++------------------ 5 files changed, 106 insertions(+), 90 deletions(-) diff --git a/zjit.rb b/zjit.rb index 6b079b7b097e0e..dd2f46d692b842 100644 --- a/zjit.rb +++ b/zjit.rb @@ -53,8 +53,8 @@ def stats_string :zjit_insn_count, :ratio_in_zjit, ], buf:, stats:) + print_counters_with_prefix(prefix: 'unhandled_yarv_insn_', prompt: 'unhandled YARV insns', buf:, stats:, limit: 20) print_counters_with_prefix(prefix: 'exit_', prompt: 'side exit reasons', buf:, stats:, limit: 20) - print_counters_with_prefix(prefix: 'specific_exit_', prompt: 'specific side exit reasons', buf:, stats:, limit: 20) buf end @@ -96,7 +96,6 @@ def print_counters_with_prefix(buf:, stats:, prefix:, prompt:, limit: nil) left_pad = counters.keys.map(&:size).max right_pad = counters.values.map { |value| number_with_delimiter(value).size }.max total = counters.values.sum - count = counters.size counters = counters.to_a counters.sort_by! { |_, value| -value } @@ -104,7 +103,7 @@ def print_counters_with_prefix(buf:, stats:, prefix:, prompt:, limit: nil) buf << "Top-#{counters.size} " if limit buf << "#{prompt}" - buf << " (%.1f%% of all #{count})" % (100.0 * counters.map(&:last).sum / total) if limit + buf << " (%.1f%% of total #{number_with_delimiter(total)})" % (100.0 * counters.map(&:last).sum / total) if limit buf << ":\n" counters.each do |key, value| padded_key = key.rjust(left_pad, ' ') diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index b372acccea21b3..192c4092bfb3a1 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -6,7 +6,7 @@ use crate::cruby::{Qundef, RUBY_OFFSET_CFP_PC, RUBY_OFFSET_CFP_SP, SIZEOF_VALUE_ use crate::hir::SideExitReason; use crate::options::{debug, get_option}; use crate::cruby::VALUE; -use crate::stats::{exit_counter_ptr, specific_exit_counter_ptr}; +use crate::stats::{exit_counter_ptr, exit_counter_ptr_for_opcode}; use crate::virtualmem::CodePtr; use crate::asm::{CodeBlock, Label}; @@ -1595,13 +1595,15 @@ impl Assembler // Using C_RET_OPND as an additional scratch register, which is no longer used if get_option!(stats) { - asm_comment!(self, "increment an exit counter"); - self.load_into(SCRATCH_OPND, Opnd::const_ptr(exit_counter_ptr(pc))); + asm_comment!(self, "increment a side exit counter"); + self.load_into(SCRATCH_OPND, Opnd::const_ptr(exit_counter_ptr(reason))); self.incr_counter_with_reg(Opnd::mem(64, SCRATCH_OPND, 0), 1.into(), C_RET_OPND); - asm_comment!(self, "increment a specific exit counter"); - self.load_into(SCRATCH_OPND, Opnd::const_ptr(specific_exit_counter_ptr(reason))); - self.incr_counter_with_reg(Opnd::mem(64, SCRATCH_OPND, 0), 1.into(), C_RET_OPND); + if let SideExitReason::UnhandledYARVInsn(opcode) = reason { + asm_comment!(self, "increment an unhandled YARV insn counter"); + self.load_into(SCRATCH_OPND, Opnd::const_ptr(exit_counter_ptr_for_opcode(opcode))); + self.incr_counter_with_reg(Opnd::mem(64, SCRATCH_OPND, 0), 1.into(), C_RET_OPND); + } } asm_comment!(self, "exit to the interpreter"); diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 3a7194ec399983..36a899438c6936 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -274,7 +274,7 @@ fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, function: &Function) -> Optio let insn = function.find(insn_id); if let Err(last_snapshot) = gen_insn(cb, &mut jit, &mut asm, function, insn_id, &insn) { debug!("ZJIT: gen_function: Failed to compile insn: {insn_id} {insn}. Generating side-exit."); - gen_side_exit(&mut jit, &mut asm, &SideExitReason::UnhandledInstruction(insn_id), &function.frame_state(last_snapshot)); + gen_side_exit(&mut jit, &mut asm, &SideExitReason::UnhandledHIRInsn(insn_id), &function.frame_state(last_snapshot)); // Don't bother generating code after a side-exit. We won't run it. // TODO(max): Generate ud2 or equivalent. break; diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index fdefe23e127cb1..1628e049cc8ad5 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -430,10 +430,10 @@ impl PtrPrintMap { pub enum SideExitReason { UnknownNewarraySend(vm_opt_newarray_send_type), UnknownCallType, - UnknownOpcode(u32), UnknownSpecialVariable(u64), - UnhandledInstruction(InsnId), UnhandledDefinedType(usize), + UnhandledHIRInsn(InsnId), + UnhandledYARVInsn(u32), FixnumAddOverflow, FixnumSubOverflow, FixnumMultOverflow, @@ -448,7 +448,7 @@ pub enum SideExitReason { impl std::fmt::Display for SideExitReason { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { match self { - SideExitReason::UnknownOpcode(opcode) => write!(f, "UnknownOpcode({})", insn_name(*opcode as usize)), + SideExitReason::UnhandledYARVInsn(opcode) => write!(f, "UnhandledYARVInsn({})", insn_name(*opcode as usize)), SideExitReason::UnknownNewarraySend(VM_OPT_NEWARRAY_SEND_MAX) => write!(f, "UnknownNewarraySend(MAX)"), SideExitReason::UnknownNewarraySend(VM_OPT_NEWARRAY_SEND_MIN) => write!(f, "UnknownNewarraySend(MIN)"), SideExitReason::UnknownNewarraySend(VM_OPT_NEWARRAY_SEND_HASH) => write!(f, "UnknownNewarraySend(HASH)"), @@ -3484,9 +3484,9 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { } } _ => { - // Unknown opcode; side-exit into the interpreter + // Unhandled opcode; side-exit into the interpreter let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - fun.push_insn(block, Insn::SideExit { state: exit_id, reason: SideExitReason::UnknownOpcode(opcode) }); + fun.push_insn(block, Insn::SideExit { state: exit_id, reason: SideExitReason::UnhandledYARVInsn(opcode) }); break; // End the block } } @@ -4817,11 +4817,11 @@ mod tests { eval(" def test = super() "); - assert_snapshot!(hir_string("test"), @r#" - fn test@:2: - bb0(v0:BasicObject): - SideExit UnknownOpcode(invokesuper) - "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:2: + bb0(v0:BasicObject): + SideExit UnhandledYARVInsn(invokesuper) + "); } #[test] @@ -4829,11 +4829,11 @@ mod tests { eval(" def test = super "); - assert_snapshot!(hir_string("test"), @r#" - fn test@:2: - bb0(v0:BasicObject): - SideExit UnknownOpcode(invokesuper) - "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:2: + bb0(v0:BasicObject): + SideExit UnhandledYARVInsn(invokesuper) + "); } #[test] @@ -5460,13 +5460,13 @@ mod tests { let iseq = crate::cruby::with_rubyvm(|| get_method_iseq("Dir", "open")); assert!(iseq_contains_opcode(iseq, YARVINSN_opt_invokebuiltin_delegate), "iseq Dir.open does not contain invokebuiltin"); let function = iseq_to_hir(iseq).unwrap(); - assert_snapshot!(hir_string_function(&function), @r#" - fn open@: - bb0(v0:BasicObject, v1:BasicObject, v2:BasicObject, v3:BasicObject, v4:BasicObject): - v5:NilClass = Const Value(nil) - v8:BasicObject = InvokeBuiltin dir_s_open, v0, v1, v2 - SideExit UnknownOpcode(getblockparamproxy) - "#); + assert_snapshot!(hir_string_function(&function), @r" + fn open@: + bb0(v0:BasicObject, v1:BasicObject, v2:BasicObject, v3:BasicObject, v4:BasicObject): + v5:NilClass = Const Value(nil) + v8:BasicObject = InvokeBuiltin dir_s_open, v0, v1, v2 + SideExit UnhandledYARVInsn(getblockparamproxy) + "); } #[test] diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 16010a1ea8dc5b..03a4a037d4f155 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -7,12 +7,16 @@ macro_rules! make_counters { default { $($default_counter_name:ident,)+ } + exit { + $($exit_counter_name:ident,)+ + } $($counter_name:ident,)+ ) => { /// Struct containing the counter values #[derive(Default, Debug)] pub struct Counters { $(pub $default_counter_name: u64,)+ + $(pub $exit_counter_name: u64,)+ $(pub $counter_name: u64,)+ } @@ -21,6 +25,7 @@ macro_rules! make_counters { #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum Counter { $($default_counter_name,)+ + $($exit_counter_name,)+ $($counter_name,)+ } @@ -28,6 +33,7 @@ macro_rules! make_counters { pub fn name(&self) -> String { match self { $( Counter::$default_counter_name => stringify!($default_counter_name).to_string(), )+ + $( Counter::$exit_counter_name => stringify!($exit_counter_name).to_string(), )+ $( Counter::$counter_name => stringify!($counter_name).to_string(), )+ } } @@ -38,6 +44,7 @@ macro_rules! make_counters { let counters = $crate::state::ZJITState::get_counters(); match counter { $( Counter::$default_counter_name => std::ptr::addr_of_mut!(counters.$default_counter_name), )+ + $( Counter::$exit_counter_name => std::ptr::addr_of_mut!(counters.$exit_counter_name), )+ $( Counter::$counter_name => std::ptr::addr_of_mut!(counters.$counter_name), )+ } } @@ -48,8 +55,13 @@ macro_rules! make_counters { $( Counter::$default_counter_name, )+ ]; - /// List of counters that are available only for --zjit-stats. - pub const STATS_ONLY_COUNTERS: &'static [Counter] = &[ + /// List of other counters that are summed as side_exit_count. + pub const EXIT_COUNTERS: &'static [Counter] = &[ + $( Counter::$exit_counter_name, )+ + ]; + + /// List of other counters that are available only for --zjit-stats. + pub const OTHER_COUNTERS: &'static [Counter] = &[ $( Counter::$counter_name, )+ ]; } @@ -68,11 +80,26 @@ make_counters! { invalidation_time_ns, } - // The number of times YARV instructions are executed on JIT code - zjit_insn_count, - - // The number of times we do a dynamic dispatch from JIT code - dynamic_send_count, + // Exit counters that are summed as side_exit_count + exit { + // exit_: Side exits reasons + exit_compilation_failure, + exit_unknown_newarray_send, + exit_unknown_call_type, + exit_unknown_special_variable, + exit_unhandled_defined_type, + exit_unhandled_hir_insn, + exit_unhandled_yarv_insn, + exit_fixnum_add_overflow, + exit_fixnum_sub_overflow, + exit_fixnum_mult_overflow, + exit_guard_type_failure, + exit_guard_bit_equals_failure, + exit_patchpoint, + exit_callee_side_exit, + exit_obj_to_string_fallback, + exit_interrupt, + } // failed_: Compilation failure reasons failed_iseq_stack_too_large, @@ -81,25 +108,11 @@ make_counters! { failed_hir_optimize, failed_asm_compile, - // exit_: Side exit reasons (ExitCounters shares the same prefix) - exit_compilation_failure, - - // specific_exit_: Side exits counted by type, not by PC - specific_exit_unknown_newarray_send, - specific_exit_unknown_call_type, - specific_exit_unknown_opcode, - specific_exit_unknown_special_variable, - specific_exit_unhandled_instruction, - specific_exit_unhandled_defined_type, - specific_exit_fixnum_add_overflow, - specific_exit_fixnum_sub_overflow, - specific_exit_fixnum_mult_overflow, - specific_exit_guard_type_failure, - specific_exit_guard_bit_equals_failure, - specific_exit_patchpoint, - specific_exit_callee_side_exit, - specific_exit_obj_to_string_fallback, - specific_exit_interrupt, + // The number of times YARV instructions are executed on JIT code + zjit_insn_count, + + // The number of times we do a dynamic dispatch from JIT code + dynamic_send_count, } /// Increase a counter by a specified amount @@ -119,32 +132,31 @@ pub(crate) use incr_counter; /// The number of side exits from each YARV instruction pub type ExitCounters = [u64; VM_INSTRUCTION_SIZE as usize]; -/// Return a raw pointer to the exit counter for the YARV instruction at a given PC -pub fn exit_counter_ptr(pc: *const VALUE) -> *mut u64 { - let opcode = unsafe { rb_vm_insn_addr2opcode((*pc).as_ptr()) }; +/// Return a raw pointer to the exit counter for a given YARV opcode +pub fn exit_counter_ptr_for_opcode(opcode: u32) -> *mut u64 { let exit_counters = ZJITState::get_exit_counters(); unsafe { exit_counters.get_unchecked_mut(opcode as usize) } } -pub fn specific_exit_counter_ptr(reason: crate::hir::SideExitReason) -> *mut u64 { +pub fn exit_counter_ptr(reason: crate::hir::SideExitReason) -> *mut u64 { use crate::hir::SideExitReason::*; use crate::stats::Counter::*; let counter = match reason { - UnknownNewarraySend(_) => specific_exit_unknown_newarray_send, - UnknownCallType => specific_exit_unknown_call_type, - UnknownOpcode(_) => specific_exit_unknown_opcode, - UnknownSpecialVariable(_) => specific_exit_unknown_special_variable, - UnhandledInstruction(_) => specific_exit_unhandled_instruction, - UnhandledDefinedType(_) => specific_exit_unhandled_defined_type, - FixnumAddOverflow => specific_exit_fixnum_add_overflow, - FixnumSubOverflow => specific_exit_fixnum_sub_overflow, - FixnumMultOverflow => specific_exit_fixnum_mult_overflow, - GuardType(_) => specific_exit_guard_type_failure, - GuardBitEquals(_) => specific_exit_guard_bit_equals_failure, - PatchPoint(_) => specific_exit_patchpoint, - CalleeSideExit => specific_exit_callee_side_exit, - ObjToStringFallback => specific_exit_obj_to_string_fallback, - Interrupt => specific_exit_interrupt, + UnknownNewarraySend(_) => exit_unknown_newarray_send, + UnknownCallType => exit_unknown_call_type, + UnknownSpecialVariable(_) => exit_unknown_special_variable, + UnhandledDefinedType(_) => exit_unhandled_defined_type, + UnhandledHIRInsn(_) => exit_unhandled_hir_insn, + UnhandledYARVInsn(_) => exit_unhandled_yarv_insn, + FixnumAddOverflow => exit_fixnum_add_overflow, + FixnumSubOverflow => exit_fixnum_sub_overflow, + FixnumMultOverflow => exit_fixnum_mult_overflow, + GuardType(_) => exit_guard_type_failure, + GuardBitEquals(_) => exit_guard_bit_equals_failure, + PatchPoint(_) => exit_patchpoint, + CalleeSideExit => exit_callee_side_exit, + ObjToStringFallback => exit_obj_to_string_fallback, + Interrupt => exit_interrupt, }; counter_ptr(counter) } @@ -199,36 +211,39 @@ pub extern "C" fn rb_zjit_stats(_ec: EcPtr, _self: VALUE, target_key: VALUE) -> return hash; } - // Set stats-only counters - for &counter in STATS_ONLY_COUNTERS { + // Set other stats-only counters + for &counter in OTHER_COUNTERS { set_stat_usize!(hash, &counter.name(), unsafe { *counter_ptr(counter) }); } - // Set side exit stats - let exit_counters = ZJITState::get_exit_counters(); + // Set side-exit counters for each SideExitReason let mut side_exit_count = 0; + for &counter in EXIT_COUNTERS { + let count = unsafe { *counter_ptr(counter) }; + side_exit_count += count; + set_stat_usize!(hash, &counter.name(), count); + } + set_stat_usize!(hash, "side_exit_count", side_exit_count); + + // Set side-exit counters for UnhandledYARVInsn + let exit_counters = ZJITState::get_exit_counters(); for op_idx in 0..VM_INSTRUCTION_SIZE as usize { let op_name = insn_name(op_idx); - let key_string = "exit_".to_owned() + &op_name; + let key_string = "unhandled_yarv_insn_".to_owned() + &op_name; let count = exit_counters[op_idx]; - side_exit_count += count; set_stat_usize!(hash, &key_string, count); } - set_stat_usize!(hash, "side_exit_count", side_exit_count); - - // Share compilation_failure among both prefixes for side-exit stats - let counters = ZJITState::get_counters(); - set_stat_usize!(hash, "specific_exit_compilation_failure", counters.exit_compilation_failure); // Only ZJIT_STATS builds support rb_vm_insn_count if unsafe { rb_vm_insn_count } > 0 { let vm_insn_count = unsafe { rb_vm_insn_count }; set_stat_usize!(hash, "vm_insn_count", vm_insn_count); - let total_insn_count = vm_insn_count + counters.zjit_insn_count; + let zjit_insn_count = ZJITState::get_counters().zjit_insn_count; + let total_insn_count = vm_insn_count + zjit_insn_count; set_stat_usize!(hash, "total_insn_count", total_insn_count); - set_stat_f64!(hash, "ratio_in_zjit", 100.0 * counters.zjit_insn_count as f64 / total_insn_count as f64); + set_stat_f64!(hash, "ratio_in_zjit", 100.0 * zjit_insn_count as f64 / total_insn_count as f64); } hash From 7f4a6afab81c8e33fb4a6bbaceca96b6672dc7aa Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Fri, 29 Aug 2025 09:32:38 -0700 Subject: [PATCH 2/8] ZJIT: Remove special case for defined?(method call) (#14401) This was fixed in https://github.com/ruby/ruby/pull/14308 Thanks, Stan, for noticing. Fixes https://github.com/Shopify/ruby/issues/703 --- test/ruby/test_zjit.rb | 23 +++++++++++++++++++++++ zjit/src/hir.rs | 6 ------ zjit/src/stats.rs | 2 -- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index c11f7bfa9a7106..98f747c4d0b0a3 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -1355,6 +1355,29 @@ def test = return defined?("x".reverse(1)), defined?("x".reverse(1).reverse) }, insns: [:defined] end + def test_defined_method_raise + assert_compiles '[nil, nil, nil]', %q{ + class C + def assert_equal expected, actual + if expected != actual + raise "NO" + end + end + + def test_defined_method + assert_equal(nil, defined?("x".reverse(1).reverse)) + end + end + + c = C.new + result = [] + result << c.test_defined_method + result << c.test_defined_method + result << c.test_defined_method + result + } + end + def test_defined_yield assert_compiles "nil", "defined?(yield)" assert_compiles '[nil, nil, "yield"]', %q{ diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 1628e049cc8ad5..fd19edaf0412a2 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -431,7 +431,6 @@ pub enum SideExitReason { UnknownNewarraySend(vm_opt_newarray_send_type), UnknownCallType, UnknownSpecialVariable(u64), - UnhandledDefinedType(usize), UnhandledHIRInsn(InsnId), UnhandledYARVInsn(u32), FixnumAddOverflow, @@ -3049,11 +3048,6 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let pushval = get_arg(pc, 2); let v = state.stack_pop()?; let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - if op_type == DEFINED_METHOD.try_into().unwrap() { - // TODO(Shopify/ruby#703): Fix codegen for defined?(method call expr) - fun.push_insn(block, Insn::SideExit { state: exit_id, reason: SideExitReason::UnhandledDefinedType(op_type)}); - break; // End the block - } state.stack_push(fun.push_insn(block, Insn::Defined { op_type, obj, pushval, v, state: exit_id })); } YARVINSN_definedivar => { diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 03a4a037d4f155..64bef8ca6a7932 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -87,7 +87,6 @@ make_counters! { exit_unknown_newarray_send, exit_unknown_call_type, exit_unknown_special_variable, - exit_unhandled_defined_type, exit_unhandled_hir_insn, exit_unhandled_yarv_insn, exit_fixnum_add_overflow, @@ -145,7 +144,6 @@ pub fn exit_counter_ptr(reason: crate::hir::SideExitReason) -> *mut u64 { UnknownNewarraySend(_) => exit_unknown_newarray_send, UnknownCallType => exit_unknown_call_type, UnknownSpecialVariable(_) => exit_unknown_special_variable, - UnhandledDefinedType(_) => exit_unhandled_defined_type, UnhandledHIRInsn(_) => exit_unhandled_hir_insn, UnhandledYARVInsn(_) => exit_unhandled_yarv_insn, FixnumAddOverflow => exit_fixnum_add_overflow, From fc4f8c879a362968d87d36d6f93d30ae9abd0a5b Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Fri, 29 Aug 2025 09:40:15 -0700 Subject: [PATCH 3/8] YJIT: Drop yjit-bench CI job (#14394) --- .github/workflows/yjit-ubuntu.yml | 13 ------------- yjit/yjit.mk | 13 ------------- 2 files changed, 26 deletions(-) diff --git a/.github/workflows/yjit-ubuntu.yml b/.github/workflows/yjit-ubuntu.yml index 3b3b75faac8054..c4d1c658446f0d 100644 --- a/.github/workflows/yjit-ubuntu.yml +++ b/.github/workflows/yjit-ubuntu.yml @@ -101,18 +101,12 @@ jobs: - test_task: 'test-bundled-gems' configure: '--enable-yjit=dev' - - test_task: 'yjit-bench' - configure: '--enable-yjit=dev' - yjit_bench_opts: '--yjit-stats' - continue-on-test_task: true - env: GITPULLOPTIONS: --no-tags origin ${{ github.ref }} RUN_OPTS: ${{ matrix.yjit_opts }} YJIT_BENCH_OPTS: ${{ matrix.yjit_bench_opts }} SPECOPTS: ${{ matrix.specopts }} RUBY_DEBUG: ci - BUNDLE_JOBS: 8 # for yjit-bench RUST_BACKTRACE: 1 runs-on: ubuntu-22.04 @@ -208,13 +202,6 @@ jobs: LAUNCHABLE_STDERR: ${{ steps.launchable.outputs.stderr_report_path }} continue-on-error: ${{ matrix.continue-on-test_task || false }} - - name: Show ${{ github.event.pull_request.base.ref }} GitHub URL for yjit-bench comparison - run: echo "https://github.com/${BASE_REPO}/commit/${BASE_SHA}" - env: - BASE_REPO: ${{ github.event.pull_request.base.repo.full_name }} - BASE_SHA: ${{ github.event.pull_request.base.sha }} - if: ${{ matrix.test_task == 'yjit-bench' && startsWith(github.event_name, 'pull') }} - - uses: ./.github/actions/slack with: label: ${{ matrix.test_task }} ${{ matrix.configure }} diff --git a/yjit/yjit.mk b/yjit/yjit.mk index c6571ecbc80c54..39587928d4d18b 100644 --- a/yjit/yjit.mk +++ b/yjit/yjit.mk @@ -31,19 +31,6 @@ ifneq ($(YJIT_SUPPORT),no) $(RUST_LIB): $(YJIT_SRC_FILES) endif -# By using YJIT_BENCH_OPTS instead of RUN_OPTS, you can skip passing the options to `make install` -YJIT_BENCH_OPTS = $(RUN_OPTS) --enable-gems -YJIT_BENCH = benchmarks/railsbench/benchmark.rb - -# Run yjit-bench's ./run_once.sh for CI -yjit-bench: install update-yjit-bench PHONY - $(Q) cd $(srcdir)/yjit-bench && PATH=$(prefix)/bin:$$PATH \ - ./run_once.sh $(YJIT_BENCH_OPTS) $(YJIT_BENCH) - -update-yjit-bench: - $(Q) $(tooldir)/git-refresh -C $(srcdir) --branch main \ - https://github.com/Shopify/yjit-bench yjit-bench $(GIT_OPTS) - RUST_VERSION = +1.58.0 # Gives quick feedback about YJIT. Not a replacement for a full test run. From b6f4b5399d22aab18fa3d163b9cc1764ad0df7dd Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Fri, 29 Aug 2025 09:46:08 -0700 Subject: [PATCH 4/8] ZJIT: Specialize monomorphic GetIvar (#14388) Specialize monomorphic `GetIvar` into: * `GuardType(HeapObject)` * `GuardShape` * `LoadIvarEmbedded` or `LoadIvarExtended` This requires profiling self for `getinstancevariable` (it's not on the operand stack). This also optimizes `GetIvar`s that happen as a result of inlining `attr_reader` and `attr_accessor`. Also move some (newly) shared JIT helpers into jit.c. --- insns.def | 1 + jit.c | 12 ++ test/ruby/test_zjit.rb | 30 +++++ yjit.c | 12 -- yjit/bindgen/src/main.rs | 4 +- yjit/src/codegen.rs | 2 +- yjit/src/cruby_bindings.inc.rs | 8 +- zjit.c | 6 - zjit/bindgen/src/main.rs | 3 +- zjit/src/codegen.rs | 41 ++++++ zjit/src/cruby.rs | 14 +- zjit/src/cruby_bindings.inc.rs | 42 +++--- zjit/src/hir.rs | 240 ++++++++++++++++++++++++++++++--- zjit/src/options.rs | 8 ++ zjit/src/profile.rs | 24 +++- zjit/src/stats.rs | 2 + 16 files changed, 384 insertions(+), 65 deletions(-) diff --git a/insns.def b/insns.def index c9de6129120fdc..953f6bb87ccd28 100644 --- a/insns.def +++ b/insns.def @@ -212,6 +212,7 @@ getinstancevariable (VALUE val) /* Ractor crashes when it accesses class/module-level instances variables. */ // attr bool leaf = false; /* has IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR() */ +// attr bool zjit_profile = true; { val = vm_getinstancevariable(GET_ISEQ(), GET_SELF(), id, ic); } diff --git a/jit.c b/jit.c index e68758368a45fb..7d7b5fd4fd0b2f 100644 --- a/jit.c +++ b/jit.c @@ -14,6 +14,12 @@ #include "iseq.h" #include "internal/gc.h" +// Field offsets for the RObject struct +enum robject_offsets { + ROBJECT_OFFSET_AS_HEAP_FIELDS = offsetof(struct RObject, as.heap.fields), + ROBJECT_OFFSET_AS_ARY = offsetof(struct RObject, as.ary), +}; + unsigned int rb_iseq_encoded_size(const rb_iseq_t *iseq) { @@ -454,3 +460,9 @@ rb_set_cfp_sp(struct rb_control_frame_struct *cfp, VALUE *sp) { cfp->sp = sp; } + +bool +rb_jit_shape_too_complex_p(shape_id_t shape_id) +{ + return rb_shape_too_complex_p(shape_id); +} diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index 98f747c4d0b0a3..8e450458dd42fc 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -1162,6 +1162,36 @@ def test() = @foo } end + def test_getinstancevariable_miss + assert_compiles '[1, 1, 4]', %q{ + class C + def foo + @foo + end + + def foo_then_bar + @foo = 1 + @bar = 2 + end + + def bar_then_foo + @bar = 3 + @foo = 4 + end + end + + o1 = C.new + o1.foo_then_bar + result = [] + result << o1.foo + result << o1.foo + o2 = C.new + o2.bar_then_foo + result << o2.foo + result + } + end + def test_setinstancevariable assert_compiles '1', %q{ def test() = @foo = 1 diff --git a/yjit.c b/yjit.c index 44788eaf2c02fd..cadae3a5df5f83 100644 --- a/yjit.c +++ b/yjit.c @@ -39,12 +39,6 @@ #include -// Field offsets for the RObject struct -enum robject_offsets { - ROBJECT_OFFSET_AS_HEAP_FIELDS = offsetof(struct RObject, as.heap.fields), - ROBJECT_OFFSET_AS_ARY = offsetof(struct RObject, as.ary), -}; - // Field offsets for the RString struct enum rstring_offsets { RUBY_OFFSET_RSTRING_LEN = offsetof(struct RString, len) @@ -758,12 +752,6 @@ rb_object_shape_count(void) return ULONG2NUM((unsigned long)rb_shapes_count()); } -bool -rb_yjit_shape_too_complex_p(shape_id_t shape_id) -{ - return rb_shape_too_complex_p(shape_id); -} - bool rb_yjit_shape_obj_too_complex_p(VALUE obj) { diff --git a/yjit/bindgen/src/main.rs b/yjit/bindgen/src/main.rs index 2993e05e2f22ee..5cf5caeb9af433 100644 --- a/yjit/bindgen/src/main.rs +++ b/yjit/bindgen/src/main.rs @@ -99,7 +99,6 @@ fn main() { .allowlist_function("rb_shape_get_iv_index") .allowlist_function("rb_shape_transition_add_ivar_no_warnings") .allowlist_function("rb_yjit_shape_obj_too_complex_p") - .allowlist_function("rb_yjit_shape_too_complex_p") .allowlist_function("rb_yjit_shape_capacity") .allowlist_function("rb_yjit_shape_index") .allowlist_var("SHAPE_ID_NUM_BITS") @@ -351,11 +350,12 @@ fn main() { .allowlist_function("rb_yjit_invokeblock_sp_pops") .allowlist_function("rb_yjit_set_exception_return") .allowlist_function("rb_yjit_str_concat_codepoint") - .allowlist_type("robject_offsets") .allowlist_type("rstring_offsets") // From jit.c .allowlist_function("rb_assert_holding_vm_lock") + .allowlist_function("rb_jit_shape_too_complex_p") + .allowlist_type("robject_offsets") // from vm_sync.h .allowlist_function("rb_vm_barrier") diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 2dae72280fae69..9e9759e7812002 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -3103,7 +3103,7 @@ fn gen_set_ivar( // If the VM ran out of shapes, or this class generated too many leaf, // it may be de-optimized into OBJ_TOO_COMPLEX_SHAPE (hash-table). - new_shape_too_complex = unsafe { rb_yjit_shape_too_complex_p(next_shape_id) }; + new_shape_too_complex = unsafe { rb_jit_shape_too_complex_p(next_shape_id) }; if new_shape_too_complex { Some((next_shape_id, None, 0_usize)) } else { diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index ca459ceafbbe7a..c22f4490db8a0c 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -994,12 +994,12 @@ pub const DEFINED_REF: defined_type = 15; pub const DEFINED_FUNC: defined_type = 16; pub const DEFINED_CONST_FROM: defined_type = 17; pub type defined_type = u32; -pub const ROBJECT_OFFSET_AS_HEAP_FIELDS: robject_offsets = 16; -pub const ROBJECT_OFFSET_AS_ARY: robject_offsets = 16; -pub type robject_offsets = u32; pub const RUBY_OFFSET_RSTRING_LEN: rstring_offsets = 16; pub type rstring_offsets = u32; pub type rb_seq_param_keyword_struct = rb_iseq_constant_body__bindgen_ty_1_rb_iseq_param_keyword; +pub const ROBJECT_OFFSET_AS_HEAP_FIELDS: robject_offsets = 16; +pub const ROBJECT_OFFSET_AS_ARY: robject_offsets = 16; +pub type robject_offsets = u32; pub type rb_iseq_param_keyword_struct = rb_iseq_constant_body__bindgen_ty_1_rb_iseq_param_keyword; extern "C" { pub fn ruby_xfree(ptr: *mut ::std::os::raw::c_void); @@ -1238,7 +1238,6 @@ extern "C" { line: ::std::os::raw::c_int, ); pub fn rb_object_shape_count() -> VALUE; - pub fn rb_yjit_shape_too_complex_p(shape_id: shape_id_t) -> bool; pub fn rb_yjit_shape_obj_too_complex_p(obj: VALUE) -> bool; pub fn rb_yjit_shape_capacity(shape_id: shape_id_t) -> attr_index_t; pub fn rb_yjit_shape_index(shape_id: shape_id_t) -> attr_index_t; @@ -1328,4 +1327,5 @@ extern "C" { pub fn rb_yarv_ary_entry_internal(ary: VALUE, offset: ::std::os::raw::c_long) -> VALUE; pub fn rb_set_cfp_pc(cfp: *mut rb_control_frame_struct, pc: *const VALUE); pub fn rb_set_cfp_sp(cfp: *mut rb_control_frame_struct, sp: *mut VALUE); + pub fn rb_jit_shape_too_complex_p(shape_id: shape_id_t) -> bool; } diff --git a/zjit.c b/zjit.c index 311bc1fe8098e6..8a7314cb7f92f4 100644 --- a/zjit.c +++ b/zjit.c @@ -337,12 +337,6 @@ rb_zjit_print_exception(void) rb_warn("Ruby error: %"PRIsVALUE"", rb_funcall(exception, rb_intern("full_message"), 0)); } -bool -rb_zjit_shape_obj_too_complex_p(VALUE obj) -{ - return rb_shape_obj_too_complex_p(obj); -} - enum { RB_INVALID_SHAPE_ID = INVALID_SHAPE_ID, }; diff --git a/zjit/bindgen/src/main.rs b/zjit/bindgen/src/main.rs index 0eb800e1f99f72..ef8537cbbbdd0f 100644 --- a/zjit/bindgen/src/main.rs +++ b/zjit/bindgen/src/main.rs @@ -111,7 +111,6 @@ fn main() { .allowlist_function("rb_shape_id_offset") .allowlist_function("rb_shape_get_iv_index") .allowlist_function("rb_shape_transition_add_ivar_no_warnings") - .allowlist_function("rb_zjit_shape_obj_too_complex_p") .allowlist_var("SHAPE_ID_NUM_BITS") // From ruby/internal/intern/object.h @@ -367,6 +366,8 @@ fn main() { // From jit.c .allowlist_function("rb_assert_holding_vm_lock") + .allowlist_function("rb_jit_shape_too_complex_p") + .allowlist_type("robject_offsets") // from vm_sync.h .allowlist_function("rb_vm_barrier") diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 36a899438c6936..441bb9e2f7f1a8 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -405,6 +405,9 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio &Insn::ToArray { val, state } => { gen_to_array(jit, asm, opnd!(val), &function.frame_state(state)) }, &Insn::DefinedIvar { self_val, id, pushval, .. } => { gen_defined_ivar(asm, opnd!(self_val), id, pushval) }, &Insn::ArrayExtend { left, right, state } => { no_output!(gen_array_extend(jit, asm, opnd!(left), opnd!(right), &function.frame_state(state))) }, + &Insn::GuardShape { val, shape, state } => gen_guard_shape(jit, asm, opnd!(val), shape, &function.frame_state(state)), + &Insn::LoadIvarEmbedded { self_val, id, index } => gen_load_ivar_embedded(asm, opnd!(self_val), id, index), + &Insn::LoadIvarExtended { self_val, id, index } => gen_load_ivar_extended(asm, opnd!(self_val), id, index), &Insn::ArrayMax { state, .. } | &Insn::FixnumDiv { state, .. } | &Insn::FixnumMod { state, .. } @@ -716,6 +719,38 @@ fn gen_array_extend(jit: &mut JITState, asm: &mut Assembler, left: Opnd, right: asm_ccall!(asm, rb_ary_concat, left, right); } +fn gen_guard_shape(jit: &mut JITState, asm: &mut Assembler, val: Opnd, shape: ShapeId, state: &FrameState) -> Opnd { + let shape_id_offset = unsafe { rb_shape_id_offset() }; + let val = asm.load(val); + let shape_opnd = Opnd::mem(SHAPE_ID_NUM_BITS as u8, val, shape_id_offset); + asm.cmp(shape_opnd, Opnd::UImm(shape.0 as u64)); + asm.jne(side_exit(jit, state, SideExitReason::GuardShape(shape))); + val +} + +fn gen_load_ivar_embedded(asm: &mut Assembler, self_val: Opnd, id: ID, index: u16) -> Opnd { + // See ROBJECT_FIELDS() from include/ruby/internal/core/robject.h + + asm_comment!(asm, "Load embedded ivar id={} index={}", id.contents_lossy(), index); + let offs = ROBJECT_OFFSET_AS_ARY as i32 + (SIZEOF_VALUE * index as usize) as i32; + let self_val = asm.load(self_val); + let ivar_opnd = Opnd::mem(64, self_val, offs); + asm.load(ivar_opnd) +} + +fn gen_load_ivar_extended(asm: &mut Assembler, self_val: Opnd, id: ID, index: u16) -> Opnd { + asm_comment!(asm, "Load extended ivar id={} index={}", id.contents_lossy(), index); + // Compile time value is *not* embedded. + + // Get a pointer to the extended table + let self_val = asm.load(self_val); + let tbl_opnd = asm.load(Opnd::mem(64, self_val, ROBJECT_OFFSET_AS_HEAP_FIELDS as i32)); + + // Read the ivar from the extended table + let ivar_opnd = Opnd::mem(64, tbl_opnd, (SIZEOF_VALUE * index as usize) as i32); + asm.load(ivar_opnd) +} + /// Compile an interpreter entry block to be inserted into an ISEQ fn gen_entry_prologue(asm: &mut Assembler, iseq: IseqPtr) { asm_comment!(asm, "ZJIT entry point: {}", iseq_get_location(iseq, 0)); @@ -1270,6 +1305,12 @@ fn gen_guard_type(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, guard asm.cmp(klass, Opnd::Value(expected_class)); asm.jne(side_exit); + } else if guard_type.bit_equal(types::HeapObject) { + let side_exit = side_exit(jit, state, GuardType(guard_type)); + asm.cmp(val, Opnd::Value(Qfalse)); + asm.je(side_exit.clone()); + asm.test(val, (RUBY_IMMEDIATE_MASK as u64).into()); + asm.jnz(side_exit); } else { unimplemented!("unsupported type: {guard_type}"); } diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index 015dd7f9122f05..5a34e5a8de61c8 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -271,6 +271,16 @@ pub struct ShapeId(pub u32); pub const INVALID_SHAPE_ID: ShapeId = ShapeId(RB_INVALID_SHAPE_ID); +impl ShapeId { + pub fn is_valid(self) -> bool { + self != INVALID_SHAPE_ID + } + + pub fn is_too_complex(self) -> bool { + unsafe { rb_jit_shape_too_complex_p(self.0) } + } +} + // Given an ISEQ pointer, convert PC to insn_idx pub fn iseq_pc_to_insn_idx(iseq: IseqPtr, pc: *mut VALUE) -> Option { let pc_zero = unsafe { rb_iseq_pc_at_idx(iseq, 0) }; @@ -489,10 +499,6 @@ impl VALUE { unsafe { rb_obj_frozen_p(self) != VALUE(0) } } - pub fn shape_too_complex(self) -> bool { - unsafe { rb_zjit_shape_obj_too_complex_p(self) } - } - pub fn shape_id_of(self) -> ShapeId { ShapeId(unsafe { rb_obj_shape_id(self) }) } diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index 4975308f68d6b6..c2f4b37a7ade2a 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -681,24 +681,25 @@ pub const YARVINSN_trace_setlocal_WC_0: ruby_vminsn_type = 214; pub const YARVINSN_trace_setlocal_WC_1: ruby_vminsn_type = 215; pub const YARVINSN_trace_putobject_INT2FIX_0_: ruby_vminsn_type = 216; pub const YARVINSN_trace_putobject_INT2FIX_1_: ruby_vminsn_type = 217; -pub const YARVINSN_zjit_opt_send_without_block: ruby_vminsn_type = 218; -pub const YARVINSN_zjit_opt_nil_p: ruby_vminsn_type = 219; -pub const YARVINSN_zjit_opt_plus: ruby_vminsn_type = 220; -pub const YARVINSN_zjit_opt_minus: ruby_vminsn_type = 221; -pub const YARVINSN_zjit_opt_mult: ruby_vminsn_type = 222; -pub const YARVINSN_zjit_opt_div: ruby_vminsn_type = 223; -pub const YARVINSN_zjit_opt_mod: ruby_vminsn_type = 224; -pub const YARVINSN_zjit_opt_eq: ruby_vminsn_type = 225; -pub const YARVINSN_zjit_opt_neq: ruby_vminsn_type = 226; -pub const YARVINSN_zjit_opt_lt: ruby_vminsn_type = 227; -pub const YARVINSN_zjit_opt_le: ruby_vminsn_type = 228; -pub const YARVINSN_zjit_opt_gt: ruby_vminsn_type = 229; -pub const YARVINSN_zjit_opt_ge: ruby_vminsn_type = 230; -pub const YARVINSN_zjit_opt_and: ruby_vminsn_type = 231; -pub const YARVINSN_zjit_opt_or: ruby_vminsn_type = 232; -pub const YARVINSN_zjit_opt_empty_p: ruby_vminsn_type = 233; -pub const YARVINSN_zjit_opt_not: ruby_vminsn_type = 234; -pub const VM_INSTRUCTION_SIZE: ruby_vminsn_type = 235; +pub const YARVINSN_zjit_getinstancevariable: ruby_vminsn_type = 218; +pub const YARVINSN_zjit_opt_send_without_block: ruby_vminsn_type = 219; +pub const YARVINSN_zjit_opt_nil_p: ruby_vminsn_type = 220; +pub const YARVINSN_zjit_opt_plus: ruby_vminsn_type = 221; +pub const YARVINSN_zjit_opt_minus: ruby_vminsn_type = 222; +pub const YARVINSN_zjit_opt_mult: ruby_vminsn_type = 223; +pub const YARVINSN_zjit_opt_div: ruby_vminsn_type = 224; +pub const YARVINSN_zjit_opt_mod: ruby_vminsn_type = 225; +pub const YARVINSN_zjit_opt_eq: ruby_vminsn_type = 226; +pub const YARVINSN_zjit_opt_neq: ruby_vminsn_type = 227; +pub const YARVINSN_zjit_opt_lt: ruby_vminsn_type = 228; +pub const YARVINSN_zjit_opt_le: ruby_vminsn_type = 229; +pub const YARVINSN_zjit_opt_gt: ruby_vminsn_type = 230; +pub const YARVINSN_zjit_opt_ge: ruby_vminsn_type = 231; +pub const YARVINSN_zjit_opt_and: ruby_vminsn_type = 232; +pub const YARVINSN_zjit_opt_or: ruby_vminsn_type = 233; +pub const YARVINSN_zjit_opt_empty_p: ruby_vminsn_type = 234; +pub const YARVINSN_zjit_opt_not: ruby_vminsn_type = 235; +pub const VM_INSTRUCTION_SIZE: ruby_vminsn_type = 236; pub type ruby_vminsn_type = u32; pub type rb_iseq_callback = ::std::option::Option< unsafe extern "C" fn(arg1: *const rb_iseq_t, arg2: *mut ::std::os::raw::c_void), @@ -724,6 +725,9 @@ pub const DEFINED_CONST_FROM: defined_type = 17; pub type defined_type = u32; pub const RB_INVALID_SHAPE_ID: _bindgen_ty_38 = 4294967295; pub type _bindgen_ty_38 = u32; +pub const ROBJECT_OFFSET_AS_HEAP_FIELDS: robject_offsets = 16; +pub const ROBJECT_OFFSET_AS_ARY: robject_offsets = 16; +pub type robject_offsets = u32; pub type rb_iseq_param_keyword_struct = rb_iseq_constant_body__bindgen_ty_1_rb_iseq_param_keyword; unsafe extern "C" { pub fn ruby_xfree(ptr: *mut ::std::os::raw::c_void); @@ -943,7 +947,6 @@ unsafe extern "C" { pub fn rb_iseq_get_zjit_payload(iseq: *const rb_iseq_t) -> *mut ::std::os::raw::c_void; pub fn rb_iseq_set_zjit_payload(iseq: *const rb_iseq_t, payload: *mut ::std::os::raw::c_void); pub fn rb_zjit_print_exception(); - pub fn rb_zjit_shape_obj_too_complex_p(obj: VALUE) -> bool; pub fn rb_zjit_singleton_class_p(klass: VALUE) -> bool; pub fn rb_zjit_defined_ivar(obj: VALUE, id: ID, pushval: VALUE) -> VALUE; pub fn rb_iseq_encoded_size(iseq: *const rb_iseq_t) -> ::std::os::raw::c_uint; @@ -1025,4 +1028,5 @@ unsafe extern "C" { pub fn rb_yarv_ary_entry_internal(ary: VALUE, offset: ::std::os::raw::c_long) -> VALUE; pub fn rb_set_cfp_pc(cfp: *mut rb_control_frame_struct, pc: *const VALUE); pub fn rb_set_cfp_sp(cfp: *mut rb_control_frame_struct, sp: *mut VALUE); + pub fn rb_jit_shape_too_complex_p(shape_id: shape_id_t) -> bool; } diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index fd19edaf0412a2..f4e14eccd167d0 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -424,6 +424,16 @@ impl PtrPrintMap { fn map_id(&self, id: u64) -> *const c_void { self.map_ptr(id as *const c_void) } + + /// Map an index into a Ruby object (e.g. for an ivar) for printing + fn map_index(&self, id: u64) -> *const c_void { + self.map_ptr(id as *const c_void) + } + + /// Map shape ID into a pointer for printing + fn map_shape(&self, id: ShapeId) -> *const c_void { + self.map_ptr(id.0 as *const c_void) + } } #[derive(Debug, Clone, Copy)] @@ -437,6 +447,7 @@ pub enum SideExitReason { FixnumSubOverflow, FixnumMultOverflow, GuardType(Type), + GuardShape(ShapeId), GuardBitEquals(VALUE), PatchPoint(Invariant), CalleeSideExit, @@ -520,6 +531,11 @@ pub enum Insn { /// Check whether an instance variable exists on `self_val` DefinedIvar { self_val: InsnId, id: ID, pushval: VALUE, state: InsnId }, + /// Read an instance variable at the given index, embedded in the object + LoadIvarEmbedded { self_val: InsnId, id: ID, index: u16 }, + /// Read an instance variable at the given index, from the extended table + LoadIvarExtended { self_val: InsnId, id: ID, index: u16 }, + /// Get a local variable from a higher scope or the heap GetLocal { level: u32, ep_offset: u32 }, /// Set a local variable in a higher scope or the heap @@ -592,6 +608,8 @@ pub enum Insn { GuardType { val: InsnId, guard_type: Type, state: InsnId }, /// Side-exit if val is not the expected VALUE. GuardBitEquals { val: InsnId, expected: VALUE, state: InsnId }, + /// Side-exit if val doesn't have the expected shape. + GuardShape { val: InsnId, shape: ShapeId, state: InsnId }, /// Generate no code (or padding if necessary) and insert a patch point /// that can be rewritten to a side exit when the Invariant is broken. @@ -665,6 +683,8 @@ impl Insn { Insn::FixnumOr { .. } => false, Insn::GetLocal { .. } => false, Insn::IsNil { .. } => false, + Insn::LoadIvarEmbedded { .. } => false, + Insn::LoadIvarExtended { .. } => false, Insn::CCall { elidable, .. } => !elidable, // TODO: NewRange is effects free if we can prove the two ends to be Fixnum, // but we don't have type information here in `impl Insn`. See rb_range_new(). @@ -810,6 +830,7 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> { Insn::FixnumOr { left, right, .. } => { write!(f, "FixnumOr {left}, {right}") }, Insn::GuardType { val, guard_type, .. } => { write!(f, "GuardType {val}, {}", guard_type.print(self.ptr_map)) }, Insn::GuardBitEquals { val, expected, .. } => { write!(f, "GuardBitEquals {val}, {}", expected.print(self.ptr_map)) }, + &Insn::GuardShape { val, shape, .. } => { write!(f, "GuardShape {val}, {:p}", self.ptr_map.map_shape(shape)) }, Insn::PatchPoint { invariant, .. } => { write!(f, "PatchPoint {}", invariant.print(self.ptr_map)) }, Insn::GetConstantPath { ic, .. } => { write!(f, "GetConstantPath {:p}", self.ptr_map.map_ptr(ic)) }, Insn::CCall { cfun, args, name, return_type: _, elidable: _ } => { @@ -838,6 +859,8 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> { } Insn::DefinedIvar { self_val, id, .. } => write!(f, "DefinedIvar {self_val}, :{}", id.contents_lossy()), Insn::GetIvar { self_val, id, .. } => write!(f, "GetIvar {self_val}, :{}", id.contents_lossy()), + &Insn::LoadIvarEmbedded { self_val, id, index } => write!(f, "LoadIvarEmbedded {self_val}, :{}@{:p}", id.contents_lossy(), self.ptr_map.map_index(index as u64)), + &Insn::LoadIvarExtended { self_val, id, index } => write!(f, "LoadIvarExtended {self_val}, :{}@{:p}", id.contents_lossy(), self.ptr_map.map_index(index as u64)), Insn::SetIvar { self_val, id, val, .. } => write!(f, "SetIvar {self_val}, :{}, {val}", id.contents_lossy()), Insn::GetGlobal { id, .. } => write!(f, "GetGlobal :{}", id.contents_lossy()), Insn::SetGlobal { id, val, .. } => write!(f, "SetGlobal :{}, {val}", id.contents_lossy()), @@ -1230,6 +1253,7 @@ impl Function { &IfFalse { val, ref target } => IfFalse { val: find!(val), target: find_branch_edge!(target) }, &GuardType { val, guard_type, state } => GuardType { val: find!(val), guard_type: guard_type, state }, &GuardBitEquals { val, expected, state } => GuardBitEquals { val: find!(val), expected: expected, state }, + &GuardShape { val, shape, state } => GuardShape { val: find!(val), shape, state }, &FixnumAdd { left, right, state } => FixnumAdd { left: find!(left), right: find!(right), state }, &FixnumSub { left, right, state } => FixnumSub { left: find!(left), right: find!(right), state }, &FixnumMult { left, right, state } => FixnumMult { left: find!(left), right: find!(right), state }, @@ -1292,6 +1316,8 @@ impl Function { &ArrayMax { ref elements, state } => ArrayMax { elements: find_vec!(elements), state: find!(state) }, &SetGlobal { id, val, state } => SetGlobal { id, val: find!(val), state }, &GetIvar { self_val, id, state } => GetIvar { self_val: find!(self_val), id, state }, + &LoadIvarEmbedded { self_val, id, index } => LoadIvarEmbedded { self_val: find!(self_val), id, index }, + &LoadIvarExtended { self_val, id, index } => LoadIvarExtended { self_val: find!(self_val), id, index }, &SetIvar { self_val, id, val, state } => SetIvar { self_val: find!(self_val), id, val: find!(val), state }, &SetLocal { val, ep_offset, level } => SetLocal { val: find!(val), ep_offset, level }, &GetSpecialSymbol { symbol_type, state } => GetSpecialSymbol { symbol_type, state }, @@ -1360,6 +1386,7 @@ impl Function { Insn::CCall { return_type, .. } => *return_type, Insn::GuardType { val, guard_type, .. } => self.type_of(*val).intersection(*guard_type), Insn::GuardBitEquals { val, expected, .. } => self.type_of(*val).intersection(Type::from_value(*expected)), + Insn::GuardShape { val, .. } => self.type_of(*val), Insn::FixnumAdd { .. } => types::Fixnum, Insn::FixnumSub { .. } => types::Fixnum, Insn::FixnumMult { .. } => types::Fixnum, @@ -1384,6 +1411,8 @@ impl Function { Insn::ArrayMax { .. } => types::BasicObject, Insn::GetGlobal { .. } => types::BasicObject, Insn::GetIvar { .. } => types::BasicObject, + Insn::LoadIvarEmbedded { .. } => types::BasicObject, + Insn::LoadIvarExtended { .. } => types::BasicObject, Insn::GetSpecialSymbol { .. } => types::BasicObject, Insn::GetSpecialNumber { .. } => types::BasicObject, Insn::ToNewArray { .. } => types::ArrayExact, @@ -1468,14 +1497,25 @@ impl Function { } } + fn chase_insn(&self, insn: InsnId) -> InsnId { + let id = self.union_find.borrow().find_const(insn); + match self.insns[id.0] { + Insn::GuardType { val, .. } => self.chase_insn(val), + Insn::GuardShape { val, .. } => self.chase_insn(val), + Insn::GuardBitEquals { val, .. } => self.chase_insn(val), + _ => id, + } + } + /// Return the interpreter-profiled type of the HIR instruction at the given ISEQ instruction /// index, if it is known. This historical type record is not a guarantee and must be checked /// with a GuardType or similar. fn profiled_type_of_at(&self, insn: InsnId, iseq_insn_idx: usize) -> Option { let Some(ref profiles) = self.profiles else { return None }; let Some(entries) = profiles.types.get(&iseq_insn_idx) else { return None }; + let insn = self.chase_insn(insn); for (entry_insn, entry_type_summary) in entries { - if self.union_find.borrow().find_const(*entry_insn) == self.union_find.borrow().find_const(insn) { + if self.union_find.borrow().find_const(*entry_insn) == insn { if entry_type_summary.is_monomorphic() || entry_type_summary.is_skewed_polymorphic() { return Some(entry_type_summary.bucket(0)); } else { @@ -1727,6 +1767,56 @@ impl Function { self.infer_types(); } + fn optimize_getivar(&mut self) { + for block in self.rpo() { + let old_insns = std::mem::take(&mut self.blocks[block.0].insns); + assert!(self.blocks[block.0].insns.is_empty()); + for insn_id in old_insns { + match self.find(insn_id) { + Insn::GetIvar { self_val, id, state } => { + let frame_state = self.frame_state(state); + let Some(recv_type) = self.profiled_type_of_at(self_val, frame_state.insn_idx) else { + // No (monomorphic) profile info + self.push_insn_id(block, insn_id); continue; + }; + if recv_type.flags().is_immediate() { + // Instance variable lookups on immediate values are always nil + self.push_insn_id(block, insn_id); continue; + } + assert!(recv_type.shape().is_valid()); + if !recv_type.flags().is_t_object() { + // Check if the receiver is a T_OBJECT + self.push_insn_id(block, insn_id); continue; + } + if recv_type.shape().is_too_complex() { + // too-complex shapes can't use index access + self.push_insn_id(block, insn_id); continue; + } + let self_val = self.push_insn(block, Insn::GuardType { val: self_val, guard_type: types::HeapObject, state }); + let self_val = self.push_insn(block, Insn::GuardShape { val: self_val, shape: recv_type.shape(), state }); + let mut ivar_index: u16 = 0; + let replacement = if ! unsafe { rb_shape_get_iv_index(recv_type.shape().0, id, &mut ivar_index) } { + // If there is no IVAR index, then the ivar was undefined when we + // entered the compiler. That means we can just return nil for this + // shape + iv name + Insn::Const { val: Const::Value(Qnil) } + } else { + if recv_type.flags().is_embedded() { + Insn::LoadIvarEmbedded { self_val, id, index: ivar_index } + } else { + Insn::LoadIvarExtended { self_val, id, index: ivar_index } + } + }; + let replacement = self.push_insn(block, replacement); + self.make_equal_to(insn_id, replacement); + } + _ => { self.push_insn_id(block, insn_id); } + } + } + } + self.infer_types(); + } + /// Optimize SendWithoutBlock that land in a C method to a direct CCall without /// runtime lookup. fn optimize_c_calls(&mut self) { @@ -2011,6 +2101,7 @@ impl Function { | &Insn::StringCopy { val, state, .. } | &Insn::GuardType { val, state, .. } | &Insn::GuardBitEquals { val, state, .. } + | &Insn::GuardShape { val, state, .. } | &Insn::ToArray { val, state } | &Insn::ToNewArray { val, state } => { worklist.push_back(val); @@ -2089,6 +2180,10 @@ impl Function { worklist.push_back(str); worklist.push_back(state); } + &Insn::LoadIvarEmbedded { self_val, .. } + | &Insn::LoadIvarExtended { self_val, .. } => { + worklist.push_back(self_val); + } &Insn::GetGlobal { state, .. } | &Insn::GetSpecialSymbol { state, .. } | &Insn::GetSpecialNumber { state, .. } | @@ -2232,6 +2327,8 @@ impl Function { // Function is assumed to have types inferred already self.optimize_direct_sends(); #[cfg(debug_assertions)] self.assert_validates(); + self.optimize_getivar(); + #[cfg(debug_assertions)] self.assert_validates(); self.optimize_c_calls(); #[cfg(debug_assertions)] self.assert_validates(); self.fold_constants(); @@ -2794,6 +2891,18 @@ impl ProfileOracle { entry.push((insn, TypeDistributionSummary::new(insn_type_distribution))) } } + + /// Map the interpreter-recorded types of self onto the HIR self + fn profile_self(&mut self, state: &FrameState, self_param: InsnId) { + let iseq_insn_idx = state.insn_idx; + let Some(operand_types) = self.payload.profile.get_operand_types(iseq_insn_idx) else { return }; + let entry = self.types.entry(iseq_insn_idx).or_insert_with(|| vec![]); + if operand_types.is_empty() { + return; + } + let self_type_distribution = &operand_types[0]; + entry.push((self_param, TypeDistributionSummary::new(self_type_distribution))) + } } /// The index of the self parameter in the HIR function @@ -2891,17 +3000,22 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let pc = unsafe { rb_iseq_pc_at_idx(iseq, insn_idx) }; state.pc = pc; let exit_state = state.clone(); - profiles.profile_stack(&exit_state); - - // Increment zjit_insn_count for each YARV instruction if --zjit-stats is enabled. - if get_option!(stats) { - fun.push_insn(block, Insn::IncrCounter(Counter::zjit_insn_count)); - } // try_into() call below is unfortunate. Maybe pick i32 instead of usize for opcodes. let opcode: u32 = unsafe { rb_iseq_opcode_at_pc(iseq, pc) } .try_into() .unwrap(); + + if opcode == YARVINSN_getinstancevariable { + profiles.profile_self(&exit_state, self_param); + } else { + profiles.profile_stack(&exit_state); + } + + // Increment zjit_insn_count for each YARV instruction if --zjit-stats is enabled. + if get_option!(stats) { + fun.push_insn(block, Insn::IncrCounter(Counter::zjit_insn_count)); + } // Move to the next instruction to compile insn_idx += insn_len(opcode as usize); @@ -8313,6 +8427,96 @@ mod opt_tests { "); } + #[test] + fn test_optimize_getivar_embedded() { + eval(" + class C + attr_reader :foo + def initialize + @foo = 42 + end + end + + O = C.new + def test(o) = o.foo + test O + test O + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:10: + bb0(v0:BasicObject, v1:BasicObject): + PatchPoint MethodRedefined(C@0x1000, foo@0x1008, cme:0x1010) + v9:HeapObject[class_exact:C] = GuardType v1, HeapObject[class_exact:C] + v12:HeapObject[class_exact:C] = GuardShape v9, 0x1038 + v13:BasicObject = LoadIvarEmbedded v12, :@foo@0x1039 + CheckInterrupts + Return v13 + "); + } + + #[test] + fn test_optimize_getivar_extended() { + eval(" + class C + attr_reader :foo + def initialize + @foo = 42 + end + end + + O = C.new + def test(o) = o.foo + test O + test O + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:10: + bb0(v0:BasicObject, v1:BasicObject): + PatchPoint MethodRedefined(C@0x1000, foo@0x1008, cme:0x1010) + v9:HeapObject[class_exact:C] = GuardType v1, HeapObject[class_exact:C] + v12:HeapObject[class_exact:C] = GuardShape v9, 0x1038 + v13:BasicObject = LoadIvarEmbedded v12, :@foo@0x1039 + CheckInterrupts + Return v13 + "); + } + + #[test] + fn test_dont_optimize_getivar_polymorphic() { + crate::options::rb_zjit_prepare_options(); + crate::options::internal_set_num_profiles(3); + eval(" + class C + attr_reader :foo, :bar + + def foo_then_bar + @foo = 1 + @bar = 2 + end + + def bar_then_foo + @bar = 3 + @foo = 4 + end + end + + O1 = C.new + O1.foo_then_bar + O2 = C.new + O2.bar_then_foo + def test(o) = o.foo + test O1 + test O2 + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:20: + bb0(v0:BasicObject, v1:BasicObject): + v4:BasicObject = SendWithoutBlock v1, :foo + CheckInterrupts + Return v4 + "); + } + #[test] fn test_inline_attr_reader_constant() { eval(" @@ -8332,9 +8536,11 @@ mod opt_tests { PatchPoint StableConstantNames(0x1000, O) v11:BasicObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) PatchPoint MethodRedefined(C@0x1010, foo@0x1018, cme:0x1020) - v13:BasicObject = GetIvar v11, :@foo + v14:HeapObject[VALUE(0x1008)] = GuardType v11, HeapObject + v15:HeapObject[VALUE(0x1008)] = GuardShape v14, 0x1048 + v16:NilClass = Const Value(nil) CheckInterrupts - Return v13 + Return v16 "); } @@ -8357,9 +8563,11 @@ mod opt_tests { PatchPoint StableConstantNames(0x1000, O) v11:BasicObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) PatchPoint MethodRedefined(C@0x1010, foo@0x1018, cme:0x1020) - v13:BasicObject = GetIvar v11, :@foo + v14:HeapObject[VALUE(0x1008)] = GuardType v11, HeapObject + v15:HeapObject[VALUE(0x1008)] = GuardShape v14, 0x1048 + v16:NilClass = Const Value(nil) CheckInterrupts - Return v13 + Return v16 "); } @@ -8379,9 +8587,10 @@ mod opt_tests { bb0(v0:BasicObject, v1:BasicObject): PatchPoint MethodRedefined(C@0x1000, foo@0x1008, cme:0x1010) v9:HeapObject[class_exact:C] = GuardType v1, HeapObject[class_exact:C] - v10:BasicObject = GetIvar v9, :@foo + v12:HeapObject[class_exact:C] = GuardShape v9, 0x1038 + v13:NilClass = Const Value(nil) CheckInterrupts - Return v10 + Return v13 "); } @@ -8401,9 +8610,10 @@ mod opt_tests { bb0(v0:BasicObject, v1:BasicObject): PatchPoint MethodRedefined(C@0x1000, foo@0x1008, cme:0x1010) v9:HeapObject[class_exact:C] = GuardType v1, HeapObject[class_exact:C] - v10:BasicObject = GetIvar v9, :@foo + v12:HeapObject[class_exact:C] = GuardShape v9, 0x1038 + v13:NilClass = Const Value(nil) CheckInterrupts - Return v10 + Return v13 "); } } diff --git a/zjit/src/options.rs b/zjit/src/options.rs index c2c9eea085f86e..155c3805bad0fd 100644 --- a/zjit/src/options.rs +++ b/zjit/src/options.rs @@ -279,6 +279,14 @@ fn update_profile_threshold() { } } +pub fn internal_set_num_profiles(n: u8) { + let options = unsafe { OPTIONS.as_mut().unwrap() }; + options.num_profiles = n; + let call_threshold = n.saturating_add(1); + unsafe { rb_zjit_call_threshold = call_threshold as u64; } + update_profile_threshold(); +} + /// Print YJIT options for `ruby --help`. `width` is width of option parts, and /// `columns` is indent width of descriptions. #[unsafe(no_mangle)] diff --git a/zjit/src/profile.rs b/zjit/src/profile.rs index 9da48a0705be64..f2b12516c72747 100644 --- a/zjit/src/profile.rs +++ b/zjit/src/profile.rs @@ -37,6 +37,10 @@ impl Profiler { *(sp.offset(-1 - n)) } } + + fn peek_at_self(&self) -> VALUE { + unsafe { rb_get_cfp_self(self.cfp) } + } } /// API called from zjit_* instruction. opcode is the bare (non-zjit_*) instruction. @@ -68,6 +72,7 @@ fn profile_insn(bare_opcode: ruby_vminsn_type, ec: EcPtr) { YARVINSN_opt_or => profile_operands(profiler, profile, 2), YARVINSN_opt_empty_p => profile_operands(profiler, profile, 1), YARVINSN_opt_not => profile_operands(profiler, profile, 1), + YARVINSN_getinstancevariable => profile_self(profiler, profile), YARVINSN_opt_send_without_block => { let cd: *const rb_call_data = profiler.insn_opnd(0).as_ptr(); let argc = unsafe { vm_ci_argc((*cd).ci) }; @@ -106,8 +111,21 @@ fn profile_operands(profiler: &mut Profiler, profile: &mut IseqProfile, n: usize } } +fn profile_self(profiler: &mut Profiler, profile: &mut IseqProfile) { + let types = &mut profile.opnd_types[profiler.insn_idx]; + if types.is_empty() { + types.resize(1, TypeDistribution::new()); + } + let obj = profiler.peek_at_self(); + // TODO(max): Handle GC-hidden classes like Array, Hash, etc and make them look normal or + // drop them or something + let ty = ProfiledType::new(obj); + unsafe { rb_gc_writebarrier(profiler.iseq.into(), ty.class()) }; + types[0].observe(ty); +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] -struct Flags(u32); +pub struct Flags(u32); impl Flags { const NONE: u32 = 0; @@ -206,6 +224,10 @@ impl ProfiledType { self.shape } + pub fn flags(&self) -> Flags { + self.flags + } + pub fn is_fixnum(&self) -> bool { self.class == unsafe { rb_cInteger } && self.flags.is_immediate() } diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 64bef8ca6a7932..cde2d4e80ba412 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -94,6 +94,7 @@ make_counters! { exit_fixnum_mult_overflow, exit_guard_type_failure, exit_guard_bit_equals_failure, + exit_guard_shape_failure, exit_patchpoint, exit_callee_side_exit, exit_obj_to_string_fallback, @@ -151,6 +152,7 @@ pub fn exit_counter_ptr(reason: crate::hir::SideExitReason) -> *mut u64 { FixnumMultOverflow => exit_fixnum_mult_overflow, GuardType(_) => exit_guard_type_failure, GuardBitEquals(_) => exit_guard_bit_equals_failure, + GuardShape(_) => exit_guard_shape_failure, PatchPoint(_) => exit_patchpoint, CalleeSideExit => exit_callee_side_exit, ObjToStringFallback => exit_obj_to_string_fallback, From dba0f72592e4bf9aedd61f8d2fe27a5b0a5fcb76 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Fri, 29 Aug 2025 09:57:26 -0700 Subject: [PATCH 5/8] Extend timeout-minutes for macOS --repeat-count=2 https://github.com/ruby/ruby/actions/runs/17308244022/job/49136485007 I'm not sure if it's stuck forever at the end or happens to take that much time around the end of it, but let me just try this first. If it doesn't work, something's wrong with --repeat-count=2 on test-all. --- .github/workflows/macos.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index aab238ea47f523..2a8f8df40fd92b 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -141,7 +141,7 @@ jobs: ulimit -c unlimited make -s ${{ matrix.test_task }} ${TESTS:+TESTS="$TESTS"} - timeout-minutes: 60 + timeout-minutes: 90 env: RUBY_TESTOPTS: '-q --tty=no' TEST_BUNDLED_GEMS_ALLOW_FAILURES: '' From 710c5c8ea16bc21018a778773d3b250e27cf34f8 Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Fri, 29 Aug 2025 07:51:21 -0400 Subject: [PATCH 6/8] ZJIT: Increment dynamic_send_count for Send too --- zjit/src/codegen.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 441bb9e2f7f1a8..17d8b817bd5bc3 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -926,6 +926,8 @@ fn gen_send( blockiseq: IseqPtr, state: &FrameState, ) -> lir::Opnd { + gen_incr_counter(asm, Counter::dynamic_send_count); + // Save PC and SP gen_save_pc(asm, state); gen_save_sp(asm, state.stack().len()); From 99bf47ab8c3fc4332b7324310615af147a4da560 Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Fri, 29 Aug 2025 08:00:29 -0400 Subject: [PATCH 7/8] ZJIT: Report stats for unhandled call types --- zjit.rb | 1 + zjit/src/backend/lir.rs | 7 +- zjit/src/hir.rs | 150 ++++++++++++++++++++-------------------- zjit/src/stats.rs | 37 +++++++++- 4 files changed, 117 insertions(+), 78 deletions(-) diff --git a/zjit.rb b/zjit.rb index dd2f46d692b842..bcd1b3eb492229 100644 --- a/zjit.rb +++ b/zjit.rb @@ -35,6 +35,7 @@ def stats_string stats = self.stats print_counters_with_prefix(prefix: 'failed_', prompt: 'compilation failure reasons', buf:, stats:) + print_counters_with_prefix(prefix: 'unhandled_call_', prompt: 'unhandled call types', buf:, stats:, limit: 20) print_counters([ :dynamic_send_count, diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index 192c4092bfb3a1..538faa41c75822 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -6,7 +6,7 @@ use crate::cruby::{Qundef, RUBY_OFFSET_CFP_PC, RUBY_OFFSET_CFP_SP, SIZEOF_VALUE_ use crate::hir::SideExitReason; use crate::options::{debug, get_option}; use crate::cruby::VALUE; -use crate::stats::{exit_counter_ptr, exit_counter_ptr_for_opcode}; +use crate::stats::{exit_counter_ptr, exit_counter_ptr_for_opcode, exit_counter_ptr_for_call_type}; use crate::virtualmem::CodePtr; use crate::asm::{CodeBlock, Label}; @@ -1604,6 +1604,11 @@ impl Assembler self.load_into(SCRATCH_OPND, Opnd::const_ptr(exit_counter_ptr_for_opcode(opcode))); self.incr_counter_with_reg(Opnd::mem(64, SCRATCH_OPND, 0), 1.into(), C_RET_OPND); } + if let SideExitReason::UnhandledCallType(call_type) = reason { + asm_comment!(self, "increment an unknown call type counter"); + self.load_into(SCRATCH_OPND, Opnd::const_ptr(exit_counter_ptr_for_call_type(call_type))); + self.incr_counter_with_reg(Opnd::mem(64, SCRATCH_OPND, 0), 1.into(), C_RET_OPND); + } } asm_comment!(self, "exit to the interpreter"); diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index f4e14eccd167d0..c55b5bbbf2015e 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -439,7 +439,7 @@ impl PtrPrintMap { #[derive(Debug, Clone, Copy)] pub enum SideExitReason { UnknownNewarraySend(vm_opt_newarray_send_type), - UnknownCallType, + UnhandledCallType(CallType), UnknownSpecialVariable(u64), UnhandledHIRInsn(InsnId), UnhandledYARVInsn(u32), @@ -2808,7 +2808,7 @@ fn compute_bytecode_info(iseq: *const rb_iseq_t) -> BytecodeInfo { BytecodeInfo { jump_targets: result, has_send } } -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Clone, Copy)] pub enum CallType { Splat, BlockArg, @@ -2846,19 +2846,19 @@ fn num_locals(iseq: *const rb_iseq_t) -> usize { } /// If we can't handle the type of send (yet), bail out. -fn unknown_call_type(flag: u32) -> bool { - if (flag & VM_CALL_KW_SPLAT_MUT) != 0 { return true; } - if (flag & VM_CALL_ARGS_SPLAT_MUT) != 0 { return true; } - if (flag & VM_CALL_ARGS_SPLAT) != 0 { return true; } - if (flag & VM_CALL_KW_SPLAT) != 0 { return true; } - if (flag & VM_CALL_ARGS_BLOCKARG) != 0 { return true; } - if (flag & VM_CALL_KWARG) != 0 { return true; } - if (flag & VM_CALL_TAILCALL) != 0 { return true; } - if (flag & VM_CALL_SUPER) != 0 { return true; } - if (flag & VM_CALL_ZSUPER) != 0 { return true; } - if (flag & VM_CALL_OPT_SEND) != 0 { return true; } - if (flag & VM_CALL_FORWARDING) != 0 { return true; } - false +fn unknown_call_type(flag: u32) -> Result<(), CallType> { + if (flag & VM_CALL_KW_SPLAT_MUT) != 0 { return Err(CallType::KwSplatMut); } + if (flag & VM_CALL_ARGS_SPLAT_MUT) != 0 { return Err(CallType::SplatMut); } + if (flag & VM_CALL_ARGS_SPLAT) != 0 { return Err(CallType::Splat); } + if (flag & VM_CALL_KW_SPLAT) != 0 { return Err(CallType::KwSplat); } + if (flag & VM_CALL_ARGS_BLOCKARG) != 0 { return Err(CallType::BlockArg); } + if (flag & VM_CALL_KWARG) != 0 { return Err(CallType::Kwarg); } + if (flag & VM_CALL_TAILCALL) != 0 { return Err(CallType::Tailcall); } + if (flag & VM_CALL_SUPER) != 0 { return Err(CallType::Super); } + if (flag & VM_CALL_ZSUPER) != 0 { return Err(CallType::Zsuper); } + if (flag & VM_CALL_OPT_SEND) != 0 { return Err(CallType::OptSend); } + if (flag & VM_CALL_FORWARDING) != 0 { return Err(CallType::Forwarding); } + Ok(()) } /// We have IseqPayload, which keeps track of HIR Types in the interpreter, but this is not useful @@ -3324,10 +3324,10 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { // NB: opt_neq has two cd; get_arg(0) is for eq and get_arg(1) is for neq let cd: *const rb_call_data = get_arg(pc, 1).as_ptr(); let call_info = unsafe { rb_get_call_data_ci(cd) }; - if unknown_call_type(unsafe { rb_vm_ci_flag(call_info) }) { + if let Err(call_type) = unknown_call_type(unsafe { rb_vm_ci_flag(call_info) }) { // Unknown call type; side-exit into the interpreter let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - fun.push_insn(block, Insn::SideExit { state: exit_id, reason: SideExitReason::UnknownCallType }); + fun.push_insn(block, Insn::SideExit { state: exit_id, reason: SideExitReason::UnhandledCallType(call_type) }); break; // End the block } let argc = unsafe { vm_ci_argc((*cd).ci) }; @@ -3345,10 +3345,10 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { // NB: these instructions have the recv for the call at get_arg(0) let cd: *const rb_call_data = get_arg(pc, 1).as_ptr(); let call_info = unsafe { rb_get_call_data_ci(cd) }; - if unknown_call_type(unsafe { rb_vm_ci_flag(call_info) }) { + if let Err(call_type) = unknown_call_type(unsafe { rb_vm_ci_flag(call_info) }) { // Unknown call type; side-exit into the interpreter let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - fun.push_insn(block, Insn::SideExit { state: exit_id, reason: SideExitReason::UnknownCallType }); + fun.push_insn(block, Insn::SideExit { state: exit_id, reason: SideExitReason::UnhandledCallType(call_type) }); break; // End the block } let argc = unsafe { vm_ci_argc((*cd).ci) }; @@ -3403,10 +3403,10 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { YARVINSN_opt_send_without_block => { let cd: *const rb_call_data = get_arg(pc, 0).as_ptr(); let call_info = unsafe { rb_get_call_data_ci(cd) }; - if unknown_call_type(unsafe { rb_vm_ci_flag(call_info) }) { + if let Err(call_type) = unknown_call_type(unsafe { rb_vm_ci_flag(call_info) }) { // Unknown call type; side-exit into the interpreter let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - fun.push_insn(block, Insn::SideExit { state: exit_id, reason: SideExitReason::UnknownCallType }); + fun.push_insn(block, Insn::SideExit { state: exit_id, reason: SideExitReason::UnhandledCallType(call_type) }); break; // End the block } let argc = unsafe { vm_ci_argc((*cd).ci) }; @@ -3421,10 +3421,10 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let cd: *const rb_call_data = get_arg(pc, 0).as_ptr(); let blockiseq: IseqPtr = get_arg(pc, 1).as_iseq(); let call_info = unsafe { rb_get_call_data_ci(cd) }; - if unknown_call_type(unsafe { rb_vm_ci_flag(call_info) }) { + if let Err(call_type) = unknown_call_type(unsafe { rb_vm_ci_flag(call_info) }) { // Unknown call type; side-exit into the interpreter let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - fun.push_insn(block, Insn::SideExit { state: exit_id, reason: SideExitReason::UnknownCallType }); + fun.push_insn(block, Insn::SideExit { state: exit_id, reason: SideExitReason::UnhandledCallType(call_type) }); break; // End the block } let argc = unsafe { vm_ci_argc((*cd).ci) }; @@ -3549,8 +3549,8 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let cd: *const rb_call_data = get_arg(pc, 0).as_ptr(); let call_info = unsafe { rb_get_call_data_ci(cd) }; - if unknown_call_type(unsafe { rb_vm_ci_flag(call_info) }) { - assert!(false, "objtostring should not have unknown call type"); + if let Err(call_type) = unknown_call_type(unsafe { rb_vm_ci_flag(call_info) }) { + assert!(false, "objtostring should not have unknown call type {call_type:?}"); } let argc = unsafe { vm_ci_argc((*cd).ci) }; assert_eq!(0, argc, "objtostring should not have args"); @@ -4872,12 +4872,12 @@ mod tests { eval(" def test(a) = foo(*a) "); - assert_snapshot!(hir_string("test"), @r#" - fn test@:2: - bb0(v0:BasicObject, v1:BasicObject): - v4:ArrayExact = ToArray v1 - SideExit UnknownCallType - "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:2: + bb0(v0:BasicObject, v1:BasicObject): + v4:ArrayExact = ToArray v1 + SideExit UnhandledCallType(Splat) + "); } #[test] @@ -4889,7 +4889,7 @@ mod tests { fn test@:2: bb0(v0:BasicObject, v1:BasicObject): v3:BasicObject = GetLocal l0, EP@3 - SideExit UnknownCallType + SideExit UnhandledCallType(BlockArg) "); } @@ -4898,12 +4898,12 @@ mod tests { eval(" def test(a) = foo(a: 1) "); - assert_snapshot!(hir_string("test"), @r#" - fn test@:2: - bb0(v0:BasicObject, v1:BasicObject): - v3:Fixnum[1] = Const Value(1) - SideExit UnknownCallType - "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:2: + bb0(v0:BasicObject, v1:BasicObject): + v3:Fixnum[1] = Const Value(1) + SideExit UnhandledCallType(Kwarg) + "); } #[test] @@ -4911,11 +4911,11 @@ mod tests { eval(" def test(a) = foo(**a) "); - assert_snapshot!(hir_string("test"), @r#" - fn test@:2: - bb0(v0:BasicObject, v1:BasicObject): - SideExit UnknownCallType - "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:2: + bb0(v0:BasicObject, v1:BasicObject): + SideExit UnhandledCallType(KwSplat) + "); } // TODO(max): Figure out how to generate a call with TAILCALL flag @@ -4965,18 +4965,18 @@ mod tests { eval(" def test(a) = foo **a, b: 1 "); - assert_snapshot!(hir_string("test"), @r#" - fn test@:2: - bb0(v0:BasicObject, v1:BasicObject): - v3:Class[VMFrozenCore] = Const Value(VALUE(0x1000)) - v5:HashExact = NewHash - v7:BasicObject = SendWithoutBlock v3, :core#hash_merge_kwd, v5, v1 - v8:Class[VMFrozenCore] = Const Value(VALUE(0x1000)) - v9:StaticSymbol[:b] = Const Value(VALUE(0x1008)) - v10:Fixnum[1] = Const Value(1) - v12:BasicObject = SendWithoutBlock v8, :core#hash_merge_ptr, v7, v9, v10 - SideExit UnknownCallType - "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:2: + bb0(v0:BasicObject, v1:BasicObject): + v3:Class[VMFrozenCore] = Const Value(VALUE(0x1000)) + v5:HashExact = NewHash + v7:BasicObject = SendWithoutBlock v3, :core#hash_merge_kwd, v5, v1 + v8:Class[VMFrozenCore] = Const Value(VALUE(0x1000)) + v9:StaticSymbol[:b] = Const Value(VALUE(0x1008)) + v10:Fixnum[1] = Const Value(1) + v12:BasicObject = SendWithoutBlock v8, :core#hash_merge_ptr, v7, v9, v10 + SideExit UnhandledCallType(KwSplatMut) + "); } #[test] @@ -4984,14 +4984,14 @@ mod tests { eval(" def test(*) = foo *, 1 "); - assert_snapshot!(hir_string("test"), @r#" - fn test@:2: - bb0(v0:BasicObject, v1:ArrayExact): - v4:ArrayExact = ToNewArray v1 - v5:Fixnum[1] = Const Value(1) - ArrayPush v4, v5 - SideExit UnknownCallType - "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:2: + bb0(v0:BasicObject, v1:ArrayExact): + v4:ArrayExact = ToNewArray v1 + v5:Fixnum[1] = Const Value(1) + ArrayPush v4, v5 + SideExit UnhandledCallType(SplatMut) + "); } #[test] @@ -7398,12 +7398,12 @@ mod opt_tests { test test "); - assert_snapshot!(hir_string("test"), @r#" - fn test@:3: - bb0(v0:BasicObject): - v2:Fixnum[1] = Const Value(1) - SideExit UnknownCallType - "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(v0:BasicObject): + v2:Fixnum[1] = Const Value(1) + SideExit UnhandledCallType(Kwarg) + "); } #[test] @@ -7414,12 +7414,12 @@ mod opt_tests { test test "); - assert_snapshot!(hir_string("test"), @r#" - fn test@:3: - bb0(v0:BasicObject): - v2:Fixnum[1] = Const Value(1) - SideExit UnknownCallType - "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(v0:BasicObject): + v2:Fixnum[1] = Const Value(1) + SideExit UnhandledCallType(Kwarg) + "); } #[test] diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index cde2d4e80ba412..ea979b534a32ca 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -85,7 +85,7 @@ make_counters! { // exit_: Side exits reasons exit_compilation_failure, exit_unknown_newarray_send, - exit_unknown_call_type, + exit_unhandled_call_type, exit_unknown_special_variable, exit_unhandled_hir_insn, exit_unhandled_yarv_insn, @@ -101,6 +101,19 @@ make_counters! { exit_interrupt, } + // unhanded_call_: Unhandled call types + unhandled_call_splat, + unhandled_call_block_arg, + unhandled_call_kwarg, + unhandled_call_kw_splat, + unhandled_call_tailcall, + unhandled_call_super, + unhandled_call_zsuper, + unhandled_call_optsend, + unhandled_call_kw_splat_mut, + unhandled_call_splat_mut, + unhandled_call_forwarding, + // failed_: Compilation failure reasons failed_iseq_stack_too_large, failed_hir_compile, @@ -138,12 +151,32 @@ pub fn exit_counter_ptr_for_opcode(opcode: u32) -> *mut u64 { unsafe { exit_counters.get_unchecked_mut(opcode as usize) } } +/// Return a raw pointer to the exit counter for a given call type +pub fn exit_counter_ptr_for_call_type(call_type: crate::hir::CallType) -> *mut u64 { + use crate::hir::CallType::*; + use crate::stats::Counter::*; + let counter = match call_type { + Splat => unhandled_call_splat, + BlockArg => unhandled_call_block_arg, + Kwarg => unhandled_call_kwarg, + KwSplat => unhandled_call_kw_splat, + Tailcall => unhandled_call_tailcall, + Super => unhandled_call_super, + Zsuper => unhandled_call_zsuper, + OptSend => unhandled_call_optsend, + KwSplatMut => unhandled_call_kw_splat_mut, + SplatMut => unhandled_call_splat_mut, + Forwarding => unhandled_call_forwarding, + }; + counter_ptr(counter) +} + pub fn exit_counter_ptr(reason: crate::hir::SideExitReason) -> *mut u64 { use crate::hir::SideExitReason::*; use crate::stats::Counter::*; let counter = match reason { UnknownNewarraySend(_) => exit_unknown_newarray_send, - UnknownCallType => exit_unknown_call_type, + UnhandledCallType(_) => exit_unhandled_call_type, UnknownSpecialVariable(_) => exit_unknown_special_variable, UnhandledHIRInsn(_) => exit_unhandled_hir_insn, UnhandledYARVInsn(_) => exit_unhandled_yarv_insn, From 39f3cab80d7b469793a1a3cf091ed875c54c3c86 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Fri, 29 Aug 2025 11:43:33 -0700 Subject: [PATCH 8/8] YJIT: Stop sharing rb_vm_send among different instructions (#14393) --- vm_insnhelper.c | 27 ++++++++++++++------------- yjit/src/codegen.rs | 19 ++++++++++++++++++- 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 0fb4d086b7fc56..eee20cd9d4e3fd 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -6093,27 +6093,28 @@ vm_sendish( VALUE rb_vm_send(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, CALL_DATA cd, ISEQ blockiseq) +{ + stack_check(ec); + VALUE bh = vm_caller_setup_arg_block(ec, GET_CFP(), cd->ci, blockiseq, false); + VALUE val = vm_sendish(ec, GET_CFP(), cd, bh, mexp_search_method); + VM_EXEC(ec, val); + return val; +} + +VALUE +rb_vm_sendforward(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, CALL_DATA cd, ISEQ blockiseq) { stack_check(ec); struct rb_forwarding_call_data adjusted_cd; struct rb_callinfo adjusted_ci; - VALUE bh; - VALUE val; - - if (vm_ci_flag(cd->ci) & VM_CALL_FORWARDING) { - bh = vm_caller_setup_fwd_args(GET_EC(), GET_CFP(), cd, blockiseq, false, &adjusted_cd, &adjusted_ci); + VALUE bh = vm_caller_setup_fwd_args(GET_EC(), GET_CFP(), cd, blockiseq, false, &adjusted_cd, &adjusted_ci); - val = vm_sendish(ec, GET_CFP(), &adjusted_cd.cd, bh, mexp_search_method); + VALUE val = vm_sendish(ec, GET_CFP(), &adjusted_cd.cd, bh, mexp_search_method); - if (cd->cc != adjusted_cd.cd.cc && vm_cc_markable(adjusted_cd.cd.cc)) { - RB_OBJ_WRITE(GET_ISEQ(), &cd->cc, adjusted_cd.cd.cc); - } - } - else { - bh = vm_caller_setup_arg_block(ec, GET_CFP(), cd->ci, blockiseq, false); - val = vm_sendish(ec, GET_CFP(), cd, bh, mexp_search_method); + if (cd->cc != adjusted_cd.cd.cc && vm_cc_markable(adjusted_cd.cd.cc)) { + RB_OBJ_WRITE(GET_ISEQ(), &cd->cc, adjusted_cd.cd.cc); } VM_EXEC(ec, val); diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 9e9759e7812002..8b472efdfdaa60 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -9564,7 +9564,24 @@ fn gen_sendforward( jit: &mut JITState, asm: &mut Assembler, ) -> Option { - return gen_send(jit, asm); + // Generate specialized code if possible + let cd = jit.get_arg(0).as_ptr(); + let block = jit.get_arg(1).as_optional_ptr().map(|iseq| BlockHandler::BlockISeq(iseq)); + if let Some(status) = perf_call! { gen_send_general(jit, asm, cd, block) } { + return Some(status); + } + + // Otherwise, fallback to dynamic dispatch using the interpreter's implementation of sendforward + let blockiseq = jit.get_arg(1).as_iseq(); + gen_send_dynamic(jit, asm, cd, unsafe { rb_yjit_sendish_sp_pops((*cd).ci) }, |asm| { + extern "C" { + fn rb_vm_sendforward(ec: EcPtr, cfp: CfpPtr, cd: VALUE, blockiseq: IseqPtr) -> VALUE; + } + asm.ccall( + rb_vm_sendforward as *const u8, + vec![EC, CFP, (cd as usize).into(), VALUE(blockiseq as usize).into()], + ) + }) } fn gen_invokeblock(