From 5808999d30fc7c4fd74048a50129fd87b590b41c Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Fri, 13 Oct 2023 08:52:23 -0700 Subject: [PATCH] YJIT: Fallback opt_getconstant_path for const_missing (#8623) * YJIT: Fallback opt_getconstant_path for const_missing * Fix a comment [ci skip] * Remove a wrapper function --- insns.def | 15 +------------ test/ruby/test_yjit.rb | 30 +++++++++++++++++++++++-- test/ruby/test_yjit_exit_locations.rb | 18 ++------------- vm_insnhelper.c | 21 ++++++++++++++++++ yjit/src/codegen.rs | 32 +++++++++++++++++++-------- yjit/src/stats.rs | 1 - 6 files changed, 75 insertions(+), 42 deletions(-) diff --git a/insns.def b/insns.def index bbf946ac67..b784d7c043 100644 --- a/insns.def +++ b/insns.def @@ -260,20 +260,7 @@ opt_getconstant_path (VALUE val) // attr bool leaf = false; /* may autoload or raise */ { - const ID *segments = ic->segments; - struct iseq_inline_constant_cache_entry *ice = ic->entry; - if (ice && vm_ic_hit_p(ice, GET_EP())) { - val = ice->value; - - VM_ASSERT(val == vm_get_ev_const_chain(ec, segments)); - } else { - ruby_vm_constant_cache_misses++; - val = vm_get_ev_const_chain(ec, segments); - vm_ic_track_const_chain(GET_CFP(), ic, segments); - // Because leaf=false, we need to undo the PC increment to get the address to this instruction - // INSN_ATTR(width) == 2 - vm_ic_update(GET_ISEQ(), ic, val, GET_EP(), GET_PC() - 2); - } + val = rb_vm_opt_getconstant_path(ec, GET_CFP(), ic); } /* Get constant variable id. If klass is Qnil and allow_nil is Qtrue, constants diff --git a/test/ruby/test_yjit.rb b/test/ruby/test_yjit.rb index 10508f8e8f..d820b052b1 100644 --- a/test/ruby/test_yjit.rb +++ b/test/ruby/test_yjit.rb @@ -476,6 +476,32 @@ class TestYJIT < Test::Unit::TestCase RUBY end + def test_opt_getconstant_path_general + assert_compiles(<<~RUBY, result: [1, 1]) + module Base + Const = 1 + end + + class Sub + def const + _const = nil # make a non-entry block for opt_getconstant_path + Const + end + + def self.const_missing(n) + Base.const_get(n) + end + end + + + sub = Sub.new + result = [] + result << sub.const # generate the general case + result << sub.const # const_missing does not invalidate the block + result + RUBY + end + def test_string_interpolation assert_compiles(<<~'RUBY', insns: %i[objtostring anytostring concatstrings], result: "foobar", call_threshold: 2) def make_str(foo, bar) @@ -1257,7 +1283,7 @@ class TestYJIT < Test::Unit::TestCase def test_setivar_on_class # Bug in https://github.com/ruby/ruby/pull/8152 - assert_compiles(<<~RUBY, result: :ok, exits: { opt_getconstant_path: 2 }) + assert_compiles(<<~RUBY, result: :ok) class Base def self.or_equal @or_equal ||= Object.new @@ -1280,7 +1306,7 @@ class TestYJIT < Test::Unit::TestCase def test_nested_send #[Bug #19464] - assert_compiles(<<~RUBY, result: [:ok, :ok], exits: { opt_getconstant_path: 1, defineclass: 1 }) + assert_compiles(<<~RUBY, result: [:ok, :ok], exits: { defineclass: 1 }) klass = Class.new do class << self alias_method :my_send, :send diff --git a/test/ruby/test_yjit_exit_locations.rb b/test/ruby/test_yjit_exit_locations.rb index 851a582470..816ab457ce 100644 --- a/test/ruby/test_yjit_exit_locations.rb +++ b/test/ruby/test_yjit_exit_locations.rb @@ -18,22 +18,8 @@ class TestYJITExitLocations < Test::Unit::TestCase refute_includes(stderr, "NoMethodError") end - def test_trace_exits_setclassvariable - script = 'class Foo; def self.foo; @@foo = 1; end; end; Foo.foo' - assert_exit_locations(script) - end - - def test_trace_exits_putobject - assert_exit_locations('true') - assert_exit_locations('123') - assert_exit_locations(':foo') - end - - def test_trace_exits_opt_not - assert_exit_locations('!false') - assert_exit_locations('!nil') - assert_exit_locations('!true') - assert_exit_locations('![]') + def test_trace_exits_expandarray_splat + assert_exit_locations('*arr = []') end private diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 9d3842ac75..3955a523d0 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -5846,6 +5846,27 @@ vm_ic_update(const rb_iseq_t *iseq, IC ic, VALUE val, const VALUE *reg_ep, const rb_rjit_constant_ic_update(iseq, ic, pos); } +VALUE +rb_vm_opt_getconstant_path(rb_execution_context_t *ec, rb_control_frame_t *const reg_cfp, IC ic) +{ + VALUE val; + const ID *segments = ic->segments; + struct iseq_inline_constant_cache_entry *ice = ic->entry; + if (ice && vm_ic_hit_p(ice, GET_EP())) { + val = ice->value; + + VM_ASSERT(val == vm_get_ev_const_chain(ec, segments)); + } else { + ruby_vm_constant_cache_misses++; + val = vm_get_ev_const_chain(ec, segments); + vm_ic_track_const_chain(GET_CFP(), ic, segments); + // Undo the PC increment to get the address to this instruction + // INSN_ATTR(width) == 2 + vm_ic_update(GET_ISEQ(), ic, val, GET_EP(), GET_PC() - 2); + } + return val; +} + static VALUE vm_once_dispatch(rb_execution_context_t *ec, ISEQ iseq, ISE is) { diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 2707932a3f..7968e6303a 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -8134,19 +8134,33 @@ fn gen_opt_getconstant_path( let ic: *const iseq_inline_constant_cache = const_cache_as_value.as_ptr(); let idlist: *const ID = unsafe { (*ic).segments }; - // See vm_ic_hit_p(). The same conditions are checked in yjit_constant_ic_update(). - let ice = unsafe { (*ic).entry }; - if ice.is_null() { - // In this case, leave a block that unconditionally side exits - // for the interpreter to invalidate. - gen_counter_incr(asm, Counter::opt_getconstant_path_no_ic_entry); - return None; - } - // Make sure there is an exit for this block as the interpreter might want // to invalidate this block from yjit_constant_ic_update(). jit_ensure_block_entry_exit(jit, asm, ocb); + // See vm_ic_hit_p(). The same conditions are checked in yjit_constant_ic_update(). + // If a cache is not filled, fallback to the general C call. + let ice = unsafe { (*ic).entry }; + if ice.is_null() { + // Prepare for const_missing + jit_prepare_routine_call(jit, asm); + + // If this does not trigger const_missing, vm_ic_update will invalidate this block. + extern "C" { + fn rb_vm_opt_getconstant_path(ec: EcPtr, cfp: CfpPtr, ic: *const u8) -> VALUE; + } + let val = asm.ccall( + rb_vm_opt_getconstant_path as *const u8, + vec![EC, CFP, Opnd::const_ptr(ic as *const u8)], + ); + + let stack_top = asm.stack_push(Type::Unknown); + asm.store(stack_top, val); + + jump_to_next_insn(jit, asm, ocb); + return Some(EndBlock); + } + if !unsafe { (*ice).ic_cref }.is_null() { // Cache is keyed on a certain lexical scope. Use the interpreter's cache. let inline_cache = asm.load(Opnd::const_ptr(ic as *const u8)); diff --git a/yjit/src/stats.rs b/yjit/src/stats.rs index 0bcae1cdc9..f20fd81242 100644 --- a/yjit/src/stats.rs +++ b/yjit/src/stats.rs @@ -404,7 +404,6 @@ make_counters! { opt_case_dispatch_megamorphic, opt_getconstant_path_ic_miss, - opt_getconstant_path_no_ic_entry, opt_getconstant_path_multi_ractor, expandarray_splat,