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/