ruby-changes:73562
From: John <ko1@a...>
Date: Thu, 15 Sep 2022 05:16:28 +0900 (JST)
Subject: [ruby-changes:73562] f98d6d3f38 (master): YJIT: Implement specialized respond_to? (#6363)
https://git.ruby-lang.org/ruby.git/commit/?id=f98d6d3f38 From f98d6d3f389e8e46775c5895ddc1a3eec4544533 Mon Sep 17 00:00:00 2001 From: John Hawthorn <john@h...> Date: Wed, 14 Sep 2022 13:15:55 -0700 Subject: YJIT: Implement specialized respond_to? (#6363) * Add rb_callable_method_entry_or_negative * YJIT: Implement specialized respond_to? This implements a specialized respond_to? in YJIT. * Update yjit/src/codegen.rs Co-authored-by: Maxime Chevalier-Boisvert <maximechevalierb@g...> --- bootstraptest/test_yjit.rb | 54 +++++++++++++++++++++++ method.h | 1 + vm_method.c | 16 ++++++- yjit.c | 6 ++- yjit/bindgen/src/main.rs | 2 + yjit/src/codegen.rs | 98 ++++++++++++++++++++++++++++++++++++++++++ yjit/src/cruby_bindings.inc.rs | 9 ++++ yjit/src/invariants.rs | 19 ++++++++ 8 files changed, 203 insertions(+), 2 deletions(-) diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb index 364ed7094b..2dff7d591a 100644 --- a/bootstraptest/test_yjit.rb +++ b/bootstraptest/test_yjit.rb @@ -3255,3 +3255,57 @@ assert_equal '[1, 2]', %q{ https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_yjit.rb#L3255 foo foo } + +# respond_to? with changing symbol +assert_equal 'false', %q{ + def foo(name) + :sym.respond_to?(name) + end + foo(:to_s) + foo(:to_s) + foo(:not_exist) +} + +# respond_to? with method being defined +assert_equal 'true', %q{ + def foo + :sym.respond_to?(:not_yet_defined) + end + foo + foo + module Kernel + def not_yet_defined = true + end + foo +} + +# respond_to? with undef method +assert_equal 'false', %q{ + module Kernel + def to_be_removed = true + end + def foo + :sym.respond_to?(:to_be_removed) + end + foo + foo + class Object + undef_method :to_be_removed + end + foo +} + +# respond_to? with respond_to_missing? +assert_equal 'true', %q{ + class Foo + end + def foo(x) + x.respond_to?(:bar) + end + foo(Foo.new) + foo(Foo.new) + class Foo + def respond_to_missing?(*) = true + end + foo(Foo.new) +} diff --git a/method.h b/method.h index 16d212a1c8..d33ab5053c 100644 --- a/method.h +++ b/method.h @@ -226,6 +226,7 @@ const rb_method_entry_t *rb_resolve_me_location(const rb_method_entry_t *, VALUE https://github.com/ruby/ruby/blob/trunk/method.h#L226 RUBY_SYMBOL_EXPORT_END const rb_callable_method_entry_t *rb_callable_method_entry(VALUE klass, ID id); +const rb_callable_method_entry_t *rb_callable_method_entry_or_negative(VALUE klass, ID id); const rb_callable_method_entry_t *rb_callable_method_entry_with_refinements(VALUE klass, ID id, VALUE *defined_class); const rb_callable_method_entry_t *rb_callable_method_entry_without_refinements(VALUE klass, ID id, VALUE *defined_class); diff --git a/vm_method.c b/vm_method.c index e88a30c5f3..fbe62ecd4c 100644 --- a/vm_method.c +++ b/vm_method.c @@ -1347,7 +1347,7 @@ negative_cme(ID mid) https://github.com/ruby/ruby/blob/trunk/vm_method.c#L1347 } static const rb_callable_method_entry_t * -callable_method_entry(VALUE klass, ID mid, VALUE *defined_class_ptr) +callable_method_entry_or_negative(VALUE klass, ID mid, VALUE *defined_class_ptr) { const rb_callable_method_entry_t *cme; @@ -1376,6 +1376,20 @@ callable_method_entry(VALUE klass, ID mid, VALUE *defined_class_ptr) https://github.com/ruby/ruby/blob/trunk/vm_method.c#L1376 } RB_VM_LOCK_LEAVE(); + return cme; +} + +// This is exposed for YJIT so that we can make assumptions that methods are +// not defined. +const rb_callable_method_entry_t * +rb_callable_method_entry_or_negative(VALUE klass, ID mid) { + return callable_method_entry_or_negative(klass, mid, NULL); +} + +static const rb_callable_method_entry_t * +callable_method_entry(VALUE klass, ID mid, VALUE *defined_class_ptr) { + const rb_callable_method_entry_t *cme; + cme = callable_method_entry_or_negative(klass, mid, defined_class_ptr); return !UNDEFINED_METHOD_ENTRY_P(cme) ? cme : NULL; } diff --git a/yjit.c b/yjit.c index facddcca0a..afa329e024 100644 --- a/yjit.c +++ b/yjit.c @@ -486,7 +486,11 @@ rb_METHOD_ENTRY_VISI(const rb_callable_method_entry_t *me) https://github.com/ruby/ruby/blob/trunk/yjit.c#L486 rb_method_type_t rb_get_cme_def_type(const rb_callable_method_entry_t *cme) { - return cme->def->type; + if (UNDEFINED_METHOD_ENTRY_P(cme)) { + return VM_METHOD_TYPE_UNDEF; + } else { + return cme->def->type; + } } ID diff --git a/yjit/bindgen/src/main.rs b/yjit/bindgen/src/main.rs index 294da21378..c3d4a39a2b 100644 --- a/yjit/bindgen/src/main.rs +++ b/yjit/bindgen/src/main.rs @@ -225,6 +225,7 @@ fn main() { https://github.com/ruby/ruby/blob/trunk/yjit/bindgen/src/main.rs#L225 .allowlist_var(".*_REDEFINED_OP_FLAG") .allowlist_type("rb_num_t") .allowlist_function("rb_callable_method_entry") + .allowlist_function("rb_callable_method_entry_or_negative") .allowlist_function("rb_vm_frame_method_entry") .allowlist_type("IVC") // pointer to iseq_inline_iv_cache_entry .allowlist_type("IC") // pointer to iseq_inline_constant_cache @@ -367,6 +368,7 @@ fn main() { https://github.com/ruby/ruby/blob/trunk/yjit/bindgen/src/main.rs#L368 .allowlist_function("rb_vm_ci_kwarg") .allowlist_function("rb_METHOD_ENTRY_VISI") .allowlist_function("rb_RCLASS_ORIGIN") + .allowlist_function("rb_method_basic_definition_p") // We define VALUE manually, don't import it .blocklist_type("VALUE") diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index f13505365e..11f7085635 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -3841,6 +3841,102 @@ fn jit_rb_str_concat( https://github.com/ruby/ruby/blob/trunk/yjit/src/codegen.rs#L3841 true } +fn jit_obj_respond_to( + jit: &mut JITState, + ctx: &mut Context, + asm: &mut Assembler, + ocb: &mut OutlinedCb, + _ci: *const rb_callinfo, + _cme: *const rb_callable_method_entry_t, + _block: Option<IseqPtr>, + argc: i32, + known_recv_class: *const VALUE, +) -> bool { + // respond_to(:sym) or respond_to(:sym, true) + if argc != 1 && argc != 2 { + return false; + } + + if known_recv_class.is_null() { + return false; + } + + let recv_class = unsafe { *known_recv_class }; + + // Get the method_id from compile time. We will later add a guard against it. + let mid_sym = jit_peek_at_stack(jit, ctx, (argc - 1) as isize); + if !mid_sym.static_sym_p() { + return false + } + let mid = unsafe { rb_sym2id(mid_sym) }; + + // Option<bool> representing the value of the "include_all" argument and whether it's known + let allow_priv = if argc == 1 { + // Default is false + Some(false) + } else { + // Get value from type information (may or may not be known) + ctx.get_opnd_type(StackOpnd(0)).known_truthy() + }; + + let mut target_cme = unsafe { rb_callable_method_entry_or_negative(recv_class, mid) }; + + // Should never be null, as in that case we will be returned a "negative CME" + assert!(!target_cme.is_null()); + + let cme_def_type = unsafe { get_cme_def_type(target_cme) }; + + if cme_def_type == VM_METHOD_TYPE_REFINED { + return false; + } + + let visibility = if cme_def_type == VM_METHOD_TYPE_UNDEF { + METHOD_VISI_UNDEF + } else { + unsafe { METHOD_ENTRY_VISI(target_cme) } + }; + + let result = match (visibility, allow_priv) { + (METHOD_VISI_UNDEF, _) => Qfalse, // No method => false + (METHOD_VISI_PUBLIC, _) => Qtrue, // Public method => true regardless of include_all + (_, Some(true)) => Qtrue, // include_all => always true + (_, _) => return false // not public and include_all not known, can't compile + }; + + if result != Qtrue { + // Only if respond_to_missing? hasn't been overridden + // In the future, we might want to jit the call to respond_to_missing? + if !assume_method_basic_definition(jit, ocb, recv_class, idRespond_to_missing.into()) { + return false; + } + } + + // Invalidate this block if method lookup changes for the method being queried. This works + // both for the case where a method does or does not exist, as for the latter we asked for a + // "negative CME" earlier. + assume_method_lookup_stable(jit, ocb, recv_class, target_cme); + + // Generate a side exit + let side_exit = get_side_exit(jit, ocb, ctx); + + if argc == 2 { + // pop include_all argument (we only use its type info) + ctx.stack_pop(1); + } + + let sym_opnd = ctx.stack_pop(1); + let recv_opnd = ctx.stack_pop(1); + + // This is necessary because we have no guarantee that sym_opnd is a constant + asm.comment("guard known mid"); + asm.cmp(sym_opnd, mid_sym.into()); + asm.jne(side_exit.into()); + + jit_putobject(jit, ctx, asm, result); + + true +} + fn jit_thread_s_current( _jit: &mut JITState, ctx: &mut Context, @@ -6292,6 +6388,8 @@ impl CodegenGlobals { https://github.com/ruby/ruby/blob/trunk/yjit/src/codegen.rs#L6388 self.yjit_reg_method(rb_cString, "<<", jit_rb_str_concat); self.yjit_reg_method(rb_cString, "+@", jit_rb_str_uplus); + self.yjit_reg_method(rb_mKernel, "respond_to?", jit_obj_respond_to); + // Thread.current self.yjit_reg_method( rb_singleton_class(rb_cThread), diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index e3dbdc0d4b..b391a6cda5 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -26,6 +26,9 @@ pub type rb_alloc_func_t = ::std::option::Option<unsafe extern "C" fn(klass: VAL https://github.com/ruby/ruby/blob/trunk/yjit/src/cruby_bindings.inc.rs#L26 extern "C" { pub fn rb_get_alloc_func(klass: VALUE) -> rb_alloc_func_t; } +extern "C" { + (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/