From 7a3322a0fd660d676f1918bd7c4a37676b44e1c2 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Mon, 4 Jan 2021 13:09:01 -0800 Subject: Fix broken JIT of getinlinecache e7fc353f04 reverted vm_ic_hit_p's signature change made in 53babf35ef, which broke JIT compilation of getinlinecache. To make sure it doesn't happen again, I separated vm_inlined_ic_hit_p to make the intention clear. --- test/ruby/test_jit.rb | 12 +++++++++++ .../ruby_vm/views/_mjit_compile_getinlinecache.erb | 2 +- vm_insnhelper.c | 25 ++++++++++++++-------- 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/test/ruby/test_jit.rb b/test/ruby/test_jit.rb index 61485c7479..3bc4a871a1 100644 --- a/test/ruby/test_jit.rb +++ b/test/ruby/test_jit.rb @@ -857,6 +857,18 @@ class TestJIT < Test::Unit::TestCase end; end + def test_inlined_getconstant + assert_eval_with_jit("#{<<~"begin;"}\n#{<<~"end;"}", stdout: '11', success_count: 1, min_calls: 2) + begin; + FOO = 1 + def const + FOO + end + print const + print const + 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_getinlinecache.erb b/tool/ruby_vm/views/_mjit_compile_getinlinecache.erb index 637dea89e6..1acfdb7f0b 100644 --- a/tool/ruby_vm/views/_mjit_compile_getinlinecache.erb +++ b/tool/ruby_vm/views/_mjit_compile_getinlinecache.erb @@ -15,7 +15,7 @@ struct iseq_inline_constant_cache_entry *ice = ic->entry; if (ice != NULL && ice->ic_serial && !status->compile_info->disable_const_cache) { % # JIT: Inline everything in IC, and cancel the slow path - fprintf(f, " if (vm_ic_hit_p((rb_serial_t)%"PRI_SERIALT_PREFIX"u, (const rb_cref_t *)0x%"PRIxVALUE", reg_cfp->ep)) {", ice->ic_serial, (VALUE)ice->ic_cref); + fprintf(f, " if (vm_inlined_ic_hit_p(0x%"PRIxVALUE", 0x%"PRIxVALUE", (const rb_cref_t *)0x%"PRIxVALUE", %"PRI_SERIALT_PREFIX"u, reg_cfp->ep)) {", ice->flags, ice->value, (VALUE)ice->ic_cref, ice->ic_serial); fprintf(f, " stack[%d] = 0x%"PRIxVALUE";\n", b->stack_size, ice->value); fprintf(f, " goto label_%d;\n", pos + insn_len(insn) + (int)dst); fprintf(f, " }"); diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 7ac5567e9f..92bc94767b 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -4613,19 +4613,26 @@ vm_opt_newarray_min(rb_num_t num, const VALUE *ptr) #define IMEMO_CONST_CACHE_SHAREABLE IMEMO_FL_USER0 -static int -vm_ic_hit_p(const struct iseq_inline_constant_cache_entry *ice, const VALUE *reg_ep) +// For MJIT inlining +static inline bool +vm_inlined_ic_hit_p(VALUE flags, VALUE value, const rb_cref_t *ic_cref, rb_serial_t ic_serial, const VALUE *reg_ep) { - VM_ASSERT(IMEMO_TYPE_P(ice, imemo_constcache)); - if (ice->ic_serial == GET_GLOBAL_CONSTANT_STATE() && - (FL_TEST_RAW((VALUE)ice, IMEMO_CONST_CACHE_SHAREABLE) || rb_ractor_main_p())) { + if (ic_serial == GET_GLOBAL_CONSTANT_STATE() && + ((flags & IMEMO_CONST_CACHE_SHAREABLE) || rb_ractor_main_p())) { - VM_ASSERT(FL_TEST_RAW((VALUE)ice, IMEMO_CONST_CACHE_SHAREABLE) ? rb_ractor_shareable_p(ice->value) : true); + VM_ASSERT((flags & IMEMO_CONST_CACHE_SHAREABLE) ? rb_ractor_shareable_p(value) : true); - return (ice->ic_cref == NULL || // no need to check CREF - ice->ic_cref == vm_get_cref(reg_ep)); + return (ic_cref == NULL || // no need to check CREF + ic_cref == vm_get_cref(reg_ep)); } - return FALSE; + return false; +} + +static bool +vm_ic_hit_p(const struct iseq_inline_constant_cache_entry *ice, const VALUE *reg_ep) +{ + VM_ASSERT(IMEMO_TYPE_P(ice, imemo_constcache)); + return vm_inlined_ic_hit_p(ice->flags, ice->value, ice->ic_cref, ice->ic_serial, reg_ep); } static void -- cgit v1.2.3