From 6c0a74e55ad1c15804297b3a471444f0cae5c058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Luiz=20Tiago=20Soares?= Date: Mon, 22 Sep 2025 12:28:27 -0400 Subject: [PATCH 1/4] ZJIT: Refactor NewRangeFixnum (#14607) ## Context #14409 https://github.com/ruby/ruby/pull/14409#discussion_r2350238583 >You may have noticed that this doesn't produce a New range instruction. For that you probably need to use a local variable (makes it harder for the bytecode compiler to reason about, but the JIT sees through it). If you have time, please rewrite these tests. https://github.com/ruby/ruby/pull/14409#discussion_r2350240389 >There's some code above to do this for you: see arguments_likely_fixnums and coerce something or other ## Changes ### Refactor tests Modify tests force the usage of `NewRangeFixnum` instruction and make tests names more consistent. ### Refactor optimize ranges Didn't found `arguments_likely_fixnums` to be applicable unless we enable profiling for `NewRange` and change the criteria of when the optimization fires. But there were other ways to clean up the code. Simplify the logic and move it to `type_specialize`. Deleted the standalone `optimize_ranges` function. --- test/ruby/test_zjit.rb | 18 ++++---- zjit/src/hir.rs | 95 ++++++++++++++++-------------------------- 2 files changed, 45 insertions(+), 68 deletions(-) diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index f383beeb3b1788..f0109bda8b147c 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -1136,19 +1136,21 @@ def test(n) = n..10 def test_new_range_fixnum_both_literals_inclusive assert_compiles '1..2', %q{ def test() - (1..2) + a = 2 + (1..a) end test; test - }, call_threshold: 2 + }, call_threshold: 2, insns: [:newrange] end def test_new_range_fixnum_both_literals_exclusive assert_compiles '1...2', %q{ def test() - (1...2) + a = 2 + (1...a) end test; test - }, call_threshold: 2 + }, call_threshold: 2, insns: [:newrange] end def test_new_range_fixnum_low_literal_inclusive @@ -1157,7 +1159,7 @@ def test(a) (1..a) end test(2); test(3) - }, call_threshold: 2 + }, call_threshold: 2, insns: [:newrange] end def test_new_range_fixnum_low_literal_exclusive @@ -1166,7 +1168,7 @@ def test(a) (1...a) end test(2); test(3) - }, call_threshold: 2 + }, call_threshold: 2, insns: [:newrange] end def test_new_range_fixnum_high_literal_inclusive @@ -1175,7 +1177,7 @@ def test(a) (a..10) end test(2); test(3) - }, call_threshold: 2 + }, call_threshold: 2, insns: [:newrange] end def test_new_range_fixnum_high_literal_exclusive @@ -1184,7 +1186,7 @@ def test(a) (a...10) end test(2); test(3) - }, call_threshold: 2 + }, call_threshold: 2, insns: [:newrange] end def test_if diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 7305bc007d934f..6b1d22ac111e38 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -1986,6 +1986,20 @@ impl Function { self.insn_types[replacement.0] = self.infer_type(replacement); self.make_equal_to(insn_id, replacement); } + Insn::NewRange { low, high, flag, state } => { + let low_is_fix = self.is_a(low, types::Fixnum); + let high_is_fix = self.is_a(high, types::Fixnum); + + if low_is_fix || high_is_fix { + let low_fix = self.coerce_to_fixnum(block, low, state); + let high_fix = self.coerce_to_fixnum(block, high, state); + let replacement = self.push_insn(block, Insn::NewRangeFixnum { low: low_fix, high: high_fix, flag, state }); + self.make_equal_to(insn_id, replacement); + self.insn_types[replacement.0] = self.infer_type(replacement); + } else { + self.push_insn_id(block, insn_id); + }; + } _ => { self.push_insn_id(block, insn_id); } } } @@ -2194,52 +2208,6 @@ impl Function { .unwrap_or(insn_id) } - fn optimize_ranges(&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::NewRange { low, high, flag, state } => { - - // The NewRange rewrite triggers mostly on literals because that is the - // case we can easily prove Fixnum statically and cheaply guard the other - // side. - let low_is_fix = self.is_a(low, types::Fixnum); - let high_is_fix = self.is_a(high, types::Fixnum); - - if low_is_fix && high_is_fix { - // Both statically fixnum => specialize directly - let repl = self.push_insn(block, Insn::NewRangeFixnum { low, high, flag, state }); - self.make_equal_to(insn_id, repl); - self.insn_types[repl.0] = self.infer_type(repl); - } else if low_is_fix { - // Only left is fixnum => guard right - let high_fix = self.coerce_to_fixnum(block, high, state); - let repl = self.push_insn(block, Insn::NewRangeFixnum { low, high: high_fix, flag, state }); - self.make_equal_to(insn_id, repl); - self.insn_types[repl.0] = self.infer_type(repl); - } else if high_is_fix { - // Only right is fixnum => guard left - let low_fix = self.coerce_to_fixnum(block, low, state); - let repl = self.push_insn(block, Insn::NewRangeFixnum { low: low_fix, high, flag, state }); - self.make_equal_to(insn_id, repl); - self.insn_types[repl.0] = self.infer_type(repl); - } else { - // Keep generic op - self.push_insn_id(block, insn_id); - } - } - _ => { - self.push_insn_id(block, insn_id); - } - } - } - } - self.infer_types(); - } - /// Use type information left by `infer_types` to fold away operations that can be evaluated at compile-time. /// /// It can fold fixnum math, truthiness tests, and branches with constant conditionals. @@ -2635,8 +2603,6 @@ impl Function { #[cfg(debug_assertions)] self.assert_validates(); self.optimize_c_calls(); #[cfg(debug_assertions)] self.assert_validates(); - self.optimize_ranges(); - #[cfg(debug_assertions)] self.assert_validates(); self.fold_constants(); #[cfg(debug_assertions)] self.assert_validates(); self.clean_cfg(); @@ -7253,41 +7219,50 @@ mod opt_tests { } #[test] - fn test_optimize_range_fixnum_inclusive_literals() { + fn test_optimize_new_range_fixnum_inclusive_literals() { eval(" def test() - (1..2) + a = 2 + (1..a) end test; test "); assert_snapshot!(hir_string("test"), @r" fn test@:3: bb0(v0:BasicObject): - v4:RangeExact[VALUE(0x1000)] = Const Value(VALUE(0x1000)) + v1:NilClass = Const Value(nil) + v5:Fixnum[2] = Const Value(2) + v8:Fixnum[1] = Const Value(1) + v16:RangeExact = NewRangeFixnum v8 NewRangeInclusive v5 CheckInterrupts - Return v4 + Return v16 "); } + #[test] - fn test_optimize_range_fixnum_exclusive_literals() { + fn test_optimize_new_range_fixnum_exclusive_literals() { eval(" def test() - (1...2) + a = 2 + (1...a) end test; test "); assert_snapshot!(hir_string("test"), @r" fn test@:3: bb0(v0:BasicObject): - v4:RangeExact[VALUE(0x1000)] = Const Value(VALUE(0x1000)) + v1:NilClass = Const Value(nil) + v5:Fixnum[2] = Const Value(2) + v8:Fixnum[1] = Const Value(1) + v16:RangeExact = NewRangeFixnum v8 NewRangeExclusive v5 CheckInterrupts - Return v4 + Return v16 "); } #[test] - fn test_optimize_range_fixnum_inclusive_high_guarded() { + fn test_optimize_new_range_fixnum_inclusive_high_guarded() { eval(" def test(a) (1..a) @@ -7306,7 +7281,7 @@ mod opt_tests { } #[test] - fn test_optimize_range_fixnum_exclusive_high_guarded() { + fn test_optimize_new_range_fixnum_exclusive_high_guarded() { eval(" def test(a) (1...a) @@ -7325,7 +7300,7 @@ mod opt_tests { } #[test] - fn test_optimize_range_fixnum_inclusive_low_guarded() { + fn test_optimize_new_range_fixnum_inclusive_low_guarded() { eval(" def test(a) (a..10) @@ -7344,7 +7319,7 @@ mod opt_tests { } #[test] - fn test_optimize_range_fixnum_exclusive_low_guarded() { + fn test_optimize_new_range_fixnum_exclusive_low_guarded() { eval(" def test(a) (a...10) From ccb82041548f871a0185eead619ce62e1df5957d Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Mon, 22 Sep 2025 17:30:00 +0100 Subject: [PATCH 2/4] ZJIT: Improve `zjit_bisect.rb` (#14592) ZJIT: Improve zjit_bisect.rb 1. The command name in the help message is outdated. 2. When the command failed by itself, the message can be clearer. --- tool/zjit_bisect.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tool/zjit_bisect.rb b/tool/zjit_bisect.rb index 8f49af4fc686d6..175bbe5feb34e2 100755 --- a/tool/zjit_bisect.rb +++ b/tool/zjit_bisect.rb @@ -17,9 +17,10 @@ end end.parse! -RUBY = ARGV[0] || raise("Usage: ruby jit_bisect.rb -- ") +usage = "Usage: zjit_bisect.rb -- " +RUBY = ARGV[0] || raise(usage) OPTIONS = ARGV[1..] -raise("Usage: ruby jit_bisect.rb -- ") if OPTIONS.empty? +raise(usage) if OPTIONS.empty? LOGGER = Logger.new($stdout) # From https://github.com/tekknolagi/omegastar @@ -103,7 +104,8 @@ def run_with_jit_list(ruby, options, jit_list) # Try running with no JIT list to get a stable baseline unless run_with_jit_list(RUBY, OPTIONS, []).success? - raise "Command failed with empty JIT list" + cmd = [RUBY, "--zjit-allowed-iseqs=/dev/null", *OPTIONS].shelljoin + raise "The command failed unexpectedly with an empty JIT list. To reproduce, try running the following: `#{cmd}`" end # Collect the JIT list from the failing Ruby process jit_list = nil From 4afc6370cba7e83c204edcdf1c2f8d1497af2f7c Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Thu, 14 Aug 2025 16:03:50 -0700 Subject: [PATCH 3/4] Add missing locks to method table --- vm_method.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/vm_method.c b/vm_method.c index 7295c74c7b6369..238ad62f4b134b 100644 --- a/vm_method.c +++ b/vm_method.c @@ -754,7 +754,9 @@ rb_clear_all_refinement_method_cache(void) void rb_method_table_insert(VALUE klass, struct rb_id_table *table, ID method_id, const rb_method_entry_t *me) { - rb_method_table_insert0(klass, table, method_id, me, RB_TYPE_P(klass, T_ICLASS) && !RICLASS_OWNS_M_TBL_P(klass)); + RB_VM_LOCKING() { + rb_method_table_insert0(klass, table, method_id, me, RB_TYPE_P(klass, T_ICLASS) && !RICLASS_OWNS_M_TBL_P(klass)); + } } void @@ -1545,7 +1547,9 @@ method_added(VALUE klass, ID mid) void rb_add_method(VALUE klass, ID mid, rb_method_type_t type, void *opts, rb_method_visibility_t visi) { - rb_method_entry_make(klass, mid, klass, visi, type, NULL, mid, opts); + RB_VM_LOCKING() { + rb_method_entry_make(klass, mid, klass, visi, type, NULL, mid, opts); + } if (type != VM_METHOD_TYPE_UNDEF && type != VM_METHOD_TYPE_REFINED) { method_added(klass, mid); @@ -1570,11 +1574,14 @@ static rb_method_entry_t * method_entry_set(VALUE klass, ID mid, const rb_method_entry_t *me, rb_method_visibility_t visi, VALUE defined_class) { - rb_method_entry_t *newme = rb_method_entry_make(klass, mid, defined_class, visi, - me->def->type, me->def, 0, NULL); - if (newme == me) { - me->def->no_redef_warning = TRUE; - METHOD_ENTRY_FLAGS_SET(newme, visi, FALSE); + rb_method_entry_t *newme; + RB_VM_LOCKING() { + newme = rb_method_entry_make(klass, mid, defined_class, visi, + me->def->type, me->def, 0, NULL); + if (newme == me) { + me->def->no_redef_warning = TRUE; + METHOD_ENTRY_FLAGS_SET(newme, visi, FALSE); + } } method_added(klass, mid); From 1d1529629ce1550fad19c2d9410c4bf4995230d2 Mon Sep 17 00:00:00 2001 From: Randy Stauner Date: Mon, 22 Sep 2025 10:03:00 -0700 Subject: [PATCH 4/4] Clarify what happens when IO.popen's block returns (#14626) --- io.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/io.c b/io.c index f6daa793990899..caa364a70bbd9a 100644 --- a/io.c +++ b/io.c @@ -7835,7 +7835,8 @@ static VALUE popen_finish(VALUE port, VALUE klass); * If a block is given, the stream is passed to the block * (again, open for reading, writing, or both); * when the block exits, the stream is closed, - * and the block's value is assigned to global variable $? and returned. + * the block's value is returned, + * and the global variable $? is set to the child's exit status. * * Optional argument +mode+ may be any valid \IO mode. * See {Access Modes}[rdoc-ref:File@Access+Modes].