ruby-changes:68691
From: Maxime <ko1@a...>
Date: Thu, 21 Oct 2021 08:12:25 +0900 (JST)
Subject: [ruby-changes:68691] 59eb98fbaa (master): Fix ujit cc/cme invalidation code for opt_send_without_block
https://git.ruby-lang.org/ruby.git/commit/?id=59eb98fbaa From 59eb98fbaa4762a935ef57a6777042068b186278 Mon Sep 17 00:00:00 2001 From: Maxime Chevalier-Boisvert <maxime.chevalierboisvert@s...> Date: Thu, 14 Jan 2021 16:58:20 -0500 Subject: Fix ujit cc/cme invalidation code for opt_send_without_block --- ujit.h | 2 +- ujit_codegen.c | 9 +++++---- ujit_core.c | 3 +++ ujit_iface.c | 49 ++++++++++++++++++++++++------------------------- ujit_iface.h | 2 +- version.c | 2 +- 6 files changed, 35 insertions(+), 32 deletions(-) diff --git a/ujit.h b/ujit.h index a4634ae2ce..a4a7437ada 100644 --- a/ujit.h +++ b/ujit.h @@ -41,7 +41,7 @@ bool rb_ujit_enabled_p(void) https://github.com/ruby/ruby/blob/trunk/ujit.h#L41 return rb_ujit_enabled; } -#define UJIT_CALL_THRESHOLD (1u) +#define UJIT_CALL_THRESHOLD (2u) void rb_ujit_method_lookup_change(VALUE cme_or_cc); void rb_ujit_compile_iseq(const rb_iseq_t *iseq); diff --git a/ujit_codegen.c b/ujit_codegen.c index 17a9447812..501254b2cc 100644 --- a/ujit_codegen.c +++ b/ujit_codegen.c @@ -676,6 +676,8 @@ cfunc_needs_frame(const rb_method_cfunc_t *cfunc) https://github.com/ruby/ruby/blob/trunk/ujit_codegen.c#L676 static bool gen_opt_send_without_block(jitstate_t* jit, ctx_t* ctx) { + //fprintf(stderr, "gen_opt_send_without_block\n"); + // Relevant definitions: // rb_execution_context_t : vm_core.h // invoker, cfunc logic : method.h, vm_method.c @@ -745,7 +747,7 @@ gen_opt_send_without_block(jitstate_t* jit, ctx_t* ctx) https://github.com/ruby/ruby/blob/trunk/ujit_codegen.c#L747 mov(cb, REG0, recv); // Callee method ID - //ID mid = vm_ci_mid(cd->ci); + ID mid = vm_ci_mid(cd->ci); //printf("JITting call to C function \"%s\", argc: %lu\n", rb_id2name(mid), argc); //print_str(cb, ""); //print_str(cb, "calling CFUNC:"); @@ -764,8 +766,7 @@ gen_opt_send_without_block(jitstate_t* jit, ctx_t* ctx) https://github.com/ruby/ruby/blob/trunk/ujit_codegen.c#L766 // Pointer to the klass field of the receiver &(recv->klass) x86opnd_t klass_opnd = mem_opnd(64, REG0, offsetof(struct RBasic, klass)); - // FIXME - //assume_method_lookup_stable(cd->cc, cme, ctx); + assume_method_lookup_stable(cd->cc, cme, jit->block); // Bail if receiver class is different from compile-time call cache class mov(cb, REG1, imm_opnd(cd->cc->klass)); @@ -1062,7 +1063,7 @@ ujit_init_codegen(void) https://github.com/ruby/ruby/blob/trunk/ujit_codegen.c#L1063 ujit_reg_op(BIN(opt_lt), gen_opt_lt, false); ujit_reg_op(BIN(opt_minus), gen_opt_minus, false); ujit_reg_op(BIN(opt_plus), gen_opt_plus, false); - //ujit_reg_op(BIN(opt_send_without_block), gen_opt_send_without_block); + ujit_reg_op(BIN(opt_send_without_block), gen_opt_send_without_block, false); ujit_reg_op(BIN(branchunless), gen_branchunless, true); ujit_reg_op(BIN(jump), gen_jump, true); } diff --git a/ujit_core.c b/ujit_core.c index 77c329970b..70306ce86b 100644 --- a/ujit_core.c +++ b/ujit_core.c @@ -331,9 +331,12 @@ void gen_branch( https://github.com/ruby/ruby/blob/trunk/ujit_core.c#L331 // Invalidate one specific block version void invalidate(block_t* block) { + fprintf(stderr, "invalidata block (%p, %d)\n", block->blockid.iseq, block->blockid.idx); + // Remove the version object from the map so we can re-generate stubs st_delete(version_tbl, (st_data_t*)&block->blockid, NULL); + // Get a pointer to the generated code for this block uint8_t* code_ptr = cb_get_ptr(cb, block->start_pos); // For each incoming branch diff --git a/ujit_iface.c b/ujit_iface.c index 85be128057..583326ce42 100644 --- a/ujit_iface.c +++ b/ujit_iface.c @@ -63,7 +63,6 @@ opcode_at_pc(const rb_iseq_t *iseq, const VALUE *pc) https://github.com/ruby/ruby/blob/trunk/ujit_iface.c#L63 } } -/* // GC root for interacting with the GC struct ujit_root_struct {}; @@ -74,20 +73,16 @@ static st_table *method_lookup_dependency; https://github.com/ruby/ruby/blob/trunk/ujit_iface.c#L73 struct compiled_region_array { int32_t size; int32_t capa; - struct compiled_region { - const rb_iseq_t *iseq; - size_t start_idx; - uint8_t *code; - } data[]; + block_t* data[]; }; // Add an element to a region array, or allocate a new region array. static struct compiled_region_array * -add_compiled_region(struct compiled_region_array *array, const rb_iseq_t *iseq, size_t start_idx, uint8_t *code) +add_compiled_region(struct compiled_region_array *array, block_t* block) { if (!array) { // Allocate a brand new array with space for one - array = malloc(sizeof(*array) + sizeof(struct compiled_region)); + array = malloc(sizeof(*array) + sizeof(block_t*)); if (!array) { return NULL; } @@ -99,7 +94,7 @@ add_compiled_region(struct compiled_region_array *array, const rb_iseq_t *iseq, https://github.com/ruby/ruby/blob/trunk/ujit_iface.c#L94 } // Check if the region is already present for (int32_t i = 0; i < array->size; i++) { - if (array->data[i].iseq == iseq && array->data[i].start_idx == start_idx) { + if (array->data[i] == block) { return array; } } @@ -110,7 +105,7 @@ add_compiled_region(struct compiled_region_array *array, const rb_iseq_t *iseq, https://github.com/ruby/ruby/blob/trunk/ujit_iface.c#L105 if (new_capa != double_capa) { return NULL; } - array = realloc(array, sizeof(*array) + new_capa * sizeof(struct compiled_region)); + array = realloc(array, sizeof(*array) + new_capa * sizeof(block_t*)); if (array == NULL) { return NULL; } @@ -118,9 +113,7 @@ add_compiled_region(struct compiled_region_array *array, const rb_iseq_t *iseq, https://github.com/ruby/ruby/blob/trunk/ujit_iface.c#L113 } int32_t size = array->size; - array->data[size].iseq = iseq; - array->data[size].start_idx = start_idx; - array->data[size].code = code; + array->data[size] = block; array->size++; return array; } @@ -128,12 +121,13 @@ add_compiled_region(struct compiled_region_array *array, const rb_iseq_t *iseq, https://github.com/ruby/ruby/blob/trunk/ujit_iface.c#L121 static int add_lookup_dependency_i(st_data_t *key, st_data_t *value, st_data_t data, int existing) { - ctx_t *ctx = (ctx_t *)data; + block_t *block = (block_t *)data; + struct compiled_region_array *regions = NULL; if (existing) { regions = (struct compiled_region_array *)*value; } - regions = add_compiled_region(regions, ctx->iseq, ctx->start_idx, ctx->code_ptr); + regions = add_compiled_region(regions, block); if (!regions) { rb_bug("ujit: failed to add method lookup dependency"); // TODO: we could bail out of compiling instead } @@ -143,10 +137,11 @@ add_lookup_dependency_i(st_data_t *key, st_data_t *value, st_data_t data, int ex https://github.com/ruby/ruby/blob/trunk/ujit_iface.c#L137 // Remember that the currently compiling region is only valid while cme and cc are valid void -assume_method_lookup_stable(const struct rb_callcache *cc, const rb_callable_method_entry_t *cme, ctx_t *ctx) +assume_method_lookup_stable(const struct rb_callcache *cc, const rb_callable_method_entry_t *cme, block_t* block) { - st_update(method_lookup_dependency, (st_data_t)cme, add_lookup_dependency_i, (st_data_t)ctx); - st_update(method_lookup_dependency, (st_data_t)cc, add_lookup_dependency_i, (st_data_t)ctx); + assert (block != NULL); + st_update(method_lookup_dependency, (st_data_t)cme, add_lookup_dependency_i, (st_data_t)block); + st_update(method_lookup_dependency, (st_data_t)cc, add_lookup_dependency_i, (st_data_t)block); // FIXME: This is a leak! When either the cme or the cc become invalid, the other also needs to go } @@ -158,7 +153,7 @@ ujit_root_mark_i(st_data_t k, st_data_t v, st_data_t ignore) https://github.com/ruby/ruby/blob/trunk/ujit_iface.c#L153 rb_gc_mark((VALUE)k); struct compiled_region_array *regions = (void *)v; for (int32_t i = 0; i < regions->size; i++) { - rb_gc_mark((VALUE)regions->data[i].iseq); + rb_gc_mark((VALUE)regions->data[i]->blockid.iseq); } return ST_CONTINUE; @@ -194,25 +189,29 @@ static const rb_data_type_t ujit_root_type = { https://github.com/ruby/ruby/blob/trunk/ujit_iface.c#L189 {ujit_root_mark, ujit_root_free, ujit_root_memsize, }, 0, 0, RUBY_TYPED_FREE_IMMEDIATELY }; -*/ // Callback when cme or cc become invalid void rb_ujit_method_lookup_change(VALUE cme_or_cc) { - /* - if (!method_lookup_dependency) return; + if (!method_lookup_dependency) + return; RUBY_ASSERT(IMEMO_TYPE_P(cme_or_cc, imemo_ment) || IMEMO_TYPE_P(cme_or_cc, imemo_callcache)); st_data_t image; if (st_lookup(method_lookup_dependency, (st_data_t)cme_or_cc, &image)) { struct compiled_region_array *array = (void *)image; + // Invalidate all regions that depend on the cme or cc for (int32_t i = 0; i < array->size; i++) { + block_t* block = array->data[i]; + + /* struct compiled_region *region = &array->data[i]; const struct rb_iseq_constant_body *body = region->iseq->body; RUBY_ASSERT((unsigned int)region->start_idx < body->iseq_size); + // Restore region address to interpreter address in bytecode sequence if (body->iseq_encoded[region->start_idx] == (VALUE)region->code) { const void *const *code_threading_table = rb_vm_get_insns_address_table(); @@ -222,11 +221,13 @@ rb_ujit_method_lookup_change(VALUE cme_or_cc) https://github.com/ruby/ruby/blob/trunk/ujit_iface.c#L221 fprintf(stderr, "cc_or_cme=%p now out of date. Restored idx=%u in iseq=%p\n", (void *)cme_or_cc, (unsigned)region->start_idx, (void *)region->iseq); } (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/