https://git.ruby-lang.org/ruby.git/commit/?id=36134f7d29

From 36134f7d293b3f02e85599a3374d670aa575aeb0 Mon Sep 17 00:00:00 2001
From: Alan Wu <XrXr@u...>
Date: Wed, 28 Apr 2021 12:59:26 -0400
Subject: Implement calls to methods with simple optional params

* Implement calls to methods with simple optional params

* Remove unnecessary MJIT_STATIC

See comment for MJIT_STATIC. I added it not knowing whether it's
required because the function next to it has it. Don't use it and wait
for problems to come up instead.

* Better naming, some comments

* Count bailing on kw only iseqs

On railsbench:
```
opt_send_without_block exit reasons:
                  bmethod      59729 (27.7%)
         optimized_method      59137 (27.5%)
      iseq_complex_callee      41362 (19.2%)
             alias_method      33346 (15.5%)
      callsite_not_simple      19170 ( 8.9%)
       iseq_only_keywords       1300 ( 0.6%)
                 kw_splat       1299 ( 0.6%)
    cfunc_ruby_array_varg         18 ( 0.0%)
```
---
 bootstraptest/test_yjit.rb | 36 ++++++++++++++++++++++
 vm_insnhelper.c            |  4 +--
 yjit_codegen.c             | 75 +++++++++++++++++++++++++++++++++-------------
 yjit_iface.h               |  5 ++--
 4 files changed, 96 insertions(+), 24 deletions(-)

diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb
index d02506b2b8..6bf0f53e9e 100644
--- a/bootstraptest/test_yjit.rb
+++ b/bootstraptest/test_yjit.rb
@@ -736,3 +736,39 @@ assert_equal '[nil, nil]', %q{ https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_yjit.rb#L736
 
   [dyn_sym.get_foo, sym.get_foo]
 }
+
+# passing too few arguments to method with optional parameters
+assert_equal 'raised', %q{
+  def opt(a, b = 0)
+  end
+
+  def use
+    opt
+  end
+
+  use rescue nil
+  begin
+    use
+    :ng
+  rescue ArgumentError
+    :raised
+  end
+}
+
+# passing too many arguments to method with optional parameters
+assert_equal 'raised', %q{
+  def opt(a, b = 0)
+  end
+
+  def use
+    opt(1, 2, 3, 4)
+  end
+
+  use rescue nil
+  begin
+    use
+    :ng
+  rescue ArgumentError
+    :raised
+  end
+}
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 00b352df3d..124de73afb 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -2228,7 +2228,7 @@ rb_simple_iseq_p(const rb_iseq_t *iseq) https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L2228
 	   iseq->body->param.flags.has_block == FALSE;
 }
 
-static bool
+bool
 rb_iseq_only_optparam_p(const rb_iseq_t *iseq)
 {
     return iseq->body->param.flags.has_opt == TRUE &&
@@ -2240,7 +2240,7 @@ rb_iseq_only_optparam_p(const rb_iseq_t *iseq) https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L2240
            iseq->body->param.flags.has_block == FALSE;
 }
 
-static bool
+bool
 rb_iseq_only_kwparam_p(const rb_iseq_t *iseq)
 {
     return iseq->body->param.flags.has_opt == FALSE &&
diff --git a/yjit_codegen.c b/yjit_codegen.c
index c545024b16..f02c519612 100644
--- a/yjit_codegen.c
+++ b/yjit_codegen.c
@@ -567,7 +567,8 @@ gen_putself(jitstate_t* jit, ctx_t* ctx) https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L567
 }
 
 // Compute the index of a local variable from its slot index
-uint32_t slot_to_local_idx(const rb_iseq_t *iseq, int32_t slot_idx)
+static uint32_t
+slot_to_local_idx(const rb_iseq_t *iseq, int32_t slot_idx)
 {
     // Convoluted rules from local_var_name() in iseq.c
     int32_t local_table_size = iseq->body->local_table_size;
@@ -633,10 +634,10 @@ gen_setlocal_wc0(jitstate_t* jit, ctx_t* ctx) https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L634
     {
         VALUE flags = ep[VM_ENV_DATA_INDEX_FLAGS];
         if (LIKELY((flags & VM_ENV_FLAG_WB_REQUIRED) == 0)) {
-    	VM_STACK_ENV_WRITE(ep, index, v);
+            VM_STACK_ENV_WRITE(ep, index, v);
         }
         else {
-    	vm_env_write_slowpath(ep, index, v);
+            vm_env_write_slowpath(ep, index, v);
         }
     }
     */
@@ -1772,8 +1773,6 @@ gen_oswb_cfunc(jitstate_t *jit, ctx_t *ctx, const struct rb_callinfo *ci, const https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L1773
     return YJIT_END_BLOCK;
 }
 
-bool rb_simple_iseq_p(const rb_iseq_t *iseq);
-
 static void
 gen_return_branch(codeblock_t* cb, uint8_t* target0, uint8_t* target1, uint8_t shape)
 {
@@ -1791,31 +1790,67 @@ gen_return_branch(codeblock_t* cb, uint8_t* target0, uint8_t* target1, uint8_t s https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L1790
     }
 }
 
+bool rb_simple_iseq_p(const rb_iseq_t *iseq);
+bool rb_iseq_only_optparam_p(const rb_iseq_t *iseq);
+bool rb_iseq_only_kwparam_p(const rb_iseq_t *iseq);
+
 static codegen_status_t
-gen_oswb_iseq(jitstate_t *jit, ctx_t *ctx, const struct rb_callinfo *ci, const rb_callable_method_entry_t *cme, int32_t argc)
+gen_oswb_iseq(jitstate_t *jit, ctx_t *ctx, const struct rb_callinfo *ci, const rb_callable_method_entry_t *cme, const int32_t argc)
 {
     const rb_iseq_t *iseq = def_iseq_ptr(cme->def);
-    const VALUE* start_pc = iseq->body->iseq_encoded;
-    int num_params = iseq->body->param.size;
-    int num_locals = iseq->body->local_table_size - num_params;
 
-    if (num_params != argc) {
-        GEN_COUNTER_INC(cb, oswb_iseq_argc_mismatch);
+    if (vm_ci_flag(ci) & VM_CALL_TAILCALL) {
+        // We can't handle tailcalls
+        GEN_COUNTER_INC(cb, oswb_iseq_tailcall);
         return YJIT_CANT_COMPILE;
     }
 
-    if (!rb_simple_iseq_p(iseq)) {
-        // Only handle iseqs that have simple parameters.
-        // See vm_callee_setup_arg().
-        GEN_COUNTER_INC(cb, oswb_iseq_not_simple);
-        return YJIT_CANT_COMPILE;
+    // Arity handling and optional parameter setup
+    int num_params = iseq->body->param.size;
+    uint32_t start_pc_offset = 0;
+    if (rb_simple_iseq_p(iseq)) {
+        if (num_params != argc) {
+            GEN_COUNTER_INC(cb, oswb_iseq_arity_error);
+            return YJIT_CANT_COMPILE;
+        }
     }
+    else if (rb_iseq_only_optparam_p(iseq)) {
+        // These are iseqs with 0 or more required parameters followed by 1
+        // or more optional parameters.
+        // We follow the logic of vm_call_iseq_setup_normal_opt_start()
+        // and these are the preconditions required for using that fast path.
+        RUBY_ASSERT(vm_ci_markable(ci) && ((vm_ci_flag(ci) &
+                        (VM_CALL_KW_SPLAT | VM_CALL_KWARG | VM_CALL_ARGS_SPLAT)) == 0));
+
+        const int required_num = iseq->body->param.lead_num;
+        const int opts_filled = argc - required_num;
+        const int opt_num = iseq->body->param.opt_num;
+
+        if (opts_filled < 0 || opts_filled > opt_num) {
+            GEN_COUNTER_INC(cb, oswb_iseq_arity_error);
+            return YJIT_CANT_COMPILE;
+        }
 
-    if (vm_ci_flag(ci) & VM_CALL_TAILCALL) {
-        // We can't handle tailcalls
-        GEN_COUNTER_INC(cb, oswb_iseq_tailcall);
+        num_params -= opt_num - opts_filled;
+        start_pc_offset = (uint32_t)iseq->body->param.opt_table[opts_filled];
+    }
+    else if (rb_iseq_only_kwparam_p(iseq)) {
+        // vm_callee_setup_arg() has a fast path for this.
+        GEN_COUNTER_INC(cb, oswb_iseq_only_keywords);
         return YJIT_CANT_COMPILE;
     }
+    else {
+        // Only handle iseqs that have simple parameter setup.
+        // See vm_callee_setup_arg().
+        GEN_COUNTER_INC(cb, oswb_iseq_complex_callee);
+        return YJIT_CANT_COMPILE;
+    }
+
+    // The starting pc of the callee frame
+    const VALUE *start_pc = &iseq->body->iseq_encoded[start_pc_offset];
+
+    // Number of locals that are not parameters
+    const int num_locals = iseq->body->local_table_size - num_params;
 
     // Create a size-exit to fall back to the interpreter
     uint8_t* side_exit = yjit_side_exit(jit, ctx);
@@ -1934,7 +1969,7 @@ gen_oswb_iseq(jitstate_t *jit, ctx_t *ctx, const struct rb_callinfo *ci, const r https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L1969
     gen_direct_jump(
         jit->block,
         &callee_ctx,
-        (blockid_t){ iseq, 0 }
+        (blockid_t){ iseq, start_pc_offset }
     );
 
     return true;
diff --git a/yjit_iface.h b/yjit_iface.h
index eabaff1955..a7a89b7c1f 100644
--- a/yjit_iface.h
+++ b/yjit_iface.h
@@ -38,8 +38,9 @@ YJIT_DECLARE_COUNTERS( https://github.com/ruby/ruby/blob/trunk/yjit_iface.h#L38
     oswb_cfunc_argc_mismatch,
     oswb_cfunc_toomany_args,
     oswb_iseq_tailcall,
-    oswb_iseq_argc_mismatch,
-    oswb_iseq_not_simple,
+    oswb_iseq_arity_error,
+    oswb_iseq_only_keywords,
+    oswb_iseq_complex_callee,
     oswb_not_implemented_method,
     oswb_getter_arity,
     oswb_se_receiver_not_heap,
-- 
cgit v1.2.1


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/