From 2b5bb8a0875b75f18e628c8b2df22760536bdad9 Mon Sep 17 00:00:00 2001
From: ko1 <ko1@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>
Date: Fri, 5 Apr 2019 08:15:11 +0000
Subject: [PATCH] add definemethod/definesmethod insn.

* insns.def: add definemethod and definesmethod (singleton method)
  instructions. Old YARV contains these instructions, but it is moved
  to methods of FrozenCore class because remove number of instructions
  can improve performance for some techniques (static stack caching
  and so on). However, we don't employ these technique and it is hard
  to optimize/analysis definition sequence. So I decide to introduce
  them (and remove definition methods). `putiseq` insn is also removed.

* vm_method.c (rb_scope_visibility_get): renamed to
  `vm_scope_visibility_get()` and make it accept `ec`.
  Same for `vm_scope_module_func_check()`.
  These fixes are result of refactoring `vm_define_method`.

* vm_insnhelper.c (rb_vm_get_cref): renamed to `vm_get_cref`
  because of consistency with other functions.


git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@67442 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
---
 compile.c             | 56 +++++++++++++----------------
 insns.def             | 33 ++++++++++-------
 test/ruby/test_jit.rb | 19 +++-------
 vm.c                  | 57 ++---------------------------
 vm_eval.c             |  2 +-
 vm_insnhelper.c       | 84 +++++++++++++++++++++++++++++++++++++------
 vm_method.c           | 35 +++---------------
 7 files changed, 131 insertions(+), 155 deletions(-)

diff --git a/compile.c b/compile.c
index 4b4b200685..b1b1cdbd09 100644
--- a/compile.c
+++ b/compile.c
@@ -7142,39 +7142,33 @@ iseq_compile_each0(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *node, in
 	break;
       }
       case NODE_DEFN:{
+        ID mid = node->nd_mid;
 	const rb_iseq_t *method_iseq = NEW_ISEQ(node->nd_defn,
-						rb_id2str(node->nd_mid),
+						rb_id2str(mid),
 						ISEQ_TYPE_METHOD, line);
 
 	debugp_param("defn/iseq", rb_iseqw_new(method_iseq));
+        ADD_INSN2(ret, line, definemethod, ID2SYM(mid), method_iseq);
 
-	ADD_INSN1(ret, line, putspecialobject, INT2FIX(VM_SPECIAL_OBJECT_VMCORE));
-	ADD_INSN1(ret, line, putobject, ID2SYM(node->nd_mid));
-	ADD_INSN1(ret, line, putiseq, method_iseq);
-	ADD_SEND (ret, line, id_core_define_method, INT2FIX(2));
-
-	if (popped) {
-	    ADD_INSN(ret, line, pop);
+        if (!popped) {
+	    ADD_INSN1(ret, line, putobject, ID2SYM(mid));
 	}
 
 	break;
       }
       case NODE_DEFS:{
-	const rb_iseq_t * singleton_method = NEW_ISEQ(node->nd_defn,
-						      rb_id2str(node->nd_mid),
-						      ISEQ_TYPE_METHOD, line);
+        ID mid = node->nd_mid;
+	const rb_iseq_t * singleton_method_iseq = NEW_ISEQ(node->nd_defn,
+                                                           rb_id2str(mid),
+                                                           ISEQ_TYPE_METHOD, line);
 
-	debugp_param("defs/iseq", rb_iseqw_new(singleton_method));
+	debugp_param("defs/iseq", rb_iseqw_new(singleton_method_iseq));
+        CHECK(COMPILE(ret, "defs: recv", node->nd_recv));
+        ADD_INSN2(ret, line, definesmethod, ID2SYM(mid), singleton_method_iseq);
 
-	ADD_INSN1(ret, line, putspecialobject, INT2FIX(VM_SPECIAL_OBJECT_VMCORE));
-	CHECK(COMPILE(ret, "defs: recv", node->nd_recv));
-	ADD_INSN1(ret, line, putobject, ID2SYM(node->nd_mid));
-	ADD_INSN1(ret, line, putiseq, singleton_method);
-	ADD_SEND (ret, line, id_core_define_singleton_method, INT2FIX(3));
-
-	if (popped) {
-	    ADD_INSN(ret, line, pop);
-	}
+	if (!popped) {
+            ADD_INSN1(ret, line, putobject, ID2SYM(mid));
+        }
 	break;
       }
       case NODE_ALIAS:{
@@ -8761,7 +8755,7 @@ ibf_load_code(const struct ibf_load *load, const rb_iseq_t *iseq, const struct r
     for (code_index=0; code_index<iseq_size;) {
 	const VALUE insn = code[code_index++];
 	const char *types = insn_op_types(insn);
-	int op_index;
+        int op_index;
 
 	for (op_index=0; types[op_index]; op_index++, code_index++) {
 	    VALUE op = code[code_index];
@@ -8779,15 +8773,15 @@ ibf_load_code(const struct ibf_load *load, const rb_iseq_t *iseq, const struct r
 		    break;
 		}
 	      case TS_ISEQ:
-	        {
-		    VALUE v = (VALUE)ibf_load_iseq(load, (const rb_iseq_t *)op);
-		    code[code_index] = v;
-		    if (!SPECIAL_CONST_P(v)) {
-			RB_OBJ_WRITTEN(iseq, Qundef, v);
-			FL_SET(iseq, ISEQ_MARKABLE_ISEQ);
-		    }
-		    break;
-		}
+                {
+                    VALUE v = (VALUE)ibf_load_iseq(load, (const rb_iseq_t *)op);
+                    code[code_index] = v;
+                    if (!SPECIAL_CONST_P(v)) {
+                        RB_OBJ_WRITTEN(iseq, Qundef, v);
+                        FL_SET(iseq, ISEQ_MARKABLE_ISEQ);
+                    }
+                    break;
+                }
 	      case TS_ISE:
 		FL_SET(iseq, ISEQ_MARKABLE_ISEQ);
 	      case TS_IC:
diff --git a/insns.def b/insns.def
index 96614b2a36..c971026cf6 100644
--- a/insns.def
+++ b/insns.def
@@ -236,7 +236,7 @@ getclassvariable
 /* "class variable access from toplevel" warning can be hooked. */
 // attr bool leaf = false; /* has rb_warning() */
 {
-    val = rb_cvar_get(vm_get_cvar_base(rb_vm_get_cref(GET_EP()), GET_CFP()), id);
+    val = rb_cvar_get(vm_get_cvar_base(vm_get_cref(GET_EP()), GET_CFP()), id);
 }
 
 /* Set value of class variable id of klass as val. */
@@ -249,7 +249,7 @@ setclassvariable
 // attr bool leaf = false; /* has rb_warning() */
 {
     vm_ensure_not_refinement_module(GET_SELF());
-    rb_cvar_set(vm_get_cvar_base(rb_vm_get_cref(GET_EP()), GET_CFP()), id, val);
+    rb_cvar_set(vm_get_cvar_base(vm_get_cref(GET_EP()), GET_CFP()), id, val);
 }
 
 /* Get constant variable id. If klass is Qnil, constants
@@ -358,17 +358,6 @@ putspecialobject
     val = vm_get_special_object(GET_EP(), type);
 }
 
-/* put iseq value. */
-DEFINE_INSN
-putiseq
-(ISEQ iseq)
-()
-(VALUE ret)
-// attr bool handles_sp = false; /* of course it doesn't */
-{
-    ret = (VALUE)iseq;
-}
-
 /* put string val. string will be copied. */
 DEFINE_INSN
 putstring
@@ -748,6 +737,24 @@ defineclass
     NEXT_INSN();
 }
 
+DEFINE_INSN
+definemethod
+(ID id, ISEQ iseq)
+()
+()
+{
+    vm_define_method(ec, Qnil, id, (VALUE)iseq, FALSE);
+}
+
+DEFINE_INSN
+definesmethod
+(ID id, ISEQ iseq)
+(VALUE obj)
+()
+{
+    vm_define_method(ec, obj, id, (VALUE)iseq, TRUE);
+}
+
 /**********************************************************/
 /* deal with control flow 2: method/iterator              */
 /**********************************************************/
diff --git a/test/ruby/test_jit.rb b/test/ruby/test_jit.rb
index 7adb1e8d6d..332d6fc2ed 100644
--- a/test/ruby/test_jit.rb
+++ b/test/ruby/test_jit.rb
@@ -183,18 +183,6 @@ class TestJIT < Test::Unit::TestCase
     assert_compile_once('2', result_inspect: '2', insns: %i[putobject])
   end
 
-  def test_compile_insn_putspecialobject_putiseq
-    assert_eval_with_jit("#{<<~"begin;"}\n#{<<~"end;"}", stdout: 'hellohello', success_count: 2, insns: %i[putspecialobject putiseq])
-    begin;
-      print 2.times.map {
-        def method_definition
-          'hello'
-        end
-        method_definition
-      }.join
-    end;
-  end
-
   def test_compile_insn_putstring_concatstrings_tostring
     assert_compile_once('"a#{}b" + "c"', result_inspect: '"abc"', insns: %i[putstring concatstrings tostring])
   end
@@ -952,8 +940,10 @@ class TestJIT < Test::Unit::TestCase
     insns = []
     RubyVM::InstructionSequence.compile(script).to_a.last.each do |(insn, *args)|
       case insn
-      when :putiseq, :send
+      when :send
         insns += collect_insns(args.last)
+      when :definemethod, :definesmethod
+        insns += collect_insns(args[1])
       when :defineclass
         insns += collect_insns(args[1])
       end
@@ -968,7 +958,8 @@ class TestJIT < Test::Unit::TestCase
     insns = iseq_array.last.select { |x| x.is_a?(Array) }.map(&:first)
     iseq_array.last.each do |(insn, *args)|
       case insn
-      when :putiseq, :send
+      when :definemethod, :definesmethod,
+           :send
         insns += collect_insns(args.last)
       end
     end
diff --git a/vm.c b/vm.c
index bbda6ee4c0..142573929a 100644
--- a/vm.c
+++ b/vm.c
@@ -1368,12 +1368,7 @@ rb_cref_t *
 rb_vm_cref(void)
 {
     const rb_execution_context_t *ec = GET_EC();
-    const rb_control_frame_t *cfp = rb_vm_get_ruby_level_next_cfp(ec, ec->cfp);
-
-    if (cfp == NULL) {
-	return NULL;
-    }
-    return rb_vm_get_cref(cfp->ep);
+    return vm_ec_cref(ec);
 }
 
 rb_cref_t *
@@ -1393,7 +1388,7 @@ rb_vm_cref_in_context(VALUE self, VALUE cbase)
     const rb_cref_t *cref;
     if (cfp->self != self) return NULL;
     if (!vm_env_cref_by_cref(cfp->ep)) return NULL;
-    cref = rb_vm_get_cref(cfp->ep);
+    cref = vm_get_cref(cfp->ep);
     if (CREF_CLASS(cref) != cbase) return NULL;
     return cref;
 }
@@ -2678,34 +2673,6 @@ rb_thread_alloc(VALUE klass)
     return self;
 }
 
-static void
-vm_define_method(VALUE obj, ID id, VALUE iseqval, int is_singleton)
-{
-    VALUE klass;
-    rb_method_visibility_t visi;
-    rb_cref_t *cref = rb_vm_cref();
-
-    if (!is_singleton) {
-	klass = CREF_CLASS(cref);
-	visi = rb_scope_visibility_get();
-    }
-    else { /* singleton */
-	klass = rb_singleton_class(obj); /* class and frozen checked in this API */
-	visi = METHOD_VISI_PUBLIC;
-    }
-
-    if (NIL_P(klass)) {
-	rb_raise(rb_eTypeError, "no class/module to add method");
-    }
-
-    rb_add_method_iseq(klass, id, (const rb_iseq_t *)iseqval, cref, visi);
-
-    if (!is_singleton && rb_scope_module_func_check()) {
-	klass = rb_singleton_class(klass);
-	rb_add_method_iseq(klass, id, (const rb_iseq_t *)iseqval, cref, METHOD_VISI_PUBLIC);
-    }
-}
-
 #define REWIND_CFP(expr) do { \
     rb_execution_context_t *ec__ = GET_EC(); \
     VALUE *const curr_sp = (ec__->cfp++)->sp; \
@@ -2715,24 +2682,6 @@ vm_define_method(VALUE obj, ID id, VALUE iseqval, int is_singleton)
     (ec__->cfp--)->sp = saved_sp; \
 } while (0)
 
-static VALUE
-m_core_define_method(VALUE self, VALUE sym, VALUE iseqval)
-{
-    REWIND_CFP({
-	vm_define_method(Qnil, SYM2ID(sym), iseqval, FALSE);
-    });
-    return sym;
-}
-
-static VALUE
-m_core_define_singleton_method(VALUE self, VALUE cbase, VALUE sym, VALUE iseqval)
-{
-    REWIND_CFP({
-	vm_define_method(cbase, SYM2ID(sym), iseqval, TRUE);
-    });
-    return sym;
-}
-
 static VALUE
 m_core_set_method_alias(VALUE self, VALUE cbase, VALUE sym1, VALUE sym2)
 {
@@ -2931,8 +2880,6 @@ Init_VM(void)
     rb_define_method_id(klass, id_core_set_method_alias, m_core_set_method_alias, 3);
     rb_define_method_id(klass, id_core_set_variable_alias, m_core_set_variable_alias, 2);
     rb_define_method_id(klass, id_core_undef_method, m_core_undef_method, 2);
-    rb_define_method_id(klass, id_core_define_method, m_core_define_method, 2);
-    rb_define_method_id(klass, id_core_define_singleton_method, m_core_define_singleton_method, 3);
     rb_define_method_id(klass, id_core_set_postexe, m_core_set_postexe, 0);
     rb_define_method_id(klass, id_core_hash_merge_ptr, m_core_hash_merge_ptr, -1);
     rb_define_method_id(klass, id_core_hash_merge_kwd, m_core_hash_merge_kwd, 2);
diff --git a/vm_eval.c b/vm_eval.c
index a4d5835720..55cd3ca3fc 100644
--- a/vm_eval.c
+++ b/vm_eval.c
@@ -1322,7 +1322,7 @@ eval_string_with_cref(VALUE self, VALUE src, rb_cref_t *cref, VALUE file, int li
 
     /* TODO: what the code checking? */
     if (!cref && block.as.captured.code.val) {
-	rb_cref_t *orig_cref = rb_vm_get_cref(vm_block_ep(&block));
+	rb_cref_t *orig_cref = vm_get_cref(vm_block_ep(&block));
 	cref = vm_cref_dup(orig_cref);
     }
     vm_set_eval_stack(ec, iseq, cref, &block);
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index f509ca9236..662e827c15 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -764,9 +764,8 @@ vm_cref_replace_with_duplicated_cref(const VALUE *ep)
     }
 }
 
-
 static rb_cref_t *
-rb_vm_get_cref(const VALUE *ep)
+vm_get_cref(const VALUE *ep)
 {
     rb_cref_t *cref = vm_env_cref(ep);
 
@@ -774,14 +773,25 @@ rb_vm_get_cref(const VALUE *ep)
 	return cref;
     }
     else {
-	rb_bug("rb_vm_get_cref: unreachable");
+	rb_bug("vm_get_cref: unreachable");
     }
 }
 
+static rb_cref_t *
+vm_ec_cref(const rb_execution_context_t *ec)
+{
+    const rb_control_frame_t *cfp = rb_vm_get_ruby_level_next_cfp(ec, ec->cfp);
+
+    if (cfp == NULL) {
+        return NULL;
+    }
+    return vm_get_cref(cfp->ep);
+}
+
 static const rb_cref_t *
 vm_get_const_key_cref(const VALUE *ep)
 {
-    const rb_cref_t *cref = rb_vm_get_cref(ep);
+    const rb_cref_t *cref = vm_get_cref(ep);
     const rb_cref_t *key_cref = cref;
 
     while (cref) {
@@ -836,7 +846,7 @@ vm_cref_push(const rb_execution_context_t *ec, VALUE klass, const VALUE *ep, int
 static inline VALUE
 vm_get_cbase(const VALUE *ep)
 {
-    const rb_cref_t *cref = rb_vm_get_cref(ep);
+    const rb_cref_t *cref = vm_get_cref(ep);
     VALUE klass = Qundef;
 
     while (cref) {
@@ -852,7 +862,7 @@ vm_get_cbase(const VALUE *ep)
 static inline VALUE
 vm_get_const_base(const VALUE *ep)
 {
-    const rb_cref_t *cref = rb_vm_get_cref(ep);
+    const rb_cref_t *cref = vm_get_cref(ep);
     VALUE klass = Qundef;
 
     while (cref) {
@@ -896,7 +906,7 @@ vm_get_ev_const(rb_execution_context_t *ec, VALUE orig_klass, ID id, int is_defi
 
     if (orig_klass == Qnil) {
 	/* in current lexical scope */
-	const rb_cref_t *root_cref = rb_vm_get_cref(ec->cfp->ep);
+	const rb_cref_t *root_cref = vm_get_cref(ec->cfp->ep);
 	const rb_cref_t *cref;
 	VALUE klass = Qnil;
 
@@ -2600,7 +2610,7 @@ vm_call_method_each_type(rb_execution_context_t *ec, rb_control_frame_t *cfp, st
         return vm_call_zsuper(ec, cfp, calling, ci, cc, RCLASS_ORIGIN(cc->me->defined_class));
 
       case VM_METHOD_TYPE_REFINED: {
-	const rb_cref_t *cref = rb_vm_get_cref(cfp->ep);
+	const rb_cref_t *cref = vm_get_cref(cfp->ep);
 	VALUE refinements = cref ? CREF_REFINEMENTS(cref) : Qnil;
 	VALUE refinement;
 	const rb_callable_method_entry_t *ref_me;
@@ -3131,7 +3141,7 @@ vm_defined(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, rb_num_t op_
 	}
 	break;
       case DEFINED_CVAR: {
-	const rb_cref_t *cref = rb_vm_get_cref(GET_EP());
+	const rb_cref_t *cref = vm_get_cref(GET_EP());
 	klass = vm_get_cvar_base(cref, GET_CFP());
 	if (rb_cvar_defined(klass, SYM2ID(obj))) {
 	    expr_type = DEFINED_CVAR;
@@ -3494,6 +3504,60 @@ vm_find_or_create_class_by_id(ID id,
     }
 }
 
+static rb_method_visibility_t
+vm_scope_visibility_get(const rb_execution_context_t *ec)
+{
+    const rb_control_frame_t *cfp = rb_vm_get_ruby_level_next_cfp(ec, ec->cfp);
+
+    if (!vm_env_cref_by_cref(cfp->ep)) {
+	return METHOD_VISI_PUBLIC;
+    }
+    else {
+	return CREF_SCOPE_VISI(vm_ec_cref(ec))->method_visi;
+    }
+}
+
+static int
+vm_scope_module_func_check(const rb_execution_context_t *ec)
+{
+    const rb_control_frame_t *cfp = rb_vm_get_ruby_level_next_cfp(ec, ec->cfp);
+
+    if (!vm_env_cref_by_cref(cfp->ep)) {
+	return FALSE;
+    }
+    else {
+	return CREF_SCOPE_VISI(vm_ec_cref(ec))->module_func;
+    }
+}
+
+static void
+vm_define_method(const rb_execution_context_t *ec, VALUE obj, ID id, VALUE iseqval, int is_singleton)
+{
+    VALUE klass;
+    rb_method_visibility_t visi;
+    rb_cref_t *cref = vm_ec_cref(ec);
+
+    if (!is_singleton) {
+	klass = CREF_CLASS(cref);
+	visi = vm_scope_visibility_get(ec);
+    }
+    else { /* singleton */
+	klass = rb_singleton_class(obj); /* class and frozen checked in this API */
+	visi = METHOD_VISI_PUBLIC;
+    }
+
+    if (NIL_P(klass)) {
+	rb_raise(rb_eTypeError, "no class/module to add method");
+    }
+
+    rb_add_method_iseq(klass, id, (const rb_iseq_t *)iseqval, cref, visi);
+
+    if (!is_singleton && vm_scope_module_func_check(ec)) {
+	klass = rb_singleton_class(klass);
+	rb_add_method_iseq(klass, id, (const rb_iseq_t *)iseqval, cref, METHOD_VISI_PUBLIC);
+    }
+}
+
 static void
 vm_search_method_wrap(
     const struct rb_control_frame_struct *reg_cfp,
@@ -3663,7 +3727,7 @@ static int
 vm_ic_hit_p(IC ic, const VALUE *reg_ep)
 {
     if (ic->ic_serial == GET_GLOBAL_CONSTANT_STATE()) {
-	return (ic->ic_cref == NULL || ic->ic_cref == rb_vm_get_cref(reg_ep));
+	return (ic->ic_cref == NULL || ic->ic_cref == vm_get_cref(reg_ep));
     }
     return FALSE;
 }
diff --git a/vm_method.c b/vm_method.c
index 9d4e25e05b..3a9dfc1c5d 100644
--- a/vm_method.c
+++ b/vm_method.c
@@ -669,7 +669,7 @@ rb_add_method(VALUE klass, ID mid, rb_method_type_t type, void *opts, rb_method_
     return me;
 }
 
-void
+MJIT_FUNC_EXPORTED void
 rb_add_method_iseq(VALUE klass, ID mid, const rb_iseq_t *iseq, rb_cref_t *cref, rb_method_visibility_t visi)
 {
     struct { /* should be same fields with rb_method_iseq_struct */
@@ -1117,34 +1117,6 @@ rb_method_boundp(VALUE klass, ID id, int ex)
     return 0;
 }
 
-static rb_method_visibility_t
-rb_scope_visibility_get(void)
-{
-    const rb_execution_context_t *ec = GET_EC();
-    const rb_control_frame_t *cfp = rb_vm_get_ruby_level_next_cfp(ec, ec->cfp);
-
-    if (!vm_env_cref_by_cref(cfp->ep)) {
-	return METHOD_VISI_PUBLIC;
-    }
-    else {
-	return CREF_SCOPE_VISI(rb_vm_cref())->method_visi;
-    }
-}
-
-static int
-rb_scope_module_func_check(void)
-{
-    const rb_execution_context_t *ec = GET_EC();
-    const rb_control_frame_t *cfp = rb_vm_get_ruby_level_next_cfp(ec, ec->cfp);
-
-    if (!vm_env_cref_by_cref(cfp->ep)) {
-	return FALSE;
-    }
-    else {
-	return CREF_SCOPE_VISI(rb_vm_cref())->module_func;
-    }
-}
-
 static void
 vm_cref_set_visibility(rb_method_visibility_t method_visi, int module_func)
 {
@@ -1170,14 +1142,15 @@ rb_attr(VALUE klass, ID id, int read, int write, int ex)
 {
     ID attriv;
     rb_method_visibility_t visi;
+    const rb_execution_context_t *ec = GET_EC();
 
     if (!ex) {
 	visi = METHOD_VISI_PUBLIC;
     }
     else {
-	switch (rb_scope_visibility_get()) {
+	switch (vm_scope_visibility_get(ec)) {
 	  case METHOD_VISI_PRIVATE:
-	    if (rb_scope_module_func_check()) {
+	    if (vm_scope_module_func_check(ec)) {
 		rb_warning("attribute accessor as module_function");
 	    }
 	    visi = METHOD_VISI_PRIVATE;