From 97f3e426a1c84d9bee604fbe458cb110e80c93af Mon Sep 17 00:00:00 2001 From: k0kubun Date: Wed, 24 Oct 2018 15:41:14 +0000 Subject: [PATCH] _mjit_compile_ivar.erb: cancel on undefined ivar I assumed somehow this check was not needed, but it did need. By canceling this instead of just warning here, we didn't lose the current performance so much. test_jit.rb: test the case that reproduces SEGV by that. TestGemStreamUI.rb: delete. This test on --jit-wait is fixed. === Optcarrot Benchmark === $ benchmark-driver benchmark.yml --rbenv 'before::before --disable-gems --jit;after::after --disable-gems --jit' -v --repeat-count 24 before: ruby 2.6.0dev (2018-10-24 trunk 65355) +JIT [x86_64-linux] after: ruby 2.6.0dev (2018-10-24 trunk 65355) +JIT [x86_64-linux] last_commit=_mjit_compile_ivar.erb: cancel on undefined ivar Calculating ------------------------------------- before after Optcarrot Lan_Master.nes 85.344 84.849 fps Comparison: Optcarrot Lan_Master.nes before: 85.3 fps after: 84.8 fps - 1.01x slower git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@65356 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- .../test-mjit-wait/TestGemStreamUI.rb | 2 -- test/ruby/test_jit.rb | 21 +++++++++++++++++++ tool/ruby_vm/views/_mjit_compile_ivar.erb | 14 ++++++------- 3 files changed, 28 insertions(+), 9 deletions(-) delete mode 100644 test/excludes/_wercker/test-mjit-wait/TestGemStreamUI.rb diff --git a/test/excludes/_wercker/test-mjit-wait/TestGemStreamUI.rb b/test/excludes/_wercker/test-mjit-wait/TestGemStreamUI.rb deleted file mode 100644 index 56edf339b2..0000000000 --- a/test/excludes/_wercker/test-mjit-wait/TestGemStreamUI.rb +++ /dev/null @@ -1,2 +0,0 @@ -# debugging this right now -exclude(/.*/, 'this fails with --jit-wait') diff --git a/test/ruby/test_jit.rb b/test/ruby/test_jit.rb index 888290faa9..5c31b504c4 100644 --- a/test/ruby/test_jit.rb +++ b/test/ruby/test_jit.rb @@ -669,6 +669,27 @@ class TestJIT < Test::Unit::TestCase end; end + def test_inlined_undefined_ivar + assert_eval_with_jit("#{<<~"begin;"}\n#{<<~"end;"}", stdout: "bbb", success_count: 2, min_calls: 3) + begin; + class Foo + def initialize + @a = :a + end + + def bar + if @b.nil? + @b = :b + end + end + end + + print(Foo.new.bar) + print(Foo.new.bar) + print(Foo.new.bar) + end; + end + def test_attr_reader assert_eval_with_jit("#{<<~"begin;"}\n#{<<~"end;"}", stdout: "4nil\nnil\n6", success_count: 2, min_calls: 2) begin; diff --git a/tool/ruby_vm/views/_mjit_compile_ivar.erb b/tool/ruby_vm/views/_mjit_compile_ivar.erb index 3f75b576da..f1963c23f7 100644 --- a/tool/ruby_vm/views/_mjit_compile_ivar.erb +++ b/tool/ruby_vm/views/_mjit_compile_ivar.erb @@ -26,19 +26,19 @@ fprintf(f, " VALUE obj = GET_SELF();\n"); fprintf(f, " const rb_serial_t ic_serial = (rb_serial_t)%"PRI_SERIALT_PREFIX"u;\n", ic_copy->ic_serial); fprintf(f, " const st_index_t index = %"PRIuSIZE";\n", ic_copy->ic_value.index); +% # JIT: cache hit path of vm_getivar, or cancel JIT. % if insn.name == 'setinstancevariable' fprintf(f, " VALUE val = stack[%d];\n", b->stack_size - 1); -% end -% -% # JIT: cache hit path of vm_getivar, or cancel JIT. fprintf(f, " if (LIKELY(RB_TYPE_P(obj, T_OBJECT) && ic_serial == RCLASS_SERIAL(RBASIC(obj)->klass) && index < ROBJECT_NUMIV(obj))) {\n"); -% if insn.name == 'setinstancevariable' fprintf(f, " VALUE *ptr = ROBJECT_IVPTR(obj);\n"); fprintf(f, " RB_OBJ_WRITE(obj, &ptr[index], val);\n"); -% else - fprintf(f, " stack[%d] = ROBJECT_IVPTR(obj)[index];\n", b->stack_size); -% end fprintf(f, " }\n"); +% else + fprintf(f, " VALUE val;\n"); + fprintf(f, " if (LIKELY(RB_TYPE_P(obj, T_OBJECT) && ic_serial == RCLASS_SERIAL(RBASIC(obj)->klass) && index < ROBJECT_NUMIV(obj) && (val = ROBJECT_IVPTR(obj)[index]) != Qundef)) {\n"); + fprintf(f, " stack[%d] = val;\n", b->stack_size); + fprintf(f, " }\n"); +% 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 + 1);