From 03645d1eefdf280c3c1ff20f9431fb8fe45799b4 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Wed, 17 Jan 2024 10:52:15 -0500 Subject: [PATCH] YJIT: Support empty splat and some block_arg calls to ivar getters (#9567) These seem odd at first glance, but they're used with `...` calls with `Module#delegate` from Active Support. These account for ~3% of fallback reasons in the `lobsters` benchmark. --- test/ruby/test_yjit.rb | 34 ++++++++++++++++++++++++++- yjit/src/codegen.rs | 52 ++++++++++++++++++++++++++++-------------- yjit/src/stats.rs | 1 + 3 files changed, 69 insertions(+), 18 deletions(-) diff --git a/test/ruby/test_yjit.rb b/test/ruby/test_yjit.rb index 55b86bea78..bd513c60e6 100644 --- a/test/ruby/test_yjit.rb +++ b/test/ruby/test_yjit.rb @@ -1514,6 +1514,24 @@ class TestYJIT < Test::Unit::TestCase assert_in_out_err(%w[--yjit-stats --yjit-disable]) end + def test_odd_calls_to_attr_reader + # Use of delegate from ActiveSupport use these kind of calls to getter methods. + assert_compiles(<<~RUBY, result: [1, 1, 1], no_send_fallbacks: true) + class One + attr_reader :one + def initialize + @one = 1 + end + end + + def calls(obj, empty, &) + [obj.one(*empty), obj.one(&), obj.one(*empty, &)] + end + + calls(One.new, []) + RUBY + end + private def code_gc_helpers @@ -1537,7 +1555,17 @@ class TestYJIT < Test::Unit::TestCase end ANY = Object.new - def assert_compiles(test_script, insns: [], call_threshold: 1, stdout: nil, exits: {}, result: ANY, frozen_string_literal: nil, mem_size: nil, code_gc: false) + def assert_compiles( + test_script, insns: [], + call_threshold: 1, + stdout: nil, + exits: {}, + result: ANY, + frozen_string_literal: nil, + mem_size: nil, + code_gc: false, + no_send_fallbacks: false + ) reset_stats = <<~RUBY RubyVM::YJIT.runtime_stats RubyVM::YJIT.reset_stats! @@ -1610,6 +1638,10 @@ class TestYJIT < Test::Unit::TestCase end end + if no_send_fallbacks + assert_equal(0, runtime_stats[:num_send_dynamic], "Expected no use of fallback implementation") + end + # Only available when --enable-yjit=dev if runtime_stats[:all_stats] missed_insns = insns.dup diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 7d42030345..378b397fe4 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -7355,23 +7355,45 @@ fn gen_send_general( ); } VM_METHOD_TYPE_IVAR => { - if flags & VM_CALL_ARGS_SPLAT != 0 { - gen_counter_incr(asm, Counter::send_args_splat_ivar); - return None; - } - - if argc != 0 { - // Argument count mismatch. Getters take no arguments. - gen_counter_incr(asm, Counter::send_getter_arity); - return None; - } - // This is a .send call not supported right now for getters if flags & VM_CALL_OPT_SEND != 0 { gen_counter_incr(asm, Counter::send_send_getter); return None; } + if flags & VM_CALL_ARGS_BLOCKARG != 0 { + match asm.ctx.get_opnd_type(StackOpnd(0)) { + Type::Nil | Type::BlockParamProxy => { + // Getters ignore the block arg, and these types of block args can be + // passed without side-effect (never any `to_proc` call). + asm.stack_pop(1); + } + _ => { + gen_counter_incr(asm, Counter::send_getter_block_arg); + return None; + } + } + } + + if argc != 0 { + // Guard for simple splat of empty array + if VM_CALL_ARGS_SPLAT == flags & (VM_CALL_ARGS_SPLAT | VM_CALL_KWARG | VM_CALL_KW_SPLAT) + && argc == 1 { + // Not using chain guards since on failure these likely end up just raising + // ArgumentError + let splat = asm.stack_opnd(0); + guard_object_is_array(asm, splat, splat.into(), Counter::guard_send_getter_splat_non_empty); + let splat_len = get_array_len(asm, splat); + asm.cmp(splat_len, 0.into()); + asm.jne(Target::side_exit(Counter::guard_send_getter_splat_non_empty)); + asm.stack_pop(1); + } else { + // Argument count mismatch. Getters take no arguments. + gen_counter_incr(asm, Counter::send_getter_arity); + return None; + } + } + if c_method_tracing_currently_enabled(jit) { // Can't generate code for firing c_call and c_return events // :attr-tracing: @@ -7386,13 +7408,9 @@ fn gen_send_general( return None; } + let recv = asm.stack_opnd(0); // the receiver should now be the stack top let ivar_name = unsafe { get_cme_def_body_attr_id(cme) }; - if flags & VM_CALL_ARGS_BLOCKARG != 0 { - gen_counter_incr(asm, Counter::send_getter_block_arg); - return None; - } - return gen_get_ivar( jit, asm, @@ -7401,7 +7419,7 @@ fn gen_send_general( comptime_recv, ivar_name, recv, - recv_opnd, + recv.into(), ); } VM_METHOD_TYPE_ATTRSET => { diff --git a/yjit/src/stats.rs b/yjit/src/stats.rs index 4b77dceaf1..264843117a 100644 --- a/yjit/src/stats.rs +++ b/yjit/src/stats.rs @@ -400,6 +400,7 @@ make_counters! { // Method calls that exit to the interpreter guard_send_block_arg_type, + guard_send_getter_splat_non_empty, guard_send_klass_megamorphic, guard_send_se_cf_overflow, guard_send_se_protected_check_failed,