From ac24f70fb0c0cc80d30ec6e96b3369079775d0dc Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Mon, 8 Sep 2025 21:01:29 +0900 Subject: [PATCH 1/5] Align the conditions for did_you_mean Probably due to the testing order, sometimes it looks like that `Gem::UnknownCommandError` happens to be used without registered in `DidYouMean`. --- lib/rubygems/exceptions.rb | 4 +--- test/rubygems/test_gem_command_manager.rb | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/rubygems/exceptions.rb b/lib/rubygems/exceptions.rb index 362b09dcbb9949..0f65c76daf5af8 100644 --- a/lib/rubygems/exceptions.rb +++ b/lib/rubygems/exceptions.rb @@ -21,13 +21,11 @@ def initialize(unknown_command) end def self.attach_correctable - return if defined?(@attached) + return if method_defined?(:corrections) if defined?(DidYouMean) && DidYouMean.respond_to?(:correct_error) DidYouMean.correct_error(Gem::UnknownCommandError, Gem::UnknownCommandSpellChecker) end - - @attached = true end end diff --git a/test/rubygems/test_gem_command_manager.rb b/test/rubygems/test_gem_command_manager.rb index eeda4d94cff1c5..50a2270e3d246e 100644 --- a/test/rubygems/test_gem_command_manager.rb +++ b/test/rubygems/test_gem_command_manager.rb @@ -79,7 +79,7 @@ def test_find_command_unknown_suggestions message = "Unknown command pish".dup - if defined?(DidYouMean) + if e.respond_to?(:corrections) message << "\nDid you mean? \"push\"" end From 08091adec3f5d454efc31969a5eaf0102acea8a8 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 8 Sep 2025 11:02:54 +0200 Subject: [PATCH 2/5] Add spec for missing or faulty `to_proc` methods Followup: https://github.com/ruby/ruby/pull/14463 --- spec/ruby/language/fixtures/send.rb | 10 ++++++++++ spec/ruby/language/send_spec.rb | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/spec/ruby/language/fixtures/send.rb b/spec/ruby/language/fixtures/send.rb index 5d1d9da21439d2..4787abee5cf476 100644 --- a/spec/ruby/language/fixtures/send.rb +++ b/spec/ruby/language/fixtures/send.rb @@ -81,6 +81,16 @@ def to_proc end end + class RawToProc + def initialize(to_proc) + @to_proc = to_proc + end + + def to_proc + @to_proc + end + end + class ToAry def initialize(obj) @obj = obj diff --git a/spec/ruby/language/send_spec.rb b/spec/ruby/language/send_spec.rb index aaccdf0998b135..bd69593d76a551 100644 --- a/spec/ruby/language/send_spec.rb +++ b/spec/ruby/language/send_spec.rb @@ -106,6 +106,24 @@ specs.yield_now(&o).should == :from_to_proc end + ruby_version_is "3.5" do + it "raises TypeError if 'to_proc' doesn't return a Proc" do + o = LangSendSpecs::RawToProc.new(42) + + -> { + specs.makeproc(&o) + }.should raise_error(TypeError, "can't convert LangSendSpecs::RawToProc to Proc (LangSendSpecs::RawToProc#to_proc gives Integer)") + end + + it "raises TypeError if block object isn't a Proc and doesn't respond to `to_proc`" do + o = Object.new + + -> { + specs.makeproc(&o) + }.should raise_error(TypeError, "no implicit conversion of Object into Proc") + end + end + it "raises a SyntaxError with both a literal block and an object as block" do -> { eval "specs.oneb(10, &l){ 42 }" From 41b11a3512317aa3965d8dc425155c9c2d7cdaf6 Mon Sep 17 00:00:00 2001 From: Randy Stauner Date: Mon, 8 Sep 2025 08:53:19 -0700 Subject: [PATCH 3/5] ZJIT: Add --zjit-stats=quiet option to collect stats without printing (#14467) Similar to YJIT's --yjit-stats=quiet, this option allows ZJIT to collect statistics and make them available via the Ruby API without printing them at exit. This is useful for programmatic access to stats without the output noise. - Added print_stats field to Options struct - Modified option parsing to support --zjit-stats=quiet - Added rb_zjit_print_stats_p primitive to check if stats should be printed - Updated zjit.rb to only register at_exit handler when print_stats is true - Update the help text shown by `ruby --help` to indicate that --zjit-stats now accepts an optional =quiet parameter. - Added test for --zjit-stats=quiet option --- test/ruby/test_zjit.rb | 26 +++++++++++++++++++++++++- zjit.c | 1 + zjit.rb | 2 +- zjit/src/options.rs | 22 +++++++++++++++++++++- 4 files changed, 48 insertions(+), 3 deletions(-) diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index b751d482e2b41d..9bfe2c0c00596b 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -27,6 +27,30 @@ def test_stats_enabled RUBY end + def test_stats_quiet + # Test that --zjit-stats=quiet collects stats but doesn't print them + script = <<~RUBY + def test = 42 + test + test + puts RubyVM::ZJIT.stats_enabled? + RUBY + + stats_header = "***ZJIT: Printing ZJIT statistics on exit***" + + # With --zjit-stats, stats should be printed to stderr + out, err, status = eval_with_jit(script, stats: true) + assert_success(out, err, status) + assert_includes(err, stats_header) + assert_equal("true\n", out) + + # With --zjit-stats=quiet, stats should NOT be printed but still enabled + out, err, status = eval_with_jit(script, stats: :quiet) + assert_success(out, err, status) + refute_includes(err, stats_header) + assert_equal("true\n", out) + end + def test_enable_through_env child_env = {'RUBY_YJIT_ENABLE' => nil, 'RUBY_ZJIT_ENABLE' => '1'} assert_in_out_err([child_env, '-v'], '') do |stdout, stderr| @@ -2490,7 +2514,7 @@ def eval_with_jit( if zjit args << "--zjit-call-threshold=#{call_threshold}" args << "--zjit-num-profiles=#{num_profiles}" - args << "--zjit-stats" if stats + args << "--zjit-stats#{"=#{stats}" unless stats == true}" if stats args << "--zjit-debug" if debug if allowed_iseqs jitlist = Tempfile.new("jitlist") diff --git a/zjit.c b/zjit.c index 313cced2aa1d6d..e1ea6d7e098060 100644 --- a/zjit.c +++ b/zjit.c @@ -343,6 +343,7 @@ rb_zjit_insn_leaf(int insn, const VALUE *opes) VALUE rb_zjit_assert_compiles(rb_execution_context_t *ec, VALUE self); VALUE rb_zjit_stats(rb_execution_context_t *ec, VALUE self, VALUE target_key); VALUE rb_zjit_stats_enabled_p(rb_execution_context_t *ec, VALUE self); +VALUE rb_zjit_print_stats_p(rb_execution_context_t *ec, VALUE self); // Preprocessed zjit.rb generated during build #include "zjit.rbinc" diff --git a/zjit.rb b/zjit.rb index 7e5807876ce9a8..d70ff1dd47622d 100644 --- a/zjit.rb +++ b/zjit.rb @@ -8,7 +8,7 @@ # for which CRuby is built. module RubyVM::ZJIT # Avoid calling a Ruby method here to avoid interfering with compilation tests - if Primitive.rb_zjit_stats_enabled_p + if Primitive.rb_zjit_print_stats_p at_exit { print_stats } end end diff --git a/zjit/src/options.rs b/zjit/src/options.rs index 3efcb933bb52ec..dbb6ee8ebbdf24 100644 --- a/zjit/src/options.rs +++ b/zjit/src/options.rs @@ -41,6 +41,9 @@ pub struct Options { /// Enable YJIT statsitics pub stats: bool, + /// Print stats on exit (when stats is also true) + pub print_stats: bool, + /// Enable debug logging pub debug: bool, @@ -78,6 +81,7 @@ impl Default for Options { exec_mem_bytes: 64 * 1024 * 1024, num_profiles: DEFAULT_NUM_PROFILES, stats: false, + print_stats: false, debug: false, disable_hir_opt: false, dump_hir_init: None, @@ -103,7 +107,7 @@ pub const ZJIT_OPTIONS: &[(&str, &str)] = &[ "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-stats[=quiet]", "Enable collecting ZJIT statistics (=quiet to suppress output)."), ("--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."), @@ -220,6 +224,11 @@ fn parse_option(str_ptr: *const std::os::raw::c_char) -> Option<()> { ("stats", "") => { options.stats = true; + options.print_stats = true; + } + ("stats", "quiet") => { + options.stats = true; + options.print_stats = false; } ("debug", "") => options.debug = true, @@ -344,3 +353,14 @@ pub extern "C" fn rb_zjit_stats_enabled_p(_ec: EcPtr, _self: VALUE) -> VALUE { Qfalse } } + +/// Return Qtrue if stats should be printed at exit. +#[unsafe(no_mangle)] +pub extern "C" fn rb_zjit_print_stats_p(_ec: EcPtr, _self: VALUE) -> VALUE { + // Builtin zjit.rb calls this even if ZJIT is disabled, so OPTIONS may not be set. + if unsafe { OPTIONS.as_ref() }.is_some_and(|opts| opts.stats && opts.print_stats) { + Qtrue + } else { + Qfalse + } +} From 5ee083c71a3893cee0a3a3229eb8b58c0bd13cba Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 8 Sep 2025 17:55:24 +0200 Subject: [PATCH 4/5] Bump ABI version Followup changes in https://github.com/ruby/ruby/pull/14470 / 03c86b053197f3cd6bece1925e634c1d74d196d0 --- include/ruby/internal/abi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/ruby/internal/abi.h b/include/ruby/internal/abi.h index 7ceb8c40b70eef..322761fc683341 100644 --- a/include/ruby/internal/abi.h +++ b/include/ruby/internal/abi.h @@ -24,7 +24,7 @@ * In released versions of Ruby, this number is not defined since teeny * versions of Ruby should guarantee ABI compatibility. */ -#define RUBY_ABI_VERSION 3 +#define RUBY_ABI_VERSION 4 /* Windows does not support weak symbols so ruby_abi_version will not exist * in the shared library. */ From 866e474ac854db0655a89a801f9bc871e9ec1ce0 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Mon, 8 Sep 2025 09:50:33 -0700 Subject: [PATCH 5/5] ZJIT: Fix backtraces on opt_new (#14461) --- .github/auto_request_review.yml | 1 - .github/workflows/zjit-macos.yml | 2 - .github/workflows/zjit-ubuntu.yml | 3 +- doc/zjit.md | 10 --- test/.excludes-zjit/TestERBCore.rb | 1 - test/.excludes-zjit/TestERBCoreWOStrScan.rb | 1 - test/ruby/test_zjit.rb | 31 +++++++ vm_insnhelper.c | 6 ++ zjit/bindgen/src/main.rs | 2 + zjit/src/codegen.rs | 14 ++++ zjit/src/cruby_bindings.inc.rs | 6 ++ zjit/src/hir.rs | 93 ++++++++++++++++----- zjit/zjit.mk | 4 - 13 files changed, 131 insertions(+), 43 deletions(-) delete mode 100644 test/.excludes-zjit/TestERBCore.rb delete mode 100644 test/.excludes-zjit/TestERBCoreWOStrScan.rb diff --git a/.github/auto_request_review.yml b/.github/auto_request_review.yml index 264e6ef1774018..c4c94681f0e387 100644 --- a/.github/auto_request_review.yml +++ b/.github/auto_request_review.yml @@ -10,7 +10,6 @@ files: 'zjit/src/cruby_bindings.inc.rs': [] 'doc/zjit*': [team:jit] 'test/ruby/test_zjit*': [team:jit] - 'test/.excludes-zjit/*': [team:jit] 'defs/jit.mk': [team:jit] options: ignore_draft: true diff --git a/.github/workflows/zjit-macos.yml b/.github/workflows/zjit-macos.yml index bb244dd1b28bcd..892b605ce8114f 100644 --- a/.github/workflows/zjit-macos.yml +++ b/.github/workflows/zjit-macos.yml @@ -111,7 +111,6 @@ jobs: RUN_OPTS="$RUN_OPTS" SPECOPTS="$SPECOPTS" TESTOPTS="$TESTOPTS" - TEST_EXCLUDES="$TEST_EXCLUDES" timeout-minutes: 60 env: RUBY_TESTOPTS: '-q --tty=no' @@ -119,7 +118,6 @@ jobs: SYNTAX_SUGGEST_TIMEOUT: '5' PRECHECK_BUNDLED_GEMS: 'no' TESTS: ${{ matrix.tests }} - TEST_EXCLUDES: '--excludes-dir=../src/test/.excludes-zjit --name=!/memory_leak/' continue-on-error: ${{ matrix.continue-on-test_task || false }} result: diff --git a/.github/workflows/zjit-ubuntu.yml b/.github/workflows/zjit-ubuntu.yml index 3861e2481fc8d9..b9efcaaf5fab7b 100644 --- a/.github/workflows/zjit-ubuntu.yml +++ b/.github/workflows/zjit-ubuntu.yml @@ -149,13 +149,12 @@ jobs: run: >- make -s ${{ matrix.test_task }} ${TESTS:+TESTS="$TESTS"} RUN_OPTS="$RUN_OPTS" MSPECOPT=--debug SPECOPTS="$SPECOPTS" - TESTOPTS="$TESTOPTS" TEST_EXCLUDES="$TEST_EXCLUDES" + TESTOPTS="$TESTOPTS" ZJIT_BINDGEN_DIFF_OPTS="$ZJIT_BINDGEN_DIFF_OPTS" timeout-minutes: 90 env: RUBY_TESTOPTS: '-q --tty=no' TEST_BUNDLED_GEMS_ALLOW_FAILURES: '' - TEST_EXCLUDES: '--excludes-dir=../src/test/.excludes-zjit --name=!/memory_leak/' PRECHECK_BUNDLED_GEMS: 'no' SYNTAX_SUGGEST_TIMEOUT: '5' ZJIT_BINDGEN_DIFF_OPTS: '--exit-code' diff --git a/doc/zjit.md b/doc/zjit.md index 04ca0a8bb45de1..b5b605d5cb0855 100644 --- a/doc/zjit.md +++ b/doc/zjit.md @@ -96,16 +96,6 @@ use `make`. -### make zjit-test-all - -``` -make zjit-test-all -``` - -This command runs all Ruby tests under `/test/ruby/` with ZJIT enabled. - -Certain tests are excluded under `/test/.excludes-zjit`. - ### test/ruby/test\_zjit.rb This command runs Ruby execution tests. diff --git a/test/.excludes-zjit/TestERBCore.rb b/test/.excludes-zjit/TestERBCore.rb deleted file mode 100644 index 9ab398de6f5065..00000000000000 --- a/test/.excludes-zjit/TestERBCore.rb +++ /dev/null @@ -1 +0,0 @@ -exclude(:test_invalid_trim_mode, 'Test fails with ZJIT') diff --git a/test/.excludes-zjit/TestERBCoreWOStrScan.rb b/test/.excludes-zjit/TestERBCoreWOStrScan.rb deleted file mode 100644 index 9ab398de6f5065..00000000000000 --- a/test/.excludes-zjit/TestERBCoreWOStrScan.rb +++ /dev/null @@ -1 +0,0 @@ -exclude(:test_invalid_trim_mode, 'Test fails with ZJIT') diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index 9bfe2c0c00596b..280a7503d41150 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -766,6 +766,37 @@ def test(a, b) = a >= b }, insns: [:opt_ge], call_threshold: 2 end + def test_opt_new_does_not_push_frame + assert_compiles 'nil', %q{ + class Foo + attr_reader :backtrace + + def initialize + @backtrace = caller + end + end + def test = Foo.new + + foo = test + foo.backtrace.find do |frame| + frame.include?('Class#new') + end + }, insns: [:opt_new] + end + + def test_opt_new_with_redefinition + assert_compiles '"foo"', %q{ + class Foo + def self.new = "foo" + + def initialize = raise("unreachable") + end + def test = Foo.new + + test + }, insns: [:opt_new] + end + def test_new_hash_empty assert_compiles '{}', %q{ def test = {} diff --git a/vm_insnhelper.c b/vm_insnhelper.c index eee20cd9d4e3fd..cc22e4a2e0e59d 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -2417,6 +2417,12 @@ vm_method_cfunc_is(const rb_iseq_t *iseq, CALL_DATA cd, VALUE recv, cfunc_type f return check_cfunc(cme, func); } +int +rb_vm_method_cfunc_is(const rb_iseq_t *iseq, CALL_DATA cd, VALUE recv, cfunc_type func) +{ + return vm_method_cfunc_is(iseq, cd, recv, func); +} + #define check_cfunc(me, func) check_cfunc(me, make_cfunc_type(func)) #define vm_method_cfunc_is(iseq, cd, recv, func) vm_method_cfunc_is(iseq, cd, recv, make_cfunc_type(func)) diff --git a/zjit/bindgen/src/main.rs b/zjit/bindgen/src/main.rs index e3609e237e0584..56400e20cd742e 100644 --- a/zjit/bindgen/src/main.rs +++ b/zjit/bindgen/src/main.rs @@ -417,6 +417,8 @@ fn main() { // From internal/object.h .allowlist_function("rb_class_allocate_instance") .allowlist_function("rb_obj_equal") + .allowlist_function("rb_class_new_instance_pass_kw") + .allowlist_function("rb_obj_alloc") // From gc.h and internal/gc.h .allowlist_function("rb_obj_info") diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 5c13d278fb8436..1db1ddc510c0dc 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -349,6 +349,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio Insn::NewRange { low, high, flag, state } => gen_new_range(jit, asm, opnd!(low), opnd!(high), *flag, &function.frame_state(*state)), Insn::NewRangeFixnum { low, high, flag, state } => gen_new_range_fixnum(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::ObjectAlloc { val, state } => gen_object_alloc(asm, opnd!(val), &function.frame_state(*state)), Insn::StringCopy { val, chilled, state } => gen_string_copy(asm, opnd!(val), *chilled, &function.frame_state(*state)), // concatstrings shouldn't have 0 strings // If it happens we abort the compilation for now @@ -385,6 +386,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio Insn::FixnumAnd { left, right } => gen_fixnum_and(asm, opnd!(left), opnd!(right)), Insn::FixnumOr { left, right } => gen_fixnum_or(asm, opnd!(left), opnd!(right)), Insn::IsNil { val } => gen_isnil(asm, opnd!(val)), + &Insn::IsMethodCfunc { val, cd, cfunc } => gen_is_method_cfunc(jit, asm, opnd!(val), cd, cfunc), Insn::Test { val } => gen_test(asm, opnd!(val)), Insn::GuardType { val, guard_type, state } => gen_guard_type(jit, asm, opnd!(val), *guard_type, &function.frame_state(*state)), Insn::GuardBitEquals { val, expected, state } => gen_guard_bit_equals(jit, asm, opnd!(val), *expected, &function.frame_state(*state)), @@ -1190,6 +1192,11 @@ fn gen_new_range_fixnum( asm_ccall!(asm, rb_range_new, low, high, (flag as i64).into()) } +fn gen_object_alloc(asm: &mut Assembler, val: lir::Opnd, state: &FrameState) -> lir::Opnd { + gen_prepare_call_with_gc(asm, state); + asm_ccall!(asm, rb_obj_alloc, val) +} + /// Compile code that exits from JIT code with a return value fn gen_return(asm: &mut Assembler, val: lir::Opnd) { // Pop the current frame (ec->cfp++) @@ -1292,6 +1299,13 @@ fn gen_isnil(asm: &mut Assembler, val: lir::Opnd) -> lir::Opnd { asm.csel_e(Opnd::Imm(1), Opnd::Imm(0)) } +fn gen_is_method_cfunc(jit: &JITState, asm: &mut Assembler, val: lir::Opnd, cd: *const rb_call_data, cfunc: *const u8) -> lir::Opnd { + unsafe extern "C" { + fn rb_vm_method_cfunc_is(iseq: IseqPtr, cd: *const rb_call_data, recv: VALUE, cfunc: *const u8) -> VALUE; + } + asm_ccall!(asm, rb_vm_method_cfunc_is, VALUE(jit.iseq as usize).into(), (cd as usize).into(), val, (cfunc as usize).into()) +} + fn gen_anytostring(asm: &mut Assembler, val: lir::Opnd, str: lir::Opnd, state: &FrameState) -> lir::Opnd { gen_prepare_call_with_gc(asm, state); diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index 0523c00ede24a7..ae394f9c604ff4 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -806,7 +806,13 @@ unsafe extern "C" { pub fn rb_id2str(id: ID) -> VALUE; pub fn rb_sym2str(symbol: VALUE) -> VALUE; pub fn rb_class2name(klass: VALUE) -> *const ::std::os::raw::c_char; + pub fn rb_class_new_instance_pass_kw( + argc: ::std::os::raw::c_int, + argv: *const VALUE, + klass: VALUE, + ) -> VALUE; pub fn rb_obj_is_kind_of(obj: VALUE, klass: VALUE) -> VALUE; + pub fn rb_obj_alloc(klass: VALUE) -> VALUE; pub fn rb_obj_frozen_p(obj: VALUE) -> VALUE; pub fn rb_class_inherited_p(scion: VALUE, ascendant: VALUE) -> VALUE; pub fn rb_backref_get() -> VALUE; diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index af44c86cbe93b3..6bec2ab552a1d8 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -519,11 +519,16 @@ pub enum Insn { HashDup { val: InsnId, state: InsnId }, + /// Allocate an instance of the `val` class without calling `#initialize` on it. + ObjectAlloc { val: InsnId, state: InsnId }, + /// Check if the value is truthy and "return" a C boolean. In reality, we will likely fuse this /// with IfTrue/IfFalse in the backend to generate jcc. Test { val: InsnId }, /// Return C `true` if `val` is `Qnil`, else `false`. IsNil { val: InsnId }, + /// Return C `true` if `val`'s method on cd resolves to the cfunc. + IsMethodCfunc { val: InsnId, cd: *const rb_call_data, cfunc: *const u8 }, Defined { op_type: usize, obj: VALUE, pushval: VALUE, v: InsnId, state: InsnId }, GetConstantPath { ic: *const iseq_inline_constant_cache, state: InsnId }, @@ -761,6 +766,7 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> { } Insn::ArrayDup { val, .. } => { write!(f, "ArrayDup {val}") } Insn::HashDup { val, .. } => { write!(f, "HashDup {val}") } + Insn::ObjectAlloc { val, .. } => { write!(f, "ObjectAlloc {val}") } Insn::StringCopy { val, .. } => { write!(f, "StringCopy {val}") } Insn::StringConcat { strings, .. } => { write!(f, "StringConcat")?; @@ -796,6 +802,7 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> { } Insn::Test { val } => { write!(f, "Test {val}") } Insn::IsNil { val } => { write!(f, "IsNil {val}") } + Insn::IsMethodCfunc { val, cd, .. } => { write!(f, "IsMethodCFunc {val}, :{}", ruby_call_method_name(*cd)) } Insn::Jump(target) => { write!(f, "Jump {target}") } Insn::IfTrue { val, target } => { write!(f, "IfTrue {val}, {target}") } Insn::IfFalse { val, target } => { write!(f, "IfFalse {val}, {target}") } @@ -1273,6 +1280,7 @@ impl Function { &ToRegexp { opt, ref values, state } => ToRegexp { opt, values: find_vec!(values), state }, &Test { val } => Test { val: find!(val) }, &IsNil { val } => IsNil { val: find!(val) }, + &IsMethodCfunc { val, cd, cfunc } => IsMethodCfunc { val: find!(val), cd, cfunc }, Jump(target) => Jump(find_branch_edge!(target)), &IfTrue { val, ref target } => IfTrue { val: find!(val), target: find_branch_edge!(target) }, &IfFalse { val, ref target } => IfFalse { val: find!(val), target: find_branch_edge!(target) }, @@ -1333,6 +1341,7 @@ impl Function { &InvokeBuiltin { bf, ref args, state, return_type } => InvokeBuiltin { bf, args: find_vec!(args), state, return_type }, &ArrayDup { val, state } => ArrayDup { val: find!(val), state }, &HashDup { val, state } => HashDup { val: find!(val), state }, + &ObjectAlloc { val, state } => ObjectAlloc { val: find!(val), state }, &CCall { cfun, ref args, name, return_type, elidable } => CCall { cfun, args: find_vec!(args), name, return_type, elidable }, &Defined { op_type, obj, pushval, v, state } => Defined { op_type, obj, pushval, v: find!(v), state: find!(state) }, &DefinedIvar { self_val, pushval, id, state } => DefinedIvar { self_val: find!(self_val), pushval, id, state }, @@ -1407,6 +1416,7 @@ impl Function { Insn::IsNil { val } if self.is_a(*val, types::NilClass) => Type::from_cbool(true), Insn::IsNil { val } if !self.type_of(*val).could_be(types::NilClass) => Type::from_cbool(false), Insn::IsNil { .. } => types::CBool, + Insn::IsMethodCfunc { .. } => types::CBool, Insn::StringCopy { .. } => types::StringExact, Insn::StringIntern { .. } => types::Symbol, Insn::StringConcat { .. } => types::StringExact, @@ -1417,6 +1427,7 @@ impl Function { Insn::HashDup { .. } => types::HashExact, Insn::NewRange { .. } => types::RangeExact, Insn::NewRangeFixnum { .. } => types::RangeExact, + Insn::ObjectAlloc { .. } => types::HeapObject, 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)), @@ -2173,12 +2184,14 @@ impl Function { | &Insn::Return { val } | &Insn::Test { val } | &Insn::SetLocal { val, .. } - | &Insn::IsNil { val } => + | &Insn::IsNil { val } + | &Insn::IsMethodCfunc { val, .. } => worklist.push_back(val), &Insn::SetGlobal { val, state, .. } | &Insn::Defined { v: val, state, .. } | &Insn::StringIntern { val, state } | &Insn::StringCopy { val, state, .. } + | &Insn::ObjectAlloc { val, state } | &Insn::GuardType { val, state, .. } | &Insn::GuardBitEquals { val, state, .. } | &Insn::GuardShape { val, state, .. } @@ -3340,16 +3353,30 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { state.stack_pop()?; } YARVINSN_opt_new => { - let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - fun.push_insn(block, Insn::CheckInterrupts { state: exit_id }); - let offset = get_arg(pc, 1).as_i64(); - let target_idx = insn_idx_at_offset(insn_idx, offset); + let cd: *const rb_call_data = get_arg(pc, 0).as_ptr(); + let dst = get_arg(pc, 1).as_i64(); + + // Check if #new resolves to rb_class_new_instance_pass_kw. + // TODO: Guard on a profiled class and add a patch point for #new redefinition + let argc = unsafe { vm_ci_argc((*cd).ci) } as usize; + let val = state.stack_topn(argc)?; + let test_id = fun.push_insn(block, Insn::IsMethodCfunc { val, cd, cfunc: rb_class_new_instance_pass_kw as *const u8 }); + + // Jump to the fallback block if it's not the expected function. + // Skip CheckInterrupts since the #new call will do it very soon anyway. + let target_idx = insn_idx_at_offset(insn_idx, dst); let target = insn_idx_to_block[&target_idx]; - // Skip the fast-path and go straight to the fallback code. We will let the - // optimizer take care of the converting Class#new->alloc+initialize instead. - fun.push_insn(block, Insn::Jump(BranchEdge { target, args: state.as_args(self_param) })); + let _branch_id = fun.push_insn(block, Insn::IfFalse { + val: test_id, + target: BranchEdge { target, args: state.as_args(self_param) } + }); queue.push_back((state.clone(), target, target_idx, local_inval)); - break; // Don't enqueue the next block as a successor + + // Move on to the fast path + let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); + let insn_id = fun.push_insn(block, Insn::ObjectAlloc { val, state: exit_id }); + state.stack_setn(argc, insn_id); + state.stack_setn(argc + 1, insn_id); } YARVINSN_jump => { let offset = get_arg(pc, 0).as_i64(); @@ -5201,14 +5228,18 @@ mod tests { bb0(v0:BasicObject): v5:BasicObject = GetConstantPath 0x1000 v6:NilClass = Const Value(nil) + v7:CBool = IsMethodCFunc v5, :new + IfFalse v7, bb1(v0, v6, v5) + v10:HeapObject = ObjectAlloc v5 + v12:BasicObject = SendWithoutBlock v10, :initialize CheckInterrupts - Jump bb1(v0, v6, v5) - bb1(v10:BasicObject, v11:NilClass, v12:BasicObject): - v15:BasicObject = SendWithoutBlock v12, :new - Jump bb2(v10, v15, v11) - bb2(v17:BasicObject, v18:BasicObject, v19:NilClass): + Jump bb2(v0, v10, v12) + bb1(v16:BasicObject, v17:NilClass, v18:BasicObject): + v21:BasicObject = SendWithoutBlock v18, :new + Jump bb2(v16, v21, v17) + bb2(v23:BasicObject, v24:BasicObject, v25:BasicObject): CheckInterrupts - Return v18 + Return v24 "); } @@ -7842,12 +7873,20 @@ mod opt_tests { bb0(v0:BasicObject): PatchPoint SingleRactorMode PatchPoint StableConstantNames(0x1000, C) - v28:Class[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + v34:Class[VALUE(0x1008)] = Const Value(VALUE(0x1008)) v6:NilClass = Const Value(nil) + v7:CBool = IsMethodCFunc v34, :new + IfFalse v7, bb1(v0, v6, v34) + v10:HeapObject = ObjectAlloc v34 + v12:BasicObject = SendWithoutBlock v10, :initialize CheckInterrupts - v15:BasicObject = SendWithoutBlock v28, :new + Jump bb2(v0, v10, v12) + bb1(v16:BasicObject, v17:NilClass, v18:Class[VALUE(0x1008)]): + v21:BasicObject = SendWithoutBlock v18, :new + Jump bb2(v16, v21, v17) + bb2(v23:BasicObject, v24:BasicObject, v25:BasicObject): CheckInterrupts - Return v15 + Return v24 "); } @@ -7867,13 +7906,23 @@ mod opt_tests { bb0(v0:BasicObject): PatchPoint SingleRactorMode PatchPoint StableConstantNames(0x1000, C) - v30:Class[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + v36:Class[VALUE(0x1008)] = Const Value(VALUE(0x1008)) v6:NilClass = Const Value(nil) v7:Fixnum[1] = Const Value(1) + v8:CBool = IsMethodCFunc v36, :new + IfFalse v8, bb1(v0, v6, v36, v7) + v11:HeapObject = ObjectAlloc v36 + PatchPoint MethodRedefined(C@0x1008, initialize@0x1010, cme:0x1018) + v38:HeapObject[class_exact:C] = GuardType v11, HeapObject[class_exact:C] + v39:BasicObject = SendWithoutBlockDirect v38, :initialize (0x1040), v7 + CheckInterrupts + Jump bb2(v0, v11, v39) + bb1(v17:BasicObject, v18:NilClass, v19:Class[VALUE(0x1008)], v20:Fixnum[1]): + v23:BasicObject = SendWithoutBlock v19, :new, v20 + Jump bb2(v17, v23, v18) + bb2(v25:BasicObject, v26:BasicObject, v27:BasicObject): CheckInterrupts - v17:BasicObject = SendWithoutBlock v30, :new, v7 - CheckInterrupts - Return v17 + Return v26 "); } diff --git a/zjit/zjit.mk b/zjit/zjit.mk index c39abf28263efe..be989bdecd41c3 100644 --- a/zjit/zjit.mk +++ b/zjit/zjit.mk @@ -50,10 +50,6 @@ zjit-check: $(MAKE) zjit-test $(MAKE) test-all TESTS='$(top_srcdir)/test/ruby/test_zjit.rb' -.PHONY: zjit-test-all -zjit-test-all: - $(MAKE) test-all RUST_BACKTRACE=1 TEST_EXCLUDES='--excludes-dir=$(top_srcdir)/test/.excludes-zjit --name=!/memory_leak/' RUN_OPTS='--zjit-call-threshold=1' - ZJIT_BINDGEN_DIFF_OPTS = # Generate Rust bindings. See source for details.