From da01faaaccba3be326bfa607f6b36699274e329c Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 19 Aug 2025 11:57:19 -0700 Subject: [PATCH 1/5] ZJIT: Remove try_num_bits (#14272) --- zjit/src/backend/lir.rs | 25 +++++++------------------ zjit/src/codegen.rs | 4 ++-- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index 460e2719dd9a38..be5bda052d1a5d 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -147,26 +147,15 @@ impl Opnd } } - /// Return Some(Opnd) with a given num_bits if self has num_bits. - /// None if self doesn't have a num_bits field. - pub fn try_num_bits(&self, num_bits: u8) -> Option { - assert!(num_bits == 8 || num_bits == 16 || num_bits == 32 || num_bits == 64); - match *self { - Opnd::Reg(reg) => Some(Opnd::Reg(reg.with_num_bits(num_bits))), - Opnd::Mem(Mem { base, disp, .. }) => Some(Opnd::Mem(Mem { base, disp, num_bits })), - Opnd::VReg { idx, .. } => Some(Opnd::VReg { idx, num_bits }), - _ => None, - } - } - - /// Return Opnd with a given num_bits if self has num_bits. - /// Panic otherwise. This should be used only when you know which Opnd self is. + /// Return Opnd with a given num_bits if self has num_bits. Panic otherwise. #[track_caller] pub fn with_num_bits(&self, num_bits: u8) -> Opnd { - if let Some(opnd) = self.try_num_bits(num_bits) { - opnd - } else { - unreachable!("with_num_bits should not be used on: {self:?}"); + assert!(num_bits == 8 || num_bits == 16 || num_bits == 32 || num_bits == 64); + match *self { + Opnd::Reg(reg) => Opnd::Reg(reg.with_num_bits(num_bits)), + Opnd::Mem(Mem { base, disp, .. }) => Opnd::Mem(Mem { base, disp, num_bits }), + Opnd::VReg { idx, .. } => Opnd::VReg { idx, num_bits }, + _ => unreachable!("with_num_bits should not be used for: {self:?}"), } } diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 5fb83b5f48a5fa..c588a93651fe6a 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -1183,8 +1183,8 @@ fn gen_guard_type(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, guard asm.jne(side_exit(jit, state, GuardType(guard_type))); } else if guard_type.is_subtype(types::StaticSymbol) { // Static symbols have (val & 0xff) == RUBY_SYMBOL_FLAG - // Use 8-bit comparison like YJIT does - debug_assert!(val.try_num_bits(8).is_some(), "GuardType should not be used for a known constant, but val was: {val:?}"); + // Use 8-bit comparison like YJIT does. GuardType should not be used + // for a known VALUE, which with_num_bits() does not support. asm.cmp(val.with_num_bits(8), Opnd::UImm(RUBY_SYMBOL_FLAG as u64)); asm.jne(side_exit(jit, state, GuardType(guard_type))); } else if guard_type.is_subtype(types::NilClass) { From c1f16fc36d5bbe4c3886e3a2439121d2cb18f622 Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Tue, 19 Aug 2025 12:08:52 -0700 Subject: [PATCH 2/5] Fix special consts unit tests for i686 (#14271) 32-bit platforms do not have flonum and something about the static symbol test was flaky. --- test/ruby/test_shapes.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/ruby/test_shapes.rb b/test/ruby/test_shapes.rb index 50b1679c187e69..aa488e04218b28 100644 --- a/test/ruby/test_shapes.rb +++ b/test/ruby/test_shapes.rb @@ -1045,9 +1045,10 @@ def test_raise_on_special_consts assert_raise ArgumentError do RubyVM::Shape.of(0) end - assert_raise ArgumentError do - RubyVM::Shape.of(:foo) - end + # 32-bit platforms don't have flonums or static symbols as special + # constants + # TODO(max): Add ArgumentError tests for symbol and flonum, skipping if + # RUBY_PLATFORM =~ /i686/ end def test_root_shape_transition_to_special_const_on_frozen From e639aaacbf7e8b48ee1a3bb73615a4792de6ec5d Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Tue, 19 Aug 2025 13:50:47 -0400 Subject: [PATCH 3/5] ZJIT: Prepare for rb_range_new() calling <=> gen_prepare_call_with_gc() was not enough because of the rb_funcall() usage in range_init(). Co-authored-by: Takashi Kokubun Co-authored-by: Max Bernstein --- test/ruby/test_zjit.rb | 15 +++++++++++++++ zjit/src/codegen.rs | 6 ++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index e18333a58f07f7..b5b5a6984797d4 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -1855,6 +1855,21 @@ def test = /#{1}#{2}#{3}/ }, insns: [:toregexp] end + def test_new_range_non_leaf + assert_compiles '(0/1)..1', %q{ + def jit_entry(v) = make_range_then_exit(v) + + def make_range_then_exit(v) + range = (v..1) + super rescue range # TODO(alan): replace super with side-exit intrinsic + end + + jit_entry(0) # profile + jit_entry(0) # compile + jit_entry(0/1r) # run without stub + }, call_threshold: 2 + end + private # Assert that every method call in `test_script` can be compiled by ZJIT diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index c588a93651fe6a..958efd49fafaa0 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -338,7 +338,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio Insn::Const { val: Const::Value(val) } => gen_const(*val), Insn::NewArray { elements, state } => gen_new_array(asm, opnds!(elements), &function.frame_state(*state)), Insn::NewHash { elements, state } => gen_new_hash(jit, asm, elements, &function.frame_state(*state)), - Insn::NewRange { low, high, flag, state } => gen_new_range(asm, opnd!(low), opnd!(high), *flag, &function.frame_state(*state)), + Insn::NewRange { low, high, flag, state } => gen_new_range(jit, 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)), // concatstrings shouldn't have 0 strings @@ -1040,13 +1040,15 @@ fn gen_new_hash( /// Compile a new range instruction fn gen_new_range( + jit: &JITState, asm: &mut Assembler, low: lir::Opnd, high: lir::Opnd, flag: RangeType, state: &FrameState, ) -> lir::Opnd { - gen_prepare_call_with_gc(asm, state); + // Sometimes calls `low.<=>(high)` + gen_prepare_non_leaf_call(jit, asm, state); // Call rb_range_new(low, high, flag) asm_ccall!(asm, rb_range_new, low, high, (flag as i64).into()) From 6b197dec4844eb5967158c16495724cac7eecab2 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Tue, 19 Aug 2025 15:14:03 -0400 Subject: [PATCH 4/5] ZJIT: Mark Insn::NewRange as having side effects --- range.c | 1 + zjit/src/hir.rs | 27 ++++++++++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/range.c b/range.c index 12077a068e469c..615154be4c95c0 100644 --- a/range.c +++ b/range.c @@ -47,6 +47,7 @@ static VALUE r_cover_p(VALUE, VALUE, VALUE, VALUE); static void range_init(VALUE range, VALUE beg, VALUE end, VALUE exclude_end) { + // Changing this condition has implications for JITs. If you do, please let maintainers know. if ((!FIXNUM_P(beg) || !FIXNUM_P(end)) && !NIL_P(beg) && !NIL_P(end)) { VALUE v; diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 7c7e09663b07ad..e7aaf64f288eb7 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -638,7 +638,6 @@ impl Insn { // NewHash's operands may be hashed and compared for equality, which could have // side-effects. Insn::NewHash { elements, .. } => elements.len() > 0, - Insn::NewRange { .. } => false, Insn::ArrayDup { .. } => false, Insn::HashDup { .. } => false, Insn::Test { .. } => false, @@ -660,6 +659,9 @@ impl Insn { Insn::GetLocal { .. } => false, Insn::IsNil { .. } => 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(). + Insn::NewRange { .. } => true, _ => true, } } @@ -6198,6 +6200,29 @@ mod opt_tests { "#]]); } + #[test] + fn test_do_not_eliminate_new_range_non_fixnum() { + eval(" + def test() + _ = (-'a'..'b') + 0 + end + test; test + "); + assert_optimized_method_hir("test", expect![[r#" + fn test@:3: + bb0(v0:BasicObject): + v1:NilClass = Const Value(nil) + v4:StringExact[VALUE(0x1000)] = Const Value(VALUE(0x1000)) + PatchPoint BOPRedefined(STRING_REDEFINED_OP_FLAG, BOP_UMINUS) + v6:StringExact[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + v8:StringExact = StringCopy v6 + v10:RangeExact = NewRange v4 NewRangeInclusive v8 + v11:Fixnum[0] = Const Value(0) + Return v11 + "#]]); + } + #[test] fn test_eliminate_new_array_with_elements() { eval(" From b3053cbb34de21ae97c07ffda17432d35f9cc1d4 Mon Sep 17 00:00:00 2001 From: Burdette Lamar Date: Tue, 19 Aug 2025 18:34:40 -0500 Subject: [PATCH 5/5] [DOC] Tweaks for Object#hash --- hash.c | 43 +++++++++++++++++++------------------------ 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/hash.c b/hash.c index de9bc97ea69cdf..8c645c3d847efc 100644 --- a/hash.c +++ b/hash.c @@ -321,40 +321,35 @@ objid_hash(VALUE obj) #endif } -/** +/* * call-seq: - * obj.hash -> integer - * - * Generates an Integer hash value for this object. This function must have the - * property that a.eql?(b) implies a.hash == b.hash. - * - * The hash value is used along with #eql? by the Hash class to determine if - * two objects reference the same hash key. Any hash value that exceeds the - * capacity of an Integer will be truncated before being used. + * hash -> integer * - * The hash value for an object may not be identical across invocations or - * implementations of Ruby. If you need a stable identifier across Ruby - * invocations and implementations you will need to generate one with a custom - * method. + * Returns the integer hash value for +self+; + * has the property that if foo.eql?(bar) + * then foo.hash == bar.hash. * - * Certain core classes such as Integer use built-in hash calculations and - * do not call the #hash method when used as a hash key. + * \Class Hash uses both #hash and #eql? to determine whether two objects + * used as hash keys are to be treated as the same key. + * A hash value that exceeds the capacity of an Integer is truncated before being used. * - * When implementing your own #hash based on multiple values, the best - * practice is to combine the class and any values using the hash code of an - * array: + * Many core classes override method Object#hash; + * other core classes (e.g., Integer) calculate the hash internally, + * and do not call the #hash method when used as a hash key. * - * For example: + * When implementing #hash for a user-defined class, + * best practice is to use Array#hash with the class name and the values + * that are important in the instance; + * this takes advantage of that method's logic for safely and efficiently + * generating a hash value: * * def hash * [self.class, a, b, c].hash * end * - * The reason for this is that the Array#hash method already has logic for - * safely and efficiently combining multiple hash values. - *-- - * \private - *++ + * The hash value may differ among invocations or implementations of Ruby. + * If you need stable hash-like identifiers across Ruby invocations and implementations, + * use a custom method to generate them. */ VALUE rb_obj_hash(VALUE obj)