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)
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/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
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/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..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())
@@ -1183,8 +1185,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) {
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("