From 5b956fbf60ee36ba1e7b56e7747f31c0e3586c46 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Mon, 11 Aug 2025 14:31:16 -0400 Subject: [PATCH 01/11] ZJIT: Fix `mismatched_lifetime_syntaxes`, new in Rust 1.89.0 --- zjit/src/backend/lir.rs | 4 ++-- zjit/src/hir.rs | 12 ++++++------ zjit/src/hir_type/mod.rs | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index 3263392cf6d371..0902d347c7a29c 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -549,13 +549,13 @@ pub enum Insn { impl Insn { /// Create an iterator that will yield a non-mutable reference to each /// operand in turn for this instruction. - pub(super) fn opnd_iter(&self) -> InsnOpndIterator { + pub(super) fn opnd_iter(&self) -> InsnOpndIterator<'_> { InsnOpndIterator::new(self) } /// Create an iterator that will yield a mutable reference to each operand /// in turn for this instruction. - pub(super) fn opnd_iter_mut(&mut self) -> InsnOpndMutIterator { + pub(super) fn opnd_iter_mut(&mut self) -> InsnOpndMutIterator<'_> { InsnOpndMutIterator::new(self) } diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 87d2a613d0e930..bff0fcd7574e7c 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -67,7 +67,7 @@ impl std::fmt::Display for VALUE { } impl VALUE { - pub fn print(self, ptr_map: &PtrPrintMap) -> VALUEPrinter { + pub fn print(self, ptr_map: &PtrPrintMap) -> VALUEPrinter<'_> { VALUEPrinter { inner: self, ptr_map } } } @@ -136,7 +136,7 @@ pub enum Invariant { } impl Invariant { - pub fn print(self, ptr_map: &PtrPrintMap) -> InvariantPrinter { + pub fn print(self, ptr_map: &PtrPrintMap) -> InvariantPrinter<'_> { InvariantPrinter { inner: self, ptr_map } } } @@ -810,12 +810,12 @@ pub struct Block { impl Block { /// Return an iterator over params - pub fn params(&self) -> Iter { + pub fn params(&self) -> Iter<'_, InsnId> { self.params.iter() } /// Return an iterator over insns - pub fn insns(&self) -> Iter { + pub fn insns(&self) -> Iter<'_, InsnId> { self.insns.iter() } } @@ -2450,12 +2450,12 @@ impl FrameState { } /// Iterate over all stack slots - pub fn stack(&self) -> Iter { + pub fn stack(&self) -> Iter<'_, InsnId> { self.stack.iter() } /// Iterate over all local variables - pub fn locals(&self) -> Iter { + pub fn locals(&self) -> Iter<'_, InsnId> { self.locals.iter() } diff --git a/zjit/src/hir_type/mod.rs b/zjit/src/hir_type/mod.rs index 607ccbde84bfae..c18b2735bee834 100644 --- a/zjit/src/hir_type/mod.rs +++ b/zjit/src/hir_type/mod.rs @@ -501,7 +501,7 @@ impl Type { self.is_subtype(types::Immediate) } - pub fn print(self, ptr_map: &PtrPrintMap) -> TypePrinter { + pub fn print(self, ptr_map: &PtrPrintMap) -> TypePrinter<'_> { TypePrinter { inner: self, ptr_map } } } From 6e3790b17f1b58d67616ef1f9b899bc4af91d334 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Mon, 11 Aug 2025 14:31:52 -0400 Subject: [PATCH 02/11] YJIT: Fix `mismatched_lifetime_syntaxes`, new in Rust 1.89.0 --- yjit/src/backend/ir.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/yjit/src/backend/ir.rs b/yjit/src/backend/ir.rs index 1df151433a8025..b704a249853ceb 100644 --- a/yjit/src/backend/ir.rs +++ b/yjit/src/backend/ir.rs @@ -528,13 +528,13 @@ pub enum Insn { impl Insn { /// Create an iterator that will yield a non-mutable reference to each /// operand in turn for this instruction. - pub(super) fn opnd_iter(&self) -> InsnOpndIterator { + pub(super) fn opnd_iter(&self) -> InsnOpndIterator<'_> { InsnOpndIterator::new(self) } /// Create an iterator that will yield a mutable reference to each operand /// in turn for this instruction. - pub(super) fn opnd_iter_mut(&mut self) -> InsnOpndMutIterator { + pub(super) fn opnd_iter_mut(&mut self) -> InsnOpndMutIterator<'_> { InsnOpndMutIterator::new(self) } From 6968668570fd43065cf4b9b4a1063a6b3fe888aa Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Mon, 11 Aug 2025 13:18:52 -0700 Subject: [PATCH 03/11] ZJIT: Add RubyVM::ZJIT.enabled? (#14159) Co-authored-by: Max Bernstein --- test/ruby/test_zjit.rb | 34 ++++++++++++++++++++++++++-------- zjit.rb | 9 +++++++-- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index c86ac62a9f92ca..bf43fd1324f307 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -9,6 +9,15 @@ return unless JITSupport.zjit_supported? class TestZJIT < Test::Unit::TestCase + def test_enabled + assert_runs 'false', <<~RUBY, zjit: false + RubyVM::ZJIT.enabled? + RUBY + assert_runs 'true', <<~RUBY, zjit: true + RubyVM::ZJIT.enabled? + RUBY + end + def test_call_itself assert_compiles '42', <<~RUBY, call_threshold: 2 def test = 42.itself @@ -1547,14 +1556,23 @@ def assert_runs(expected, test_script, insns: [], assert_compiles: false, **opts end # Run a Ruby process with ZJIT options and a pipe for writing test results - def eval_with_jit(script, call_threshold: 1, num_profiles: 1, stats: false, debug: true, timeout: 1000, pipe_fd:) - args = [ - "--disable-gems", - "--zjit-call-threshold=#{call_threshold}", - "--zjit-num-profiles=#{num_profiles}", - ] - args << "--zjit-stats" if stats - args << "--zjit-debug" if debug + def eval_with_jit( + script, + call_threshold: 1, + num_profiles: 1, + zjit: true, + stats: false, + debug: true, + timeout: 1000, + pipe_fd: + ) + args = ["--disable-gems"] + if zjit + args << "--zjit-call-threshold=#{call_threshold}" + args << "--zjit-num-profiles=#{num_profiles}" + args << "--zjit-stats" if stats + args << "--zjit-debug" if debug + end args << "-e" << script_shell_encode(script) pipe_r, pipe_w = IO.pipe # Separate thread so we don't deadlock when diff --git a/zjit.rb b/zjit.rb index 2bc779ef2895ec..a307abdf88821e 100644 --- a/zjit.rb +++ b/zjit.rb @@ -14,7 +14,12 @@ module RubyVM::ZJIT end class << RubyVM::ZJIT - # Return ZJIT statistics as a Hash + # Check if \ZJIT is enabled + def enabled? + Primitive.cexpr! 'RBOOL(rb_zjit_enabled_p)' + end + + # Return \ZJIT statistics as a Hash def stats stats = Primitive.rb_zjit_stats @@ -26,7 +31,7 @@ def stats stats end - # Get the summary of ZJIT statistics as a String + # Get the summary of \ZJIT statistics as a String def stats_string buf = +'' stats = self.stats From 319550527ff8fbff6ee586fb75da2234de5d2feb Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Mon, 11 Aug 2025 13:21:45 -0700 Subject: [PATCH 04/11] ZJIT: Add compile/profile/GC/invalidation time stats (#14158) Co-authored-by: Stan Lo --- zjit.rb | 24 ++++++++++++- zjit/src/codegen.rs | 7 ++-- zjit/src/gc.rs | 46 ++++++++++++++----------- zjit/src/invariants.rs | 24 +++++++------ zjit/src/profile.rs | 8 +++-- zjit/src/stats.rs | 77 +++++++++++++++++++++++++++++++++++------- 6 files changed, 138 insertions(+), 48 deletions(-) diff --git a/zjit.rb b/zjit.rb index a307abdf88821e..fc2c80c52f2939 100644 --- a/zjit.rb +++ b/zjit.rb @@ -22,6 +22,7 @@ def enabled? # Return \ZJIT statistics as a Hash def stats stats = Primitive.rb_zjit_stats + return nil if stats.nil? if stats.key?(:vm_insns_count) && stats.key?(:zjit_insns_count) stats[:total_insns_count] = stats[:vm_insns_count] + stats[:zjit_insns_count] @@ -37,15 +38,29 @@ def stats_string stats = self.stats [ + :compile_time_ns, + :profile_time_ns, + :gc_time_ns, + :invalidation_time_ns, :total_insns_count, :vm_insns_count, :zjit_insns_count, :ratio_in_zjit, ].each do |key| + # Some stats like vm_insns_count and ratio_in_zjit are not supported on the release build + next unless stats.key?(key) value = stats[key] - if key == :ratio_in_zjit + + case key + when :ratio_in_zjit value = '%0.1f%%' % value + when /_time_ns\z/ + key = key.to_s.sub(/_time_ns\z/, '_time') + value = "#{number_with_delimiter(value / 10**6)}ms" + else + value = number_with_delimiter(value) end + buf << "#{'%-18s' % "#{key}:"} #{value}\n" end buf @@ -59,6 +74,13 @@ def assert_compiles # :nodoc: # :stopdoc: private + def number_with_delimiter(number) + s = number.to_s + i = s.index('.') || s.size + s.insert(i -= 3, ',') while i > 3 + s + end + # Print ZJIT stats def print_stats $stderr.write stats_string diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 1d6901bac4a73f..f9532dfe031661 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -7,7 +7,7 @@ use crate::backend::current::{Reg, ALLOC_REGS}; use crate::invariants::{track_bop_assumption, track_cme_assumption, track_single_ractor_assumption, track_stable_constant_names_assumption}; use crate::gc::{append_gc_offsets, get_or_create_iseq_payload, get_or_create_iseq_payload_ptr}; use crate::state::ZJITState; -use crate::stats::{counter_ptr, Counter}; +use crate::stats::{counter_ptr, with_time_stat, Counter, Counter::compile_time_ns}; use crate::{asm::CodeBlock, cruby::*, options::debug, virtualmem::CodePtr}; use crate::backend::lir::{self, asm_comment, asm_ccall, Assembler, Opnd, SideExitContext, Target, CFP, C_ARG_OPNDS, C_RET_OPND, EC, NATIVE_STACK_PTR, NATIVE_BASE_PTR, SP}; use crate::hir::{iseq_to_hir, Block, BlockId, BranchEdge, Invariant, RangeType, SideExitReason, SideExitReason::*, SpecialObjectType, SELF_PARAM_IDX}; @@ -86,7 +86,7 @@ pub extern "C" fn rb_zjit_iseq_gen_entry_point(iseq: IseqPtr, _ec: EcPtr) -> *co // Take a lock to avoid writing to ISEQ in parallel with Ractors. // with_vm_lock() does nothing if the program doesn't use Ractors. let code_ptr = with_vm_lock(src_loc!(), || { - gen_iseq_entry_point(iseq) + with_time_stat(compile_time_ns, || gen_iseq_entry_point(iseq)) }); // Assert that the ISEQ compiles if RubyVM::ZJIT.assert_compiles is enabled @@ -1359,7 +1359,8 @@ c_callable! { with_vm_lock(src_loc!(), || { // Get a pointer to compiled code or the side-exit trampoline let cb = ZJITState::get_code_block(); - let code_ptr = if let Some(code_ptr) = function_stub_hit_body(cb, iseq, branch_ptr) { + let code_ptr = with_time_stat(compile_time_ns, || function_stub_hit_body(cb, iseq, branch_ptr)); + let code_ptr = if let Some(code_ptr) = code_ptr { code_ptr } else { // gen_push_frame() doesn't set PC and SP, so we need to set them for side-exit diff --git a/zjit/src/gc.rs b/zjit/src/gc.rs index d94d86036baed2..ea1b0ed2ea5ca7 100644 --- a/zjit/src/gc.rs +++ b/zjit/src/gc.rs @@ -1,7 +1,8 @@ // This module is responsible for marking/moving objects on GC. use std::{ffi::c_void, ops::Range}; -use crate::{cruby::*, profile::IseqProfile, state::ZJITState, virtualmem::CodePtr}; +use crate::{cruby::*, profile::IseqProfile, state::ZJITState, stats::with_time_stat, virtualmem::CodePtr}; +use crate::stats::Counter::gc_time_ns; /// This is all the data ZJIT stores on an ISEQ. We mark objects in this struct on GC. #[derive(Debug)] @@ -65,6 +66,7 @@ fn payload_ptr_as_mut(payload_ptr: *mut IseqPayload) -> &'static mut IseqPayload unsafe { payload_ptr.as_mut() }.unwrap() } +/// GC callback for marking GC objects in the per-ISEQ payload. #[unsafe(no_mangle)] pub extern "C" fn rb_zjit_iseq_mark(payload: *mut c_void) { let payload = if payload.is_null() { @@ -80,7 +82,29 @@ pub extern "C" fn rb_zjit_iseq_mark(payload: *mut c_void) { &*(payload as *const IseqPayload) } }; + with_time_stat(gc_time_ns, || iseq_mark(payload)); +} + +/// GC callback for updating GC objects in the per-ISEQ payload. +#[unsafe(no_mangle)] +pub extern "C" fn rb_zjit_iseq_update_references(payload: *mut c_void) { + let payload = if payload.is_null() { + return; // nothing to update + } else { + // SAFETY: The GC takes the VM lock while marking, which + // we assert, so we should be synchronized and data race free. + // + // For aliasing, having the VM lock hopefully also implies that no one + // else has an overlapping &mut IseqPayload. + unsafe { + rb_assert_holding_vm_lock(); + &mut *(payload as *mut IseqPayload) + } + }; + with_time_stat(gc_time_ns, || iseq_update_references(payload)); +} +fn iseq_mark(payload: &IseqPayload) { // Mark objects retained by profiling instructions payload.profile.each_object(|object| { unsafe { rb_gc_mark_movable(object); } @@ -100,24 +124,8 @@ pub extern "C" fn rb_zjit_iseq_mark(payload: *mut c_void) { } } -/// GC callback for updating GC objects in the per-ISEQ payload. -/// This is a mirror of [rb_zjit_iseq_mark]. -#[unsafe(no_mangle)] -pub extern "C" fn rb_zjit_iseq_update_references(payload: *mut c_void) { - let payload = if payload.is_null() { - return; // nothing to update - } else { - // SAFETY: The GC takes the VM lock while marking, which - // we assert, so we should be synchronized and data race free. - // - // For aliasing, having the VM lock hopefully also implies that no one - // else has an overlapping &mut IseqPayload. - unsafe { - rb_assert_holding_vm_lock(); - &mut *(payload as *mut IseqPayload) - } - }; - +/// This is a mirror of [iseq_mark]. +fn iseq_update_references(payload: &mut IseqPayload) { // Move objects retained by profiling instructions payload.profile.each_object_mut(|object| { *object = unsafe { rb_gc_location(*object) }; diff --git a/zjit/src/invariants.rs b/zjit/src/invariants.rs index 85bc04fc715253..3f291415be6306 100644 --- a/zjit/src/invariants.rs +++ b/zjit/src/invariants.rs @@ -1,20 +1,24 @@ use std::{collections::{HashMap, HashSet}, mem}; use crate::{backend::lir::{asm_comment, Assembler}, cruby::{rb_callable_method_entry_t, ruby_basic_operators, src_loc, with_vm_lock, IseqPtr, RedefinitionFlag, ID}, gc::IseqPayload, hir::Invariant, options::debug, state::{zjit_enabled_p, ZJITState}, virtualmem::CodePtr}; +use crate::stats::with_time_stat; +use crate::stats::Counter::invalidation_time_ns; use crate::gc::remove_gc_offsets; macro_rules! compile_patch_points { ($cb:expr, $patch_points:expr, $($comment_args:tt)*) => { - for patch_point in $patch_points { - let written_range = $cb.with_write_ptr(patch_point.patch_point_ptr, |cb| { - let mut asm = Assembler::new(); - asm_comment!(asm, $($comment_args)*); - asm.jmp(patch_point.side_exit_ptr.into()); - asm.compile(cb).expect("can write existing code"); - }); - // Stop marking GC offsets corrupted by the jump instruction - remove_gc_offsets(patch_point.payload_ptr, &written_range); - } + with_time_stat(invalidation_time_ns, || { + for patch_point in $patch_points { + let written_range = $cb.with_write_ptr(patch_point.patch_point_ptr, |cb| { + let mut asm = Assembler::new(); + asm_comment!(asm, $($comment_args)*); + asm.jmp(patch_point.side_exit_ptr.into()); + asm.compile(cb).expect("can write existing code"); + }); + // Stop marking GC offsets corrupted by the jump instruction + remove_gc_offsets(patch_point.payload_ptr, &written_range); + } + }); }; } diff --git a/zjit/src/profile.rs b/zjit/src/profile.rs index 12b10b98ee57bc..7ffaea29dcdc81 100644 --- a/zjit/src/profile.rs +++ b/zjit/src/profile.rs @@ -3,6 +3,8 @@ use crate::{cruby::*, gc::get_or_create_iseq_payload, options::get_option}; use crate::distribution::{Distribution, DistributionSummary}; +use crate::stats::Counter::profile_time_ns; +use crate::stats::with_time_stat; /// Ephemeral state for profiling runtime information struct Profiler { @@ -41,13 +43,13 @@ impl Profiler { #[unsafe(no_mangle)] pub extern "C" fn rb_zjit_profile_insn(bare_opcode: u32, ec: EcPtr) { with_vm_lock(src_loc!(), || { - let mut profiler = Profiler::new(ec); - profile_insn(&mut profiler, bare_opcode as ruby_vminsn_type); + with_time_stat(profile_time_ns, || profile_insn(bare_opcode as ruby_vminsn_type, ec)); }); } /// Profile a YARV instruction -fn profile_insn(profiler: &mut Profiler, bare_opcode: ruby_vminsn_type) { +fn profile_insn(bare_opcode: ruby_vminsn_type, ec: EcPtr) { + let profiler = &mut Profiler::new(ec); let profile = &mut get_or_create_iseq_payload(profiler.iseq).profile; match bare_opcode { YARVINSN_opt_nil_p => profile_operands(profiler, profile, 1), diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 4fbad5d24701df..5b39ecdf4bd7f6 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -1,38 +1,75 @@ -// Maxime would like to rebuild an improved stats system -// Individual stats should be tagged as always available, or only available in stats mode -// We could also tag which stats are fallback or exit counters, etc. Maybe even tag units? -// -// Comptime vs Runtime stats? +use std::time::Instant; use crate::{cruby::*, options::get_option, state::{zjit_enabled_p, ZJITState}}; macro_rules! make_counters { - ($($counter_name:ident,)+) => { + ( + default { + $($default_counter_name:ident,)+ + } + $($counter_name:ident,)+ + ) => { /// Struct containing the counter values #[derive(Default, Debug)] - pub struct Counters { $(pub $counter_name: u64),+ } + pub struct Counters { + $(pub $default_counter_name: u64,)+ + $(pub $counter_name: u64,)+ + } /// Enum to represent a counter #[allow(non_camel_case_types)] #[derive(Clone, Copy, PartialEq, Eq, Debug)] - pub enum Counter { $($counter_name),+ } + pub enum Counter { + $($default_counter_name,)+ + $($counter_name,)+ + } + + impl Counter { + pub fn name(&self) -> String { + match self { + $( Counter::$default_counter_name => stringify!($default_counter_name).to_string(), )+ + $( Counter::$counter_name => stringify!($counter_name).to_string(), )+ + } + } + } /// Map a counter to a pointer pub fn counter_ptr(counter: Counter) -> *mut u64 { let counters = $crate::state::ZJITState::get_counters(); match counter { - $( Counter::$counter_name => std::ptr::addr_of_mut!(counters.$counter_name) ),+ + $( Counter::$default_counter_name => std::ptr::addr_of_mut!(counters.$default_counter_name), )+ + $( Counter::$counter_name => std::ptr::addr_of_mut!(counters.$counter_name), )+ } } + + /// The list of counters that are available without --zjit-stats. + /// They are incremented only by `incr_counter()` and don't use `gen_incr_counter()`. + pub const DEFAULT_COUNTERS: &'static [Counter] = &[ + $( Counter::$default_counter_name, )+ + ]; } } // Declare all the counters we track make_counters! { + // Default counters that are available without --zjit-stats + default { + compile_time_ns, + profile_time_ns, + gc_time_ns, + invalidation_time_ns, + } + // The number of times YARV instructions are executed on JIT code zjit_insns_count, } +/// Increase a counter by a specified amount +fn incr_counter(counter: Counter, amount: u64) { + let ptr = counter_ptr(counter); + unsafe { *ptr += amount; } +} + pub fn zjit_alloc_size() -> usize { 0 // TODO: report the actual memory usage } @@ -49,14 +86,30 @@ pub extern "C" fn rb_zjit_stats(_ec: EcPtr, _self: VALUE) -> VALUE { } let hash = unsafe { rb_hash_new() }; - // TODO: Set counters that are always available here + let counters = ZJITState::get_counters(); + + for &counter in DEFAULT_COUNTERS { + let counter_val = unsafe { *counter_ptr(counter) }; + set_stat(hash, &counter.name(), counter_val); + } // Set counters that are enabled when --zjit-stats is enabled if get_option!(stats) { - let counters = ZJITState::get_counters(); set_stat(hash, "zjit_insns_count", counters.zjit_insns_count); - set_stat(hash, "vm_insns_count", unsafe { rb_vm_insns_count }); + + if unsafe { rb_vm_insns_count } > 0 { + set_stat(hash, "vm_insns_count", unsafe { rb_vm_insns_count }); + } } hash } + +/// Measure the time taken by func() and add that to zjit_compile_time. +pub fn with_time_stat(counter: Counter, func: F) -> R where F: FnOnce() -> R { + let start = Instant::now(); + let ret = func(); + let nanos = Instant::now().duration_since(start).as_nanos(); + incr_counter(counter, nanos as u64); + ret +} From 4f34eddbd3c701bdc1ccc93a192a127e0c33202c Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Mon, 11 Aug 2025 14:35:34 -0700 Subject: [PATCH 05/11] YJIT, ZJIT: Fix JITs compiling prelude (#14171) --- ruby.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ruby.c b/ruby.c index a01e3d8afa9524..6d2b5833b67902 100644 --- a/ruby.c +++ b/ruby.c @@ -1819,8 +1819,10 @@ ruby_opt_init(ruby_cmdline_options_t *opt) if (rb_namespace_available()) rb_initialize_main_namespace(); + rb_namespace_init_done(); + ruby_init_prelude(); - // Initialize JITs after prelude because JITing prelude is typically not optimal. + // Initialize JITs after ruby_init_prelude() because JITing prelude is typically not optimal. #if USE_YJIT rb_yjit_init(opt->yjit); #endif @@ -1831,8 +1833,6 @@ ruby_opt_init(ruby_cmdline_options_t *opt) } #endif - rb_namespace_init_done(); - ruby_init_prelude(); ruby_set_script_name(opt->script_name); require_libraries(&opt->req_list); } From e29d33345402f554220c35617e897e6d52bbecff Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Mon, 11 Aug 2025 23:07:26 +0100 Subject: [PATCH 06/11] ZJIT: Implement `concatstrings` insn (#14154) Co-authored-by: Alexander Momchilov --- test/ruby/test_zjit.rb | 16 +++++ zjit/src/backend/lir.rs | 6 ++ zjit/src/codegen.rs | 51 ++++++++++++++++ zjit/src/hir.rs | 126 ++++++++++++++++++++++++++++------------ 4 files changed, 161 insertions(+), 38 deletions(-) diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index bf43fd1324f307..4dc0919b6b92e3 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -1506,6 +1506,22 @@ def test(val) = val.class }, call_threshold: 2 end + def test_string_concat + assert_compiles '"123"', %q{ + def test = "#{1}#{2}#{3}" + + test + }, insns: [:concatstrings] + end + + def test_string_concat_empty + assert_compiles '""', %q{ + def test = "#{}" + + test + }, insns: [:concatstrings] + end + private # Assert that every method call in `test_script` can be compiled by ZJIT diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index 0902d347c7a29c..86bea62fcd34f0 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -2233,6 +2233,12 @@ impl Assembler { out } + pub fn sub_into(&mut self, left: Opnd, right: Opnd) -> Opnd { + let out = self.sub(left, right); + self.mov(left, out); + out + } + #[must_use] pub fn mul(&mut self, left: Opnd, right: Opnd) -> Opnd { let out = self.new_vreg(Opnd::match_num_bits(&[left, right])); diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index f9532dfe031661..b1b43abbe6dc03 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -330,6 +330,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio Insn::NewRange { low, high, flag, state } => gen_new_range(asm, opnd!(low), opnd!(high), *flag, &function.frame_state(*state)), Insn::ArrayDup { val, state } => gen_array_dup(asm, opnd!(val), &function.frame_state(*state)), Insn::StringCopy { val, chilled, state } => gen_string_copy(asm, opnd!(val), *chilled, &function.frame_state(*state)), + Insn::StringConcat { strings, state } => gen_string_concat(jit, asm, opnds!(strings), &function.frame_state(*state))?, Insn::Param { idx } => unreachable!("block.insns should not have Insn::Param({idx})"), Insn::Snapshot { .. } => return Some(()), // we don't need to do anything for this instruction at the moment Insn::Jump(branch) => return gen_jump(jit, asm, branch), @@ -1456,6 +1457,56 @@ pub fn gen_stub_exit(cb: &mut CodeBlock) -> Option { }) } +fn gen_string_concat(jit: &mut JITState, asm: &mut Assembler, strings: Vec, state: &FrameState) -> Option { + let n = strings.len(); + + // concatstrings shouldn't have 0 strings + // If it happens we abort the compilation for now + if n == 0 { + return None; + } + + gen_prepare_non_leaf_call(jit, asm, state)?; + + // Calculate the compile-time NATIVE_STACK_PTR offset from NATIVE_BASE_PTR + // At this point, frame_setup(&[], jit.c_stack_slots) has been called, + // which allocated aligned_stack_bytes(jit.c_stack_slots) on the stack + let frame_size = aligned_stack_bytes(jit.c_stack_slots); + let allocation_size = aligned_stack_bytes(n); + + asm_comment!(asm, "allocate {} bytes on C stack for {} strings", allocation_size, n); + asm.sub_into(NATIVE_STACK_PTR, allocation_size.into()); + + // Calculate the total offset from NATIVE_BASE_PTR to our buffer + let total_offset_from_base = (frame_size + allocation_size) as i32; + + for (idx, &string_opnd) in strings.iter().enumerate() { + let slot_offset = -total_offset_from_base + (idx as i32 * SIZEOF_VALUE_I32); + asm.mov( + Opnd::mem(VALUE_BITS, NATIVE_BASE_PTR, slot_offset), + string_opnd + ); + } + + let first_string_ptr = asm.lea(Opnd::mem(64, NATIVE_BASE_PTR, -total_offset_from_base)); + + let result = asm_ccall!(asm, rb_str_concat_literals, n.into(), first_string_ptr); + + asm_comment!(asm, "restore C stack pointer"); + asm.add_into(NATIVE_STACK_PTR, allocation_size.into()); + + Some(result) +} + +/// Given the number of spill slots needed for a function, return the number of bytes +/// the function needs to allocate on the stack for the stack frame. +fn aligned_stack_bytes(num_slots: usize) -> usize { + // Both x86_64 and arm64 require the stack to be aligned to 16 bytes. + // Since SIZEOF_VALUE is 8 bytes, we need to round up the size to the nearest even number. + let num_slots = num_slots + (num_slots % 2); + num_slots * SIZEOF_VALUE +} + impl Assembler { /// Make a C call while marking the start and end positions of it fn ccall_with_branch(&mut self, fptr: *const u8, opnds: Vec, branch: &Rc) -> Opnd { diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index bff0fcd7574e7c..5111ab30f912a8 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -445,6 +445,7 @@ pub enum Insn { StringCopy { val: InsnId, chilled: bool, state: InsnId }, StringIntern { val: InsnId }, + StringConcat { strings: Vec, state: InsnId }, /// Put special object (VMCORE, CBASE, etc.) based on value_type PutSpecialObject { value_type: SpecialObjectType }, @@ -675,6 +676,16 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> { Insn::ArrayDup { val, .. } => { write!(f, "ArrayDup {val}") } Insn::HashDup { val, .. } => { write!(f, "HashDup {val}") } Insn::StringCopy { val, .. } => { write!(f, "StringCopy {val}") } + Insn::StringConcat { strings, .. } => { + write!(f, "StringConcat")?; + let mut prefix = " "; + for string in strings { + write!(f, "{prefix}{string}")?; + prefix = ", "; + } + + Ok(()) + } Insn::Test { val } => { write!(f, "Test {val}") } Insn::IsNil { val } => { write!(f, "IsNil {val}") } Insn::Jump(target) => { write!(f, "Jump {target}") } @@ -1135,6 +1146,7 @@ impl Function { &Throw { throw_state, val } => Throw { throw_state, val: find!(val) }, &StringCopy { val, chilled, state } => StringCopy { val: find!(val), chilled, state }, &StringIntern { val } => StringIntern { val: find!(val) }, + &StringConcat { ref strings, state } => StringConcat { strings: find_vec!(strings), state: find!(state) }, &Test { val } => Test { val: find!(val) }, &IsNil { val } => IsNil { val: find!(val) }, &Jump(ref target) => Jump(find_branch_edge!(target)), @@ -1258,6 +1270,7 @@ impl Function { Insn::IsNil { .. } => types::CBool, Insn::StringCopy { .. } => types::StringExact, Insn::StringIntern { .. } => types::StringExact, + Insn::StringConcat { .. } => types::StringExact, Insn::NewArray { .. } => types::ArrayExact, Insn::ArrayDup { .. } => types::ArrayExact, Insn::NewHash { .. } => types::HashExact, @@ -1887,6 +1900,10 @@ impl Function { worklist.push_back(high); worklist.push_back(state); } + &Insn::StringConcat { ref strings, state, .. } => { + worklist.extend(strings); + worklist.push_back(state); + } | &Insn::StringIntern { val } | &Insn::Return { val } | &Insn::Throw { val, .. } @@ -2469,6 +2486,16 @@ impl FrameState { self.stack.pop().ok_or_else(|| ParseError::StackUnderflow(self.clone())) } + fn stack_pop_n(&mut self, count: usize) -> Result, ParseError> { + // Check if we have enough values on the stack + let stack_len = self.stack.len(); + if stack_len < count { + return Err(ParseError::StackUnderflow(self.clone())); + } + + Ok(self.stack.split_off(stack_len - count)) + } + /// Get a stack-top operand fn stack_top(&self) -> Result { self.stack.last().ok_or_else(|| ParseError::StackUnderflow(self.clone())).copied() @@ -2789,24 +2816,23 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let insn_id = fun.push_insn(block, Insn::StringIntern { val }); state.stack_push(insn_id); } + YARVINSN_concatstrings => { + let count = get_arg(pc, 0).as_u32(); + let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); + let strings = state.stack_pop_n(count as usize)?; + let insn_id = fun.push_insn(block, Insn::StringConcat { strings, state: exit_id }); + state.stack_push(insn_id); + } YARVINSN_newarray => { let count = get_arg(pc, 0).as_usize(); let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - let mut elements = vec![]; - for _ in 0..count { - elements.push(state.stack_pop()?); - } - elements.reverse(); + let elements = state.stack_pop_n(count)?; state.stack_push(fun.push_insn(block, Insn::NewArray { elements, state: exit_id })); } YARVINSN_opt_newarray_send => { let count = get_arg(pc, 0).as_usize(); let method = get_arg(pc, 1).as_u32(); - let mut elements = vec![]; - for _ in 0..count { - elements.push(state.stack_pop()?); - } - elements.reverse(); + let elements = state.stack_pop_n(count)?; let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); let (bop, insn) = match method { VM_OPT_NEWARRAY_SEND_MAX => (BOP_MAX, Insn::ArrayMax { elements, state: exit_id }), @@ -2871,13 +2897,10 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { } YARVINSN_pushtoarray => { let count = get_arg(pc, 0).as_usize(); - let mut vals = vec![]; - for _ in 0..count { - vals.push(state.stack_pop()?); - } + let vals = state.stack_pop_n(count)?; let array = state.stack_pop()?; let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - for val in vals.into_iter().rev() { + for val in vals.into_iter() { fun.push_insn(block, Insn::ArrayPush { array, val, state: exit_id }); } state.stack_push(array); @@ -3079,12 +3102,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { } let argc = unsafe { vm_ci_argc((*cd).ci) }; - let mut args = vec![]; - for _ in 0..argc { - args.push(state.stack_pop()?); - } - args.reverse(); - + let args = state.stack_pop_n(argc as usize)?; let recv = state.stack_pop()?; let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); let send = fun.push_insn(block, Insn::SendWithoutBlock { self_val: recv, cd, args, state: exit_id }); @@ -3160,12 +3178,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { } let argc = unsafe { vm_ci_argc((*cd).ci) }; - let mut args = vec![]; - for _ in 0..argc { - args.push(state.stack_pop()?); - } - args.reverse(); - + let args = state.stack_pop_n(argc as usize)?; let recv = state.stack_pop()?; let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); let send = fun.push_insn(block, Insn::SendWithoutBlock { self_val: recv, cd, args, state: exit_id }); @@ -3183,12 +3196,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { } let argc = unsafe { vm_ci_argc((*cd).ci) }; - let mut args = vec![]; - for _ in 0..argc { - args.push(state.stack_pop()?); - } - args.reverse(); - + let args = state.stack_pop_n(argc as usize)?; let recv = state.stack_pop()?; let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); let send = fun.push_insn(block, Insn::Send { self_val: recv, cd, blockiseq, args, state: exit_id }); @@ -5224,7 +5232,47 @@ mod tests { v3:Fixnum[1] = Const Value(1) v5:BasicObject = ObjToString v3 v7:String = AnyToString v3, str: v5 - SideExit UnknownOpcode(concatstrings) + v9:StringExact = StringConcat v2, v7 + Return v9 + "#]]); + } + + #[test] + fn test_string_concat() { + eval(r##" + def test = "#{1}#{2}#{3}" + "##); + assert_method_hir_with_opcode("test", YARVINSN_concatstrings, expect![[r#" + fn test@:2: + bb0(v0:BasicObject): + v2:Fixnum[1] = Const Value(1) + v4:BasicObject = ObjToString v2 + v6:String = AnyToString v2, str: v4 + v7:Fixnum[2] = Const Value(2) + v9:BasicObject = ObjToString v7 + v11:String = AnyToString v7, str: v9 + v12:Fixnum[3] = Const Value(3) + v14:BasicObject = ObjToString v12 + v16:String = AnyToString v12, str: v14 + v18:StringExact = StringConcat v6, v11, v16 + Return v18 + "#]]); + } + + #[test] + fn test_string_concat_empty() { + eval(r##" + def test = "#{}" + "##); + assert_method_hir_with_opcode("test", YARVINSN_concatstrings, expect![[r#" + fn test@:2: + bb0(v0:BasicObject): + v2:StringExact[VALUE(0x1000)] = Const Value(VALUE(0x1000)) + v3:NilClass = Const Value(nil) + v5:BasicObject = ObjToString v3 + v7:String = AnyToString v3, str: v5 + v9:StringExact = StringConcat v2, v7 + Return v9 "#]]); } @@ -7172,7 +7220,8 @@ mod opt_tests { v2:StringExact[VALUE(0x1000)] = Const Value(VALUE(0x1000)) v3:StringExact[VALUE(0x1008)] = Const Value(VALUE(0x1008)) v5:StringExact = StringCopy v3 - SideExit UnknownOpcode(concatstrings) + v11:StringExact = StringConcat v2, v5 + Return v11 "#]]); } @@ -7186,9 +7235,10 @@ mod opt_tests { bb0(v0:BasicObject): v2:StringExact[VALUE(0x1000)] = Const Value(VALUE(0x1000)) v3:Fixnum[1] = Const Value(1) - v10:BasicObject = SendWithoutBlock v3, :to_s - v7:String = AnyToString v3, str: v10 - SideExit UnknownOpcode(concatstrings) + v11:BasicObject = SendWithoutBlock v3, :to_s + v7:String = AnyToString v3, str: v11 + v9:StringExact = StringConcat v2, v7 + Return v9 "#]]); } From 9fb34f4f169736d457e3ff4d6fb4a7a596e211c6 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Mon, 11 Aug 2025 15:36:37 -0700 Subject: [PATCH 07/11] ZJIT: Add --zjit-exec-mem-size (#14175) * ZJIT: Add --zjit-exec-mem-size * Add a comment about the limit --- zjit/src/options.rs | 35 ++++++++++++++++++++++++++++++----- zjit/src/state.rs | 6 +++--- zjit/src/stats.rs | 9 +++++---- 3 files changed, 38 insertions(+), 12 deletions(-) diff --git a/zjit/src/options.rs b/zjit/src/options.rs index 92f56b8916559f..07584c9b99d380 100644 --- a/zjit/src/options.rs +++ b/zjit/src/options.rs @@ -22,6 +22,10 @@ pub static mut OPTIONS: Option = None; #[derive(Clone, Debug)] pub struct Options { + /// Hard limit of the executable memory block to allocate in bytes. + /// Note that the command line argument is expressed in MiB and not bytes. + pub exec_mem_bytes: usize, + /// Number of times YARV instructions should be profiled. pub num_profiles: u8, @@ -58,6 +62,7 @@ pub struct Options { impl Default for Options { fn default() -> Self { Options { + exec_mem_bytes: 64 * 1024 * 1024, num_profiles: 1, stats: false, debug: false, @@ -74,12 +79,18 @@ impl Default for Options { } /// `ruby --help` descriptions for user-facing options. Do not add options for ZJIT developers. -/// Note that --help allows only 80 chars per line, including indentation. 80-char limit --> | +/// Note that --help allows only 80 chars per line, including indentation, and it also puts the +/// description in a separate line if the option name is too long. 80-char limit --> | (any character beyond this `|` column fails the test) pub const ZJIT_OPTIONS: &'static [(&str, &str)] = &[ - ("--zjit-call-threshold=num", "Number of calls to trigger JIT (default: 2)."), - ("--zjit-num-profiles=num", "Number of profiled calls before JIT (default: 1, max: 255)."), - ("--zjit-stats", "Enable collecting ZJIT statistics."), - ("--zjit-perf", "Dump ISEQ symbols into /tmp/perf-{}.map for Linux perf."), + // TODO: Hide --zjit-exec-mem-size from ZJIT_OPTIONS once we add --zjit-mem-size (Shopify/ruby#686) + ("--zjit-exec-mem-size=num", + "Size of executable memory block in MiB (default: 64)."), + ("--zjit-call-threshold=num", + "Number of calls to trigger JIT (default: 2)."), + ("--zjit-num-profiles=num", + "Number of profiled calls before JIT (default: 1, max: 255)."), + ("--zjit-stats", "Enable collecting ZJIT statistics."), + ("--zjit-perf", "Dump ISEQ symbols into /tmp/perf-{}.map for Linux perf."), ("--zjit-log-compiled-iseqs=path", "Log compiled ISEQs to the file. The file will be truncated."), ]; @@ -163,6 +174,20 @@ fn parse_option(str_ptr: *const std::os::raw::c_char) -> Option<()> { match (opt_name, opt_val) { ("", "") => {}, // Simply --zjit + ("mem-size", _) => match opt_val.parse::() { + Ok(n) => { + // Reject 0 or too large values that could overflow. + // The upper bound is 1 TiB but we could make it smaller. + if n == 0 || n > 1024 * 1024 { + return None + } + + // Convert from MiB to bytes internally for convenience + options.exec_mem_bytes = n * 1024 * 1024; + } + Err(_) => return None, + }, + ("call-threshold", _) => match opt_val.parse() { Ok(n) => { unsafe { rb_zjit_call_threshold = n; } diff --git a/zjit/src/state.rs b/zjit/src/state.rs index 79be91fd85e5c6..dca04b7a728497 100644 --- a/zjit/src/state.rs +++ b/zjit/src/state.rs @@ -48,7 +48,7 @@ impl ZJITState { use crate::cruby::*; use crate::options::*; - let exec_mem_size: usize = 64 * 1024 * 1024; // TODO: implement the option + let exec_mem_bytes: usize = get_option!(exec_mem_bytes); let virt_block: *mut u8 = unsafe { rb_zjit_reserve_addr_space(64 * 1024 * 1024) }; // Memory protection syscalls need page-aligned addresses, so check it here. Assuming @@ -73,8 +73,8 @@ impl ZJITState { crate::virtualmem::sys::SystemAllocator {}, page_size, NonNull::new(virt_block).unwrap(), - exec_mem_size, - 64 * 1024 * 1024, // TODO: support the option + exec_mem_bytes, + exec_mem_bytes, // TODO: change this to --zjit-mem-size (Shopify/ruby#686) ); let mem_block = Rc::new(RefCell::new(mem_block)); diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 5b39ecdf4bd7f6..fa8b741eea913e 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -70,10 +70,6 @@ fn incr_counter(counter: Counter, amount: u64) { unsafe { *ptr += amount; } } -pub fn zjit_alloc_size() -> usize { - 0 // TODO: report the actual memory usage -} - /// Return a Hash object that contains ZJIT statistics #[unsafe(no_mangle)] pub extern "C" fn rb_zjit_stats(_ec: EcPtr, _self: VALUE) -> VALUE { @@ -113,3 +109,8 @@ pub fn with_time_stat(counter: Counter, func: F) -> R where F: FnOnce() -> incr_counter(counter, nanos as u64); ret } + +/// The number of bytes ZJIT has allocated on the Rust heap. +pub fn zjit_alloc_size() -> usize { + 0 // TODO: report the actual memory usage to support --zjit-mem-size (Shopify/ruby#686) +} From 39effad4862c7cac31ad8b1dc0bdd984a3f894b6 Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Mon, 11 Aug 2025 21:34:54 +0100 Subject: [PATCH 08/11] [DOC] ZJIT: Add ZJIT to autolink_excluded_words This tells RDoc to not automatically link to the `ZJIT` module so we don't need to keep escaping the word ZJIT in the documentation/comments. --- .rdoc_options | 1 + zjit.rb | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.rdoc_options b/.rdoc_options index 02cf1f00d8771b..76c1c8e0db4041 100644 --- a/.rdoc_options +++ b/.rdoc_options @@ -19,5 +19,6 @@ autolink_excluded_words: - RDoc - Ruby - Set +- ZJIT canonical_root: https://docs.ruby-lang.org/en/master diff --git a/zjit.rb b/zjit.rb index fc2c80c52f2939..7f98c5adc7b97c 100644 --- a/zjit.rb +++ b/zjit.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true -# This module allows for introspection of \ZJIT, CRuby's just-in-time compiler. +# This module allows for introspection of ZJIT, CRuby's just-in-time compiler. # Everything in the module is highly implementation specific and the API might # be less stable compared to the standard library. # -# This module may not exist if \ZJIT does not support the particular platform +# This module may not exist if ZJIT does not support the particular platform # for which CRuby is built. module RubyVM::ZJIT # Avoid calling a Ruby method here to avoid interfering with compilation tests @@ -14,12 +14,12 @@ module RubyVM::ZJIT end class << RubyVM::ZJIT - # Check if \ZJIT is enabled + # Check if ZJIT is enabled def enabled? Primitive.cexpr! 'RBOOL(rb_zjit_enabled_p)' end - # Return \ZJIT statistics as a Hash + # Return ZJIT statistics as a Hash def stats stats = Primitive.rb_zjit_stats return nil if stats.nil? @@ -32,7 +32,7 @@ def stats stats end - # Get the summary of \ZJIT statistics as a String + # Get the summary of ZJIT statistics as a String def stats_string buf = +'' stats = self.stats From 4da569b53ef355e7d11085ff448599f25599bad3 Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Mon, 11 Aug 2025 22:27:27 +0100 Subject: [PATCH 09/11] [DOC] YJIT: Add YJIT to autolink_excluded_words --- .rdoc_options | 1 + yjit.rb | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.rdoc_options b/.rdoc_options index 76c1c8e0db4041..b8b511efe67f1e 100644 --- a/.rdoc_options +++ b/.rdoc_options @@ -20,5 +20,6 @@ autolink_excluded_words: - Ruby - Set - ZJIT +- YJIT canonical_root: https://docs.ruby-lang.org/en/master diff --git a/yjit.rb b/yjit.rb index 1655529b5ee7f5..751400a43ecc8d 100644 --- a/yjit.rb +++ b/yjit.rb @@ -1,14 +1,14 @@ # frozen_string_literal: true # :markup: markdown -# This module allows for introspection of \YJIT, CRuby's just-in-time compiler. +# This module allows for introspection of YJIT, CRuby's just-in-time compiler. # Everything in the module is highly implementation specific and the API might # be less stable compared to the standard library. # -# This module may not exist if \YJIT does not support the particular platform +# This module may not exist if YJIT does not support the particular platform # for which CRuby is built. module RubyVM::YJIT - # Check if \YJIT is enabled. + # Check if YJIT is enabled. def self.enabled? Primitive.cexpr! 'RBOOL(rb_yjit_enabled_p)' end @@ -33,8 +33,8 @@ def self.reset_stats! Primitive.rb_yjit_reset_stats_bang end - # Enable \YJIT compilation. `stats` option decides whether to enable \YJIT stats or not. `log` decides - # whether to enable \YJIT compilation logging or not. Optional `mem_size` and `call_threshold` can be + # Enable YJIT compilation. `stats` option decides whether to enable YJIT stats or not. `log` decides + # whether to enable YJIT compilation logging or not. Optional `mem_size` and `call_threshold` can be # provided to override default configuration. # # * `stats`: From 0070c26aecdf0f6692ad6a03315ccf64f593c38e Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Mon, 11 Aug 2025 15:40:28 -0400 Subject: [PATCH 10/11] ZJIT: CI: Use Rust version built into GitHub Actions image Saves the work of installing Rust for most jobs. Keep a job on each platform that tests 1.85.0, the minimum supported version, though. --- .github/workflows/zjit-macos.yml | 10 +++++++--- .github/workflows/zjit-ubuntu.yml | 6 +++++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/.github/workflows/zjit-macos.yml b/.github/workflows/zjit-macos.yml index ab922849f45373..7b5d9f6b6107ce 100644 --- a/.github/workflows/zjit-macos.yml +++ b/.github/workflows/zjit-macos.yml @@ -34,6 +34,7 @@ jobs: include: - test_task: 'zjit-check' configure: '--enable-yjit=dev --enable-zjit' + rust_version: "1.85.0" - test_task: 'ruby' # build test for combo build configure: '--enable-yjit --enable-zjit' @@ -81,14 +82,17 @@ jobs: # Set fetch-depth: 10 so that Launchable can receive commits information. fetch-depth: 10 + - name: Install Rust + if: ${{ matrix.rust_version }} + run: | + rustup install ${{ matrix.rust_version }} --profile minimal + rustup default ${{ matrix.rust_version }} + - uses: taiki-e/install-action@v2 with: tool: nextest@0.9 if: ${{ matrix.test_task == 'zjit-check' }} - - name: Install Rust # TODO(alan): remove when GitHub images catch up past 1.85.0 - run: rustup default 1.85.0 - - name: Run configure run: ../src/configure -C --disable-install-doc ${{ matrix.configure }} diff --git a/.github/workflows/zjit-ubuntu.yml b/.github/workflows/zjit-ubuntu.yml index 09fa137f45a571..da53a52b7d9f1d 100644 --- a/.github/workflows/zjit-ubuntu.yml +++ b/.github/workflows/zjit-ubuntu.yml @@ -39,6 +39,7 @@ jobs: - test_task: 'zjit-check' configure: '--enable-yjit --enable-zjit=dev' + rust_version: '1.85.0' - test_task: 'zjit-test-all' configure: '--enable-zjit=dev' @@ -98,7 +99,10 @@ jobs: fetch-depth: 10 - name: Install Rust - run: rustup default 1.85.0 + if: ${{ matrix.rust_version }} + run: | + rustup install ${{ matrix.rust_version }} --profile minimal + rustup default ${{ matrix.rust_version }} - name: Install rustfmt if: ${{ matrix.test_task == 'zjit-bindgen' }} From 8b1afbc6ed84364506f43c2107f6cb634d92f4be Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Mon, 11 Aug 2025 16:44:22 -0400 Subject: [PATCH 11/11] CI: Surface Rust warnings on PRs that touch any Rust code Rust PRs will have a failed CI step if they trigger any warnings. This helps us stay on top of warnings from new Rust releases and also ones we accidentally write. Fix a typo for demo, since this only runs when Rust files are changed. --- .github/workflows/rust-warnings.yml | 53 +++++++++++++++++++++++++++++ zjit/src/hir.rs | 2 +- 2 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/rust-warnings.yml diff --git a/.github/workflows/rust-warnings.yml b/.github/workflows/rust-warnings.yml new file mode 100644 index 00000000000000..b05ebbfe64ff3e --- /dev/null +++ b/.github/workflows/rust-warnings.yml @@ -0,0 +1,53 @@ +# Surface Rust warnings on PRs that touch any Rust code. +# Not a required check so we never block people over new warnings +# that might come from a new Rust version being released. +name: Rust warnings +on: + pull_request: + types: + - opened + - synchronize + - reopened + paths: + - '**.rs' + - '!**.inc.rs' + merge_group: + +concurrency: + group: ${{ github.workflow }} / ${{ startsWith(github.event_name, 'pull') && github.ref_name || github.sha }} + cancel-in-progress: ${{ startsWith(github.event_name, 'pull') }} + +permissions: + contents: read + +jobs: + make: + env: + GITPULLOPTIONS: --no-tags origin ${{ github.ref }} + + runs-on: ubuntu-24.04 + + if: >- + ${{!(false + || contains(github.event.head_commit.message, '[DOC]') + || contains(github.event.head_commit.message, 'Document') + || contains(github.event.pull_request.title, '[DOC]') + || contains(github.event.pull_request.title, 'Document') + || contains(github.event.pull_request.labels.*.name, 'Documentation') + || (github.event_name == 'push' && github.event.pull_request.user.login == 'dependabot[bot]') + )}} + + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - name: Install Rust + run: rustup default beta + + - name: Rust warnings + run: | + set -euo pipefail + cargo check --quiet --all-features --message-format=json \ + | jq -r 'select(.reason == "compiler-message" and .message.level == "warning") | .message.rendered' \ + > warnings.txt + cat warnings.txt + ! grep --quiet '[^[:space:]]' warnings.txt diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 5111ab30f912a8..f3d39281ad1cec 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -343,7 +343,7 @@ impl<'a> std::fmt::Display for ConstPrinter<'a> { /// /// Because this is extra state external to any pointer being printed, a /// printing adapter struct that wraps the pointer along with this map is -/// required to make use of this effectly. The [`std::fmt::Display`] +/// required to make use of this effectively. The [`std::fmt::Display`] /// implementation on the adapter struct can then be reused to implement /// `Display` on the inner type with a default [`PtrPrintMap`], which /// does not perform any mapping.