ruby-changes:55137
From: k0kubun <ko1@a...>
Date: Mon, 25 Mar 2019 23:26:17 +0900 (JST)
Subject: [ruby-changes:55137] k0kubun:r67344 (trunk): Prefer using vm_base_ptr rather than cfp->bp
k0kubun 2019-03-25 23:26:11 +0900 (Mon, 25 Mar 2019) New Revision: 67344 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=67344 Log: Prefer using vm_base_ptr rather than cfp->bp in MJIT implementation. This allows us to drop cfp->bp by just modifying vm_base_ptr in the future. No performance impact: $ benchmark-driver benchmark.yml --rbenv='before::before --disable-gems --jit;bp_::after --disable-gems --jit;vm_env_ptr::ruby-svn --disable-gems --jit' -v --output=all --repeat-count=12 before: ruby 2.7.0dev (2019-03-24 trunk 67341) +JIT [x86_64-linux] bp_: ruby 2.7.0dev (2019-03-24 trunk 67342) +JIT [x86_64-linux] vm_env_ptr: ruby 2.7.0dev (2019-03-25 trunk 67343) +JIT [x86_64-linux] last_commit=Prefer using vm_base_ptr rather than cfp->bp Calculating ------------------------------------- before bp_ vm_env_ptr Optcarrot Lan_Master.nes 77.15059205092646 70.18873044267853 69.62171387083328 fps 78.75767783870441 77.49867689173411 75.43496867709587 79.60102690369321 77.78037687683523 79.36688927929428 80.25144236638835 78.74729849101701 80.42363742291455 82.22375417165489 80.44265482494045 80.90287243299306 82.29166786292619 80.51740049420938 81.81153053252902 83.35386925305345 80.91054205210609 81.93562989125176 83.39770634366975 81.34550754145043 82.24544621470430 83.88523450309972 81.60698516017347 82.76801860263230 84.17553130135879 82.69615943446324 83.02530407910871 84.42132328119858 83.00969158037691 83.19968539409922 84.60731429793329 83.32703363300098 83.81352746019631 Modified files: trunk/mjit_compile.c trunk/tool/ruby_vm/views/_mjit_compile_insn.erb trunk/tool/ruby_vm/views/_mjit_compile_insn_body.erb trunk/tool/ruby_vm/views/_mjit_compile_ivar.erb trunk/tool/ruby_vm/views/_mjit_compile_pc_and_sp.erb trunk/tool/ruby_vm/views/_mjit_compile_send.erb trunk/vm_core.h trunk/vm_insnhelper.c Index: tool/ruby_vm/views/_mjit_compile_insn_body.erb =================================================================== --- tool/ruby_vm/views/_mjit_compile_insn_body.erb (revision 67343) +++ tool/ruby_vm/views/_mjit_compile_insn_body.erb (revision 67344) @@ -76,7 +76,7 @@ https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/_mjit_compile_insn_body.erb#L76 % 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); + fprintf(f, " reg_cfp->sp = vm_base_ptr(reg_cfp) + %d;\n", b->stack_size); } fprintf(f, " reg_cfp->pc = original_body_iseq + %d;\n", pos); fprintf(f, " goto cancel;\n"); Index: tool/ruby_vm/views/_mjit_compile_pc_and_sp.erb =================================================================== --- tool/ruby_vm/views/_mjit_compile_pc_and_sp.erb (revision 67343) +++ tool/ruby_vm/views/_mjit_compile_pc_and_sp.erb (revision 67344) @@ -20,7 +20,7 @@ https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/_mjit_compile_pc_and_sp.erb#L20 { rb_snum_t i, push_size; push_size = -<%= insn.call_attribute('sp_inc') %> + <%= insn.rets.size %> - <%= insn.pops.size %>; - fprintf(f, " reg_cfp->sp = (VALUE *)reg_cfp->bp_ + %ld;\n", push_size); /* POPN(INSN_ATTR(popn)); */ + fprintf(f, " reg_cfp->sp = vm_base_ptr(reg_cfp) + %ld;\n", push_size); /* POPN(INSN_ATTR(popn)); */ for (i = 0; i < push_size; i++) { fprintf(f, " *(reg_cfp->sp + %ld) = stack[%ld];\n", i - push_size, (rb_snum_t)b->stack_size - push_size + i); } @@ -29,8 +29,8 @@ https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/_mjit_compile_pc_and_sp.erb#L29 } else { % if insn.handles_sp? - fprintf(f, " reg_cfp->sp = (VALUE *)reg_cfp->bp_ + %d;\n", b->stack_size - <%= insn.pops.size %>); /* POPN(INSN_ATTR(popn)); */ + fprintf(f, " reg_cfp->sp = vm_base_ptr(reg_cfp) + %d;\n", b->stack_size - <%= insn.pops.size %>); /* POPN(INSN_ATTR(popn)); */ % else - fprintf(f, " reg_cfp->sp = (VALUE *)reg_cfp->bp_ + %d;\n", b->stack_size); + fprintf(f, " reg_cfp->sp = vm_base_ptr(reg_cfp) + %d;\n", b->stack_size); % end } Index: tool/ruby_vm/views/_mjit_compile_send.erb =================================================================== --- tool/ruby_vm/views/_mjit_compile_send.erb (revision 67343) +++ tool/ruby_vm/views/_mjit_compile_send.erb (revision 67344) @@ -37,7 +37,7 @@ https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/_mjit_compile_send.erb#L37 fprintf(f, " if (UNLIKELY(GET_GLOBAL_METHOD_STATE() != %"PRI_SERIALT_PREFIX"u ||\n", cc_copy->method_state); fprintf(f, " RCLASS_SERIAL(CLASS_OF(stack[%d])) != %"PRI_SERIALT_PREFIX"u)) {\n", b->stack_size - 1 - argc, cc_copy->class_serial); fprintf(f, " reg_cfp->pc = original_body_iseq + %d;\n", pos); - fprintf(f, " reg_cfp->sp = (VALUE *)reg_cfp->bp_ + %d;\n", b->stack_size); + fprintf(f, " reg_cfp->sp = vm_base_ptr(reg_cfp) + %d;\n", b->stack_size); fprintf(f, " goto cancel;\n"); fprintf(f, " }\n"); @@ -77,7 +77,7 @@ https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/_mjit_compile_send.erb#L77 % # JIT: We should evaluate ISeq modified for TracePoint if it's enabled. Note: This is slow. fprintf(f, " if (UNLIKELY(ruby_vm_event_enabled_global_flags & ISEQ_TRACE_EVENTS)) {\n"); - fprintf(f, " reg_cfp->sp = (VALUE *)reg_cfp->bp_ + %d;\n", b->stack_size + (int)<%= insn.call_attribute('sp_inc') %>); + fprintf(f, " reg_cfp->sp = vm_base_ptr(reg_cfp) + %d;\n", b->stack_size + (int)<%= insn.call_attribute('sp_inc') %>); if (!pc_moved_p) { fprintf(f, " reg_cfp->pc = original_body_iseq + %d;\n", next_pos); } Index: tool/ruby_vm/views/_mjit_compile_ivar.erb =================================================================== --- tool/ruby_vm/views/_mjit_compile_ivar.erb (revision 67343) +++ tool/ruby_vm/views/_mjit_compile_ivar.erb (revision 67344) @@ -41,7 +41,7 @@ https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/_mjit_compile_ivar.erb#L41 % end fprintf(f, " else {\n"); fprintf(f, " reg_cfp->pc = original_body_iseq + %d;\n", pos); - fprintf(f, " reg_cfp->sp = (VALUE *)reg_cfp->bp_ + %d;\n", b->stack_size); + fprintf(f, " reg_cfp->sp = vm_base_ptr(reg_cfp) + %d;\n", b->stack_size); fprintf(f, " goto cancel;\n"); fprintf(f, " }\n"); Index: tool/ruby_vm/views/_mjit_compile_insn.erb =================================================================== --- tool/ruby_vm/views/_mjit_compile_insn.erb (revision 67343) +++ tool/ruby_vm/views/_mjit_compile_insn.erb (revision 67344) @@ -62,7 +62,7 @@ https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/_mjit_compile_insn.erb#L62 % # JIT: We should evaluate ISeq modified for TracePoint if it's enabled. Note: This is slow. % unless insn.always_leaf? fprintf(f, " if (UNLIKELY(ruby_vm_event_enabled_global_flags & ISEQ_TRACE_EVENTS)) {\n"); - fprintf(f, " reg_cfp->sp = (VALUE *)reg_cfp->bp_ + %d;\n", b->stack_size + (int)<%= insn.call_attribute('sp_inc') %>); + fprintf(f, " reg_cfp->sp = vm_base_ptr(reg_cfp) + %d;\n", b->stack_size + (int)<%= insn.call_attribute('sp_inc') %>); if (!pc_moved_p) { fprintf(f, " reg_cfp->pc = original_body_iseq + %d;\n", next_pos); } Index: vm_core.h =================================================================== --- vm_core.h (revision 67343) +++ vm_core.h (revision 67344) @@ -763,7 +763,7 @@ typedef struct rb_control_frame_struct { https://github.com/ruby/ruby/blob/trunk/vm_core.h#L763 VALUE self; /* cfp[3] / block[0] */ const VALUE *ep; /* cfp[4] / block[1] */ const void *block_code; /* cfp[5] / block[2] */ /* iseq or ifunc */ - VALUE *bp_; /* cfp[6] */ + VALUE *__bp__; /* cfp[6] */ /* outside vm_push_frame, use vm_base_ptr instead. */ #if VM_DEBUG_BP_CHECK VALUE *bp_check; /* cfp[7] */ Index: vm_insnhelper.c =================================================================== --- vm_insnhelper.c (revision 67343) +++ vm_insnhelper.c (revision 67344) @@ -300,7 +300,7 @@ vm_push_frame(rb_execution_context_t *ec https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L300 /* Store initial value of ep as bp to skip calculation cost of bp on JIT cancellation. */ cfp->ep = sp; - cfp->bp_ = cfp->sp = sp + 1; + cfp->__bp__ = cfp->sp = sp + 1; #if VM_DEBUG_BP_CHECK cfp->bp_check = sp + 1; @@ -1637,10 +1637,10 @@ double_cmp_ge(double a, double b) https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L1637 return a >= b ? Qtrue : Qfalse; } -static VALUE * +static inline VALUE * vm_base_ptr(const rb_control_frame_t *cfp) { -#if 0 +#if 0 // we may optimize and use this once we confirm it does not spoil performance on JIT. const rb_control_frame_t *prev_cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp); if (cfp->iseq && VM_FRAME_RUBYFRAME_P(cfp)) { @@ -1663,7 +1663,7 @@ vm_base_ptr(const rb_control_frame_t *cf https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L1663 return NULL; } #else - return cfp->bp_; + return cfp->__bp__; #endif } Index: mjit_compile.c =================================================================== --- mjit_compile.c (revision 67343) +++ mjit_compile.c (revision 67344) @@ -191,7 +191,7 @@ compile_cancel_handler(FILE *f, const st https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L191 fprintf(f, "\ncancel:\n"); if (status->local_stack_p) { for (i = 0; i < body->stack_max; i++) { - fprintf(f, " *((VALUE *)reg_cfp->bp_ + %d) = stack[%d];\n", i, i); + fprintf(f, " *(vm_base_ptr(reg_cfp) + %d) = stack[%d];\n", i, i); } } fprintf(f, " return Qundef;\n"); -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/