YJIT: Fix ruby2_keywords edge case and drop bogus checks

YJIT didn't guard for ruby2_keywords hash in case of splat calls that
land in methods with a rest parameter, creating incorrect results.

The compile checks didn't correspond to any actual effects of
ruby2_keywords, so it was masking this bug and YJIT was refusing to
compile things needlessly. About 16% of fallback reasons in `lobsters`
was due to the ISeq check.

We already handle the tagging part with
exit_if_supplying_kw_and_has_no_kw() and should now have a dynamic guard
for all splat cases.

bootstraptest/test_yjit.rb

@@ -4369,3 +4369,19 @@ def entry
   def defined_yield = defined?(yield)
   [defined_yield, defined_yield {}]
 }
+
+# splat with ruby2_keywords into rest parameter
+assert_equal '[[{:a=>1}], {}]', %q{
+  ruby2_keywords def foo(*args) = args
+
+  def bar(*args, **kw) = [args, kw]
+
+  def pass_bar(*args) = bar(*args)
+
+  def body
+    args = foo(a: 1)
+    pass_bar(*args)
+  end
+
+  body
+}

yjit/src/codegen.rs

@@ -5555,18 +5555,6 @@ fn gen_send_cfunc(
         return None;
     }
 
-    // In order to handle backwards compatibility between ruby 3 and 2
-    // ruby2_keywords was introduced. It is called only on methods
-    // with splat and changes they way they handle them.
-    // We are just going to not compile these.
-    // https://docs.ruby-lang.org/en/3.2/Module.html#method-i-ruby2_keywords
-    if unsafe {
-        get_iseq_flags_ruby2_keywords(jit.iseq) && flags & VM_CALL_ARGS_SPLAT != 0
-    } {
-        gen_counter_incr(asm, Counter::send_args_splat_cfunc_ruby2_keywords);
-        return None;
-    }
-
     let kw_arg = unsafe { vm_ci_kwarg(ci) };
     let kw_arg_num = if kw_arg.is_null() {
         0
@@ -6020,6 +6008,23 @@ fn push_splat_args(required_args: u32, asm: &mut Assembler) {
     }
 }
 
+c_callable! {
+    /// Return non-zero when `obj` is an array and its last item is a
+    /// `ruby2_keywords` hash. We don't support this kind of splat.
+    fn splat_array_ends_in_ruby2_keywords(obj: VALUE) -> usize {
+        unsafe {
+            if !RB_TYPE_P(obj, RUBY_T_ARRAY) {
+                return 0;
+            }
+            let last = rb_ary_entry_internal(obj, -1);
+            if !RB_TYPE_P(last, RUBY_T_HASH) {
+                return 0;
+            }
+            return last.builtin_flags() & RHASH_PASS_AS_KEYWORDS as usize;
+        }
+    }
+}
+
 fn gen_send_bmethod(
     jit: &mut JITState,
     asm: &mut Assembler,
@@ -6152,7 +6157,6 @@ fn gen_send_iseq(
     exit_if_has_post(asm, iseq)?;
     exit_if_has_kwrest(asm, iseq)?;
     exit_if_kw_splat(asm, flags)?;
-    exit_if_splat_and_ruby2_keywords(asm, jit, flags)?;
     exit_if_has_rest_and_captured(asm, iseq_has_rest, captured_opnd)?;
     exit_if_has_rest_and_supplying_kws(asm, iseq_has_rest, iseq, supplying_kws)?;
     exit_if_supplying_kw_and_has_no_kw(asm, supplying_kws, iseq)?;
@@ -6473,7 +6477,14 @@ fn gen_send_iseq(
 
     // push_splat_args does stack manipulation so we can no longer side exit
     if let Some(array_length) = splat_array_length {
-        if !iseq_has_rest {
+        if iseq_has_rest {
+            // All splats need to guard for ruby2_keywords hash.
+            // When splatting into a rest param since the index
+            // for the last item in the array is dynamic.
+            let bad_splat = asm.ccall(splat_array_ends_in_ruby2_keywords as _, vec![asm.stack_opnd(0)]);
+            asm.cmp(bad_splat, 0.into());
+            asm.jnz(Target::side_exit(Counter::guard_send_splatarray_last_ruby_2_keywords));
+        } else {
             // Speculate that future splats will be done with
             // an array that has the same length. We will insert guards.
             argc = argc - 1 + array_length as i32;
@@ -6968,20 +6979,6 @@ fn exit_if_kw_splat(asm: &mut Assembler, flags: u32) -> Option<()> {
     exit_if(asm, flags & VM_CALL_KW_SPLAT != 0, Counter::send_iseq_kw_splat)
 }
 
-#[must_use]
-fn exit_if_splat_and_ruby2_keywords(asm: &mut Assembler, jit: &mut JITState, flags: u32) -> Option<()> {
-    // In order to handle backwards compatibility between ruby 3 and 2
-    // ruby2_keywords was introduced. It is called only on methods
-    // with splat and changes they way they handle them.
-    // We are just going to not compile these.
-    // https://www.rubydoc.info/stdlib/core/Proc:ruby2_keywords
-    exit_if(
-        asm,
-        unsafe { get_iseq_flags_ruby2_keywords(jit.iseq) } && flags & VM_CALL_ARGS_SPLAT != 0,
-        Counter::send_iseq_ruby2_keywords,
-    )
-}
-
 #[must_use]
 fn exit_if_has_rest_and_captured(asm: &mut Assembler, iseq_has_rest: bool, captured_opnd: Option<Opnd>) -> Option<()> {
     exit_if(asm, iseq_has_rest && captured_opnd.is_some(), Counter::send_iseq_has_rest_and_captured)

yjit/src/cruby.rs

@@ -165,7 +165,6 @@ pub use rb_get_iseq_flags_has_lead as get_iseq_flags_has_lead;
 pub use rb_get_iseq_flags_has_opt as get_iseq_flags_has_opt;
 pub use rb_get_iseq_flags_has_kw as get_iseq_flags_has_kw;
 pub use rb_get_iseq_flags_has_rest as get_iseq_flags_has_rest;
-pub use rb_get_iseq_flags_ruby2_keywords as get_iseq_flags_ruby2_keywords;
 pub use rb_get_iseq_flags_has_post as get_iseq_flags_has_post;
 pub use rb_get_iseq_flags_has_kwrest as get_iseq_flags_has_kwrest;
 pub use rb_get_iseq_flags_has_block as get_iseq_flags_has_block;

yjit/src/stats.rs

@@ -358,10 +358,8 @@ make_counters! {
     send_args_splat_opt_call,
     send_args_splat_cfunc_var_args,
     send_args_splat_cfunc_zuper,
-    send_args_splat_cfunc_ruby2_keywords,
     send_iseq_splat_arity_error,
     send_splat_too_long,
-    send_iseq_ruby2_keywords,
     send_send_not_imm,
     send_send_wrong_args,
     send_send_null_mid,