ruby-changes:68963
From: Aaron <ko1@a...>
Date: Thu, 21 Oct 2021 08:19:28 +0900 (JST)
Subject: [ruby-changes:68963] 435d7c5088 (master): Improve set instance variable
https://git.ruby-lang.org/ruby.git/commit/?id=435d7c5088 From 435d7c5088295be99d83464f2c924401844f03af Mon Sep 17 00:00:00 2001 From: Aaron Patterson <tenderlove@r...> Date: Wed, 21 Apr 2021 13:01:03 -0700 Subject: Improve set instance variable This commit improves the set ivar implementation. --- bootstraptest/test_yjit.rb | 85 ++++++++++++++++++++++ yjit_codegen.c | 176 ++++++++++++++++++++++++++++++--------------- yjit_iface.h | 7 ++ 3 files changed, 212 insertions(+), 56 deletions(-) diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb index 2da9086a60..5c7e74635a 100644 --- a/bootstraptest/test_yjit.rb +++ b/bootstraptest/test_yjit.rb @@ -239,6 +239,91 @@ assert_normal_exit %q{ https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_yjit.rb#L239 end } +# test setinstancevariable on extended objects +assert_equal '1', %q{ + class Extended + attr_reader :one + + def write_many + @a = 1 + @b = 2 + @c = 3 + @d = 4 + @one = 1 + end + end + + foo = Extended.new + foo.write_many + foo.write_many + foo.write_many +} + +# test setinstancevariable on embedded objects +assert_equal '1', %q{ + class Embedded + attr_reader :one + + def write_one + @one = 1 + end + end + + foo = Embedded.new + foo.write_one + foo.write_one + foo.write_one +} + +# test setinstancevariable after extension +assert_equal '[10, 11, 12, 13, 1]', %q{ + class WillExtend + attr_reader :one + + def make_extended + @foo1 = 10 + @foo2 = 11 + @foo3 = 12 + @foo4 = 13 + end + + def write_one + @one = 1 + end + + def read_all + [@foo1, @foo2, @foo3, @foo4, @one] + end + end + + foo = WillExtend.new + foo.write_one + foo.write_one + foo.make_extended + foo.write_one + foo.read_all +} + +# test setinstancevariable on frozen object +assert_equal 'object was not modified', %q{ + class WillFreeze + def write + @ivar = 1 + end + end + + wf = WillFreeze.new + wf.write + wf.write + wf.freeze + + begin + wf.write + rescue FrozenError + "object was not modified" + end +} + # Test getinstancevariable and inline caches assert_equal '6', %q{ class Foo diff --git a/yjit_codegen.c b/yjit_codegen.c index 9f5b0dde1d..bae2275208 100644 --- a/yjit_codegen.c +++ b/yjit_codegen.c @@ -745,6 +745,112 @@ enum { https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L745 OSWB_MAX_DEPTH = 5, // up to 5 different classes }; +// Codegen for setting an instance variable. +// Preconditions: +// - receiver is in REG0 +// - receiver has the same class as CLASS_OF(comptime_receiver) +// - no stack push or pops to ctx since the entry to the codegen of the instruction being compiled +static codegen_status_t +gen_set_ivar(jitstate_t *jit, ctx_t *ctx, const int max_chain_depth, VALUE comptime_receiver, ID ivar_name, insn_opnd_t reg0_opnd, uint8_t *side_exit) +{ + VALUE comptime_val_klass = CLASS_OF(comptime_receiver); + const ctx_t starting_context = *ctx; // make a copy for use with jit_chain_guard + + // If the class uses the default allocator, instances should all be T_OBJECT + // NOTE: This assumes nobody changes the allocator of the class after allocation. + // Eventually, we can encode whether an object is T_OBJECT or not + // inside object shapes. + if (rb_get_alloc_func(comptime_val_klass) != rb_class_allocate_instance) { + GEN_COUNTER_INC(cb, setivar_not_object); + return YJIT_CANT_COMPILE; + } + RUBY_ASSERT(BUILTIN_TYPE(comptime_receiver) == T_OBJECT); // because we checked the allocator + + // ID for the name of the ivar + ID id = ivar_name; + struct rb_iv_index_tbl_entry *ent; + struct st_table *iv_index_tbl = ROBJECT_IV_INDEX_TBL(comptime_receiver); + + // Bail if this is a heap object, because this needs a write barrier + ADD_COMMENT(cb, "guard value is immediate"); + test(cb, REG1, imm_opnd(RUBY_IMMEDIATE_MASK)); + jz_ptr(cb, COUNTED_EXIT(side_exit, setivar_val_heapobject)); + + // Lookup index for the ivar the instruction loads + if (iv_index_tbl && rb_iv_index_tbl_lookup(iv_index_tbl, id, &ent)) { + uint32_t ivar_index = ent->index; + + x86opnd_t val_to_write = ctx_stack_pop(ctx, 1); + mov(cb, REG1, val_to_write); + + x86opnd_t flags_opnd = member_opnd(REG0, struct RBasic, flags); + + // Bail if this object is frozen + ADD_COMMENT(cb, "guard self is not frozen"); + test(cb, flags_opnd, imm_opnd(RUBY_FL_FREEZE)); + jz_ptr(cb, COUNTED_EXIT(side_exit, setivar_frozen)); + + // Pop receiver if it's on the temp stack + if (!reg0_opnd.is_self) { + (void)ctx_stack_pop(ctx, 1); + } + + // Compile time self is embedded and the ivar index lands within the object + if (RB_FL_TEST_RAW(comptime_receiver, ROBJECT_EMBED) && ivar_index < ROBJECT_EMBED_LEN_MAX) { + // See ROBJECT_IVPTR() from include/ruby/internal/core/robject.h + + // Guard that self is embedded + // TODO: BT and JC is shorter + ADD_COMMENT(cb, "guard embedded setivar"); + test(cb, flags_opnd, imm_opnd(ROBJECT_EMBED)); + jit_chain_guard(JCC_JZ, jit, &starting_context, max_chain_depth, side_exit); + + // Load the variable + x86opnd_t ivar_opnd = mem_opnd(64, REG0, offsetof(struct RObject, as.ary) + ivar_index * SIZEOF_VALUE); + + mov(cb, ivar_opnd, REG1); + + // Push the ivar on the stack + // For attr_writer we'll need to push the value on the stack + //x86opnd_t out_opnd = ctx_stack_push(ctx, TYPE_UNKNOWN); + } + else { + // Compile time value is *not* embeded. + + // Guard that value is *not* embedded + // See ROBJECT_IVPTR() from include/ruby/internal/core/robject.h + ADD_COMMENT(cb, "guard extended setivar"); + x86opnd_t flags_opnd = member_opnd(REG0, struct RBasic, flags); + test(cb, flags_opnd, imm_opnd(ROBJECT_EMBED)); + jit_chain_guard(JCC_JNZ, jit, &starting_context, max_chain_depth, side_exit); + + // check that the extended table is big enough + if (ivar_index >= ROBJECT_EMBED_LEN_MAX + 1) { + // Check that the slot is inside the extended table (num_slots > index) + x86opnd_t num_slots = mem_opnd(32, REG0, offsetof(struct RObject, as.heap.numiv)); + cmp(cb, num_slots, imm_opnd(ivar_index)); + jle_ptr(cb, COUNTED_EXIT(side_exit, setivar_idx_out_of_range)); + } + + // Get a pointer to the extended table + x86opnd_t tbl_opnd = mem_opnd(64, REG0, offsetof(struct RObject, as.heap.ivptr)); + mov(cb, REG0, tbl_opnd); + + // Write the ivar to the extended table + x86opnd_t ivar_opnd = mem_opnd(64, REG0, sizeof(VALUE) * ivar_index); + mov(cb, REG1, val_to_write); + mov(cb, ivar_opnd, REG1); + } + + // Jump to next instruction. This allows guard chains to share the same successor. + jit_jump_to_next_insn(jit, ctx); + return YJIT_END_BLOCK; + } + + GEN_COUNTER_INC(cb, setivar_name_not_mapped); + return YJIT_CANT_COMPILE; +} + // Codegen for getting an instance variable. // Preconditions: // - receiver is in REG0 @@ -867,7 +973,7 @@ gen_getinstancevariable(jitstate_t *jit, ctx_t *ctx) https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L973 // Guard that the receiver has the same class as the one from compile time. mov(cb, REG0, member_opnd(REG_CFP, rb_control_frame_t, self)); - guard_self_is_heap(cb, REG0, side_exit, ctx); + guard_self_is_heap(cb, REG0, COUNTED_EXIT(side_exit, getivar_se_self_not_heap), ctx); jit_guard_known_klass(jit, ctx, comptime_val_klass, OPND_SELF, GETIVAR_MAX_DEPTH, side_exit); @@ -877,69 +983,27 @@ gen_getinstancevariable(jitstate_t *jit, ctx_t *ctx) https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L983 static codegen_status_t gen_setinstancevariable(jitstate_t* jit, ctx_t* ctx) { - IVC ic = (IVC)jit_get_arg(jit, 1); - - // Check that the inline cache has been set, slot index is known - if (!ic->entry) { - return YJIT_CANT_COMPILE; + // Defer compilation so we can specialize on a runtime `self` + if (!jit_at_current_insn(jit)) { + defer_compilation(jit->block, jit->insn_idx, ctx); + return YJIT_END_BLOCK; } - // If the class uses the default allocator, instances should all be T_OBJECT - // NOTE: This assumes nobody changes the allocator of the class after allocation. - // Eventually, we can encode whether an object is T_OBJECT or not - // inside object shapes. - if (rb_get_alloc_func(ic->entry->class_value) != rb_class_allocate_instance) { - return YJIT_CANT_COMPILE; - } + ID ivar_name = (ID)jit_get_arg(jit, 0); - uint32_t ivar_index = ic->entry->index; + VALUE comptime_val = jit_peek_at_self(jit, ctx); + VALUE comptime_val_klass = CLASS_OF(comptime_val); - // Create a size-exit to fall back to the interpreter - uint8_t* side_exit = yjit_side_exit(jit, ctx); + // Generate a side exit + uint8_t *side_exit = yjit_side_exit(jit, ctx); - // Load self from CFP + // Guard that the receiver has the same class as the one from compile time. mov(cb, REG0, member_opnd(REG_CFP, rb_control_frame_t, self)); + guard_self_is_heap(cb, REG0, COUNTED_EXIT(side_exit, setivar_se_self_not_heap), ctx); - guard_self_is_heap(cb, REG0, side_exit, ctx); - - // Bail if receiver class is different from compiled time call cache class - x86opnd_t klass_opnd = mem_opnd(64, REG0, offsetof(struct RBasic, klass)); - mov(cb, REG1, klass_opnd); - x86opnd_t serial_opnd = mem_opnd(64, REG1, offsetof(struct RClass, class_serial)); - cmp(cb, serial_opnd, imm_opnd(ic (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/