From bfb8cc83ad5bb4bcb4ae7e8d5bb6b9e9f8f675cd Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Tue, 16 Jan 2024 18:13:32 -0500 Subject: [PATCH] YJIT: Support empty splat and some block_arg calls to ivar getters 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 | 50 ++++++++++++++++++++++++++++-------------- yjit/src/stats.rs | 1 + 3 files changed, 68 insertions(+), 17 deletions(-) diff --git a/test/ruby/test_yjit.rb b/test/ruby/test_yjit.rb index 55b86bea78f11c..bd513c60e660cd 100644 --- a/test/ruby/test_yjit.rb +++ b/test/ruby/test_yjit.rb @@ -1514,6 +1514,24 @@ def test_disable_stats 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 @@ def assert_no_exits(script) 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 @@ def collect_insns(iseq) 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 2c3e61356c6a02..2cb5f65777d59d 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -7307,21 +7307,43 @@ fn gen_send_general( ); } VM_METHOD_TYPE_IVAR => { - if flags & VM_CALL_ARGS_SPLAT != 0 { - gen_counter_incr(asm, Counter::send_args_splat_ivar); + // 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 argc != 0 { - // Argument count mismatch. Getters take no arguments. - gen_counter_incr(asm, Counter::send_getter_arity); - 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; + } + } } - // 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 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) { @@ -7338,13 +7360,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, @@ -7353,7 +7371,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 924628e13acedf..785b7254a33101 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,