[pull] master from ruby:master

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 <code>a.eql?(b)</code> implies <code>a.hash == b.hash</code>.
- *
- * 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 <tt>foo.eql?(bar)</tt>
+ * then <tt>foo.hash == bar.hash</tt>.
  *
- * 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)

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;
 

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

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

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<Opnd> {
-        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:?}"),
         }
     }
 

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) {

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@<compiled>: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("