[前][次][番号順一覧][スレッド一覧]

ruby-changes:52527

From: shyouhei <ko1@a...>
Date: Fri, 14 Sep 2018 16:44:53 +0900 (JST)
Subject: [ruby-changes:52527] shyouhei:r64737 (trunk): move ADD_PC around (take 2)

shyouhei	2018-09-14 16:44:44 +0900 (Fri, 14 Sep 2018)

  New Revision: 64737

  https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=64737

  Log:
    move ADD_PC around (take 2)
    
    Now that we can say for sure if an instruction calls a method or
    not internally, it is now possible to reroute the bugs that
    forced us to revert the "move PC around" optimization.
    
    First try: r62051
    Reverted:  r63763
    See also:  r63999
    
    ----
    
    trunk: ruby 2.6.0dev (2018-09-13 trunk 64736) [x86_64-darwin15]
    ours: ruby 2.6.0dev (2018-09-13 trunk 64736) [x86_64-darwin15]
    last_commit=move ADD_PC around (take 2)
    Calculating -------------------------------------
                               trunk        ours
             so_ackermann      1.884       2.278 i/s -       1.000 times in 0.530926s 0.438935s
                 so_array      1.178       1.157 i/s -       1.000 times in 0.848786s 0.864467s
          so_binary_trees      0.176       0.177 i/s -       1.000 times in 5.683895s 5.657707s
           so_concatenate      0.220       0.221 i/s -       1.000 times in 4.546896s 4.518949s
           so_count_words      6.729       6.470 i/s -       1.000 times in 0.148602s 0.154561s
             so_exception      3.324       3.688 i/s -       1.000 times in 0.300872s 0.271147s
              so_fannkuch      0.546       0.968 i/s -       1.000 times in 1.831328s 1.033376s
                 so_fasta      0.541       0.547 i/s -       1.000 times in 1.849923s 1.827091s
          so_k_nucleotide      0.800       0.777 i/s -       1.000 times in 1.250635s 1.286295s
                 so_lists      2.101       1.848 i/s -       1.000 times in 0.475954s 0.541095s
            so_mandelbrot      0.435       0.408 i/s -       1.000 times in 2.299328s 2.450535s
                so_matrix      1.946       1.912 i/s -       1.000 times in 0.513872s 0.523076s
        so_meteor_contest      0.311       0.317 i/s -       1.000 times in 3.219297s 3.152052s
                 so_nbody      0.746       0.703 i/s -       1.000 times in 1.339815s 1.423441s
           so_nested_loop      0.899       0.901 i/s -       1.000 times in 1.111767s 1.109555s
                so_nsieve      0.559       0.579 i/s -       1.000 times in 1.787763s 1.726552s
           so_nsieve_bits      0.435       0.428 i/s -       1.000 times in 2.296282s 2.333852s
                so_object      1.368       1.442 i/s -       1.000 times in 0.731237s 0.693684s
          so_partial_sums      0.616       0.546 i/s -       1.000 times in 1.623592s 1.833097s
              so_pidigits      0.831       0.832 i/s -       1.000 times in 1.203117s 1.202334s
                so_random      2.934       2.724 i/s -       1.000 times in 0.340791s 0.367150s
    so_reverse_complement      0.583       0.866 i/s -       1.000 times in 1.714144s 1.154615s
                 so_sieve      1.829       2.081 i/s -       1.000 times in 0.546607s 0.480562s
          so_spectralnorm      0.524       0.558 i/s -       1.000 times in 1.908716s 1.792382s

  Modified files:
    trunk/gc.c
    trunk/insns.def
    trunk/tool/ruby_vm/views/_insn_entry.erb
    trunk/tool/ruby_vm/views/_mjit_compile_insn_body.erb
    trunk/vm_insnhelper.h
Index: gc.c
===================================================================
--- gc.c	(revision 64736)
+++ gc.c	(revision 64737)
@@ -1807,7 +1807,10 @@ rb_objspace_set_event_hook(const rb_even https://github.com/ruby/ruby/blob/trunk/gc.c#L1807
 static void
 gc_event_hook_body(rb_execution_context_t *ec, rb_objspace_t *objspace, const rb_event_flag_t event, VALUE data)
 {
+    /* increment PC because source line is calculated with PC-1 */
+    const VALUE *pc = ec->cfp->pc++;
     EXEC_EVENT_HOOK(ec, event, ec->cfp->self, 0, 0, 0, data);
+    ec->cfp->pc = pc;
 }
 
 #define gc_event_hook_available_p(objspace) ((objspace)->flags.has_hook)
Index: vm_insnhelper.h
===================================================================
--- vm_insnhelper.h	(revision 64736)
+++ vm_insnhelper.h	(revision 64737)
@@ -112,7 +112,6 @@ enum vm_regan_acttype { https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.h#L112
 #define DEC_SP(x)  (VM_REG_SP -= (COLLECT_USAGE_REGISTER_HELPER(SP, SET, (x))))
 #define SET_SV(x)  (*GET_SP() = (x))
   /* set current stack value as x */
-#define ADJ_SP(x)  INC_SP(x)
 
 /* instruction sequence C struct */
 #define GET_ISEQ() (GET_CFP()->iseq)
@@ -184,9 +183,6 @@ enum vm_regan_acttype { https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.h#L183
 
 #define GET_BLOCK_HANDLER() (GET_LEP()[VM_ENV_DATA_INDEX_SPECVAL])
 
-#define WIDTH_OF_opt_send_without_block \
-    ((rb_snum_t)attr_width_opt_send_without_block(0, 0))
-
 /**********************************************************/
 /* deal with control flow 3: exception                    */
 /**********************************************************/
@@ -197,9 +193,8 @@ enum vm_regan_acttype { https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.h#L193
 /**********************************************************/
 
 #if VM_CHECK_MODE > 0
-#define DECLARE_CANARY bool leaf; VALUE *canary
 #define SETUP_CANARY() \
-    if ((leaf = INSN_ATTR(leaf))) { \
+    if (leaf) { \
         canary = GET_SP(); \
         SET_SV(vm_stack_canary); \
     }
@@ -208,7 +203,6 @@ enum vm_regan_acttype { https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.h#L203
         vm_canary_is_found_dead(INSN_ATTR(bin), *canary); \
     }
 #else
-#define DECLARE_CANARY          /* void */
 #define SETUP_CANARY()          /* void */
 #define CHECK_CANARY()          /* void */
 #endif
@@ -231,6 +225,16 @@ enum vm_regan_acttype { https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.h#L225
 #define USE_IC_FOR_SPECIALIZED_METHOD 1
 #endif
 
+#ifndef MJIT_HEADER
+#define CALL_SIMPLE_METHOD() do { \
+    rb_snum_t x = leaf ? INSN_ATTR(width) : 0; \
+    rb_snum_t y = attr_width_opt_send_without_block(0, 0); \
+    rb_snum_t z = x - y; \
+    ADD_PC(z); \
+    DISPATCH_ORIGINAL_INSN(opt_send_without_block); \
+} while (0)
+#endif
+
 #define NEXT_CLASS_SERIAL() (++ruby_vm_class_serial)
 #define GET_GLOBAL_METHOD_STATE() (ruby_vm_global_method_state)
 #define INC_GLOBAL_METHOD_STATE() (++ruby_vm_global_method_state)
Index: tool/ruby_vm/views/_insn_entry.erb
===================================================================
--- tool/ruby_vm/views/_insn_entry.erb	(revision 64736)
+++ tool/ruby_vm/views/_insn_entry.erb	(revision 64737)
@@ -13,11 +13,12 @@ INSN_ENTRY(<%= insn.name %>) https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/_insn_entry.erb#L13
 %#  NAME_OF_CURRENT_INSN is used in vm_exec.h
 #   define NAME_OF_CURRENT_INSN <%= insn.name %>
 #   define INSN_ATTR(x) <%= insn.call_attribute(' ## x ## ') %>
+    bool leaf;
+    MAYBE_UNUSED(VALUE *) canary;
 % unless insn.declarations.empty?
     <%= insn.declarations.join(";\n    ") %>;
 
 % end
-<%= insn.handle_canary "DECLARE_CANARY" -%>
     START_OF_ORIGINAL_INSN(<%= insn.name %>);
 % insn.preamble.each do |konst|
 <%= render_c_expr konst -%>
@@ -30,7 +31,7 @@ INSN_ENTRY(<%= insn.name %>) https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/_insn_entry.erb#L31
     <%= pop[:name] %> = <%= insn.cast_from_VALUE pop, "TOPN(#{i})"%>;
 % end
     DEBUG_ENTER_INSN(INSN_ATTR(name));
-    ADD_PC(INSN_ATTR(width));
+    if (! (leaf = INSN_ATTR(leaf))) ADD_PC(INSN_ATTR(width));
 % if insn.handles_sp?
     POPN(INSN_ATTR(popn));
 % end
@@ -47,11 +48,12 @@ INSN_ENTRY(<%= insn.name %>) https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/_insn_entry.erb#L48
     PUSH(<%= insn.cast_to_VALUE ret %>);
 %   end
 % else
-    ADJ_SP(INSN_ATTR(sp_inc));
+    INC_SP(INSN_ATTR(sp_inc));
 %   insn.rets.reverse_each.with_index do |ret, i|
     TOPN(<%= i %>) = <%= insn.cast_to_VALUE ret %>;
 %   end
 % end
+    if (leaf) ADD_PC(INSN_ATTR(width));
     END_INSN(<%= insn.name %>);
 #   undef INSN_ATTR
 #   undef NAME_OF_CURRENT_INSN
Index: tool/ruby_vm/views/_mjit_compile_insn_body.erb
===================================================================
--- tool/ruby_vm/views/_mjit_compile_insn_body.erb	(revision 64736)
+++ tool/ruby_vm/views/_mjit_compile_insn_body.erb	(revision 64737)
@@ -67,7 +67,7 @@ https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/_mjit_compile_insn_body.erb#L67
         next_pos = pos + insn_len(insn) + (unsigned int)<%= dest %>;
         fprintf(f, "            goto label_%d;\n", next_pos);
 %     end
-%   when /\A\s+DISPATCH_ORIGINAL_INSN\([^)]+\);\s+\z/
+%   when /\A\s+CALL_SIMPLE_METHOD\(\);\s+\z/
 %     # For `opt_xxx`'s fallbacks.
         if (status->local_stack_p) {
             fprintf(f, "            reg_cfp->sp = (VALUE *)reg_cfp->bp + %d;\n", b->stack_size + 1);
Index: insns.def
===================================================================
--- insns.def	(revision 64736)
+++ insns.def	(revision 64737)
@@ -761,10 +761,7 @@ opt_str_freeze https://github.com/ruby/ruby/blob/trunk/insns.def#L761
 
     if (val == Qundef) {
         PUSH(rb_str_resurrect(str));
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-        DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -778,10 +775,7 @@ opt_str_uminus https://github.com/ruby/ruby/blob/trunk/insns.def#L775
 
     if (val == Qundef) {
         PUSH(rb_str_resurrect(str));
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-        DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1058,10 +1052,7 @@ opt_plus https://github.com/ruby/ruby/blob/trunk/insns.def#L1052
     val = vm_opt_plus(recv, obj);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+	CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1075,10 +1066,7 @@ opt_minus https://github.com/ruby/ruby/blob/trunk/insns.def#L1066
     val = vm_opt_minus(recv, obj);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1092,10 +1080,7 @@ opt_mult https://github.com/ruby/ruby/blob/trunk/insns.def#L1080
     val = vm_opt_mult(recv, obj);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1109,10 +1094,7 @@ opt_div https://github.com/ruby/ruby/blob/trunk/insns.def#L1094
     val = vm_opt_div(recv, obj);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1126,10 +1108,7 @@ opt_mod https://github.com/ruby/ruby/blob/trunk/insns.def#L1108
     val = vm_opt_mod(recv, obj);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1147,10 +1126,7 @@ opt_eq https://github.com/ruby/ruby/blob/trunk/insns.def#L1126
     val = opt_eq_func(recv, obj, ci, cc);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1166,10 +1142,7 @@ opt_neq https://github.com/ruby/ruby/blob/trunk/insns.def#L1142
     val = vm_opt_neq(ci, cc, ci_eq, cc_eq, recv, obj);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1183,10 +1156,7 @@ opt_lt https://github.com/ruby/ruby/blob/trunk/insns.def#L1156
     val = vm_opt_lt(recv, obj);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1200,10 +1170,7 @@ opt_le https://github.com/ruby/ruby/blob/trunk/insns.def#L1170
     val = vm_opt_le(recv, obj);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1217,10 +1184,7 @@ opt_gt https://github.com/ruby/ruby/blob/trunk/insns.def#L1184
     val = vm_opt_gt(recv, obj);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1234,10 +1198,7 @@ opt_ge https://github.com/ruby/ruby/blob/trunk/insns.def#L1198
     val = vm_opt_ge(recv, obj);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1251,10 +1212,7 @@ opt_ltlt https://github.com/ruby/ruby/blob/trunk/insns.def#L1212
     val = vm_opt_ltlt(recv, obj);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1273,10 +1231,7 @@ opt_aref https://github.com/ruby/ruby/blob/trunk/insns.def#L1231
     val = vm_opt_aref(recv, obj);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1293,10 +1248,7 @@ opt_aset https://github.com/ruby/ruby/blob/trunk/insns.def#L1248
     val = vm_opt_aset(recv, obj, set);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1318,9 +1270,8 @@ opt_aset_with https://github.com/ruby/ruby/blob/trunk/insns.def#L1270
 #ifndef MJIT_HEADER
 	TOPN(0) = rb_str_resurrect(key);
 	PUSH(val);
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
 #endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1338,9 +1289,8 @@ opt_aref_with https://github.com/ruby/ruby/blob/trunk/insns.def#L1289
     if (val == Qundef) {
 #ifndef MJIT_HEADER
 	PUSH(rb_str_resurrect(key));
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
 #endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+	CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1354,10 +1304,7 @@ opt_length https://github.com/ruby/ruby/blob/trunk/insns.def#L1304
     val = vm_opt_length(recv, BOP_LENGTH);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1371,10 +1318,7 @@ opt_size https://github.com/ruby/ruby/blob/trunk/insns.def#L1318
     val = vm_opt_length(recv, BOP_SIZE);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1388,10 +1332,7 @@ opt_empty_p https://github.com/ruby/ruby/blob/trunk/insns.def#L1332
     val = vm_opt_empty_p(recv);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1405,10 +1346,7 @@ opt_succ https://github.com/ruby/ruby/blob/trunk/insns.def#L1346
     val = vm_opt_succ(recv);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1422,10 +1360,7 @@ opt_not https://github.com/ruby/ruby/blob/trunk/insns.def#L1360
     val = vm_opt_not(ci, cc, recv);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 
@@ -1450,10 +1385,7 @@ opt_regexpmatch2 https://github.com/ruby/ruby/blob/trunk/insns.def#L1385
     val = vm_opt_regexpmatch2(obj2, obj1);
 
     if (val == Qundef) {
-#ifndef MJIT_HEADER
-        ADD_PC(-WIDTH_OF_opt_send_without_block);
-#endif
-	DISPATCH_ORIGINAL_INSN(opt_send_without_block);
+        CALL_SIMPLE_METHOD();
     }
 }
 

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

[前][次][番号順一覧][スレッド一覧]