From eed92f90e2da803ffb59f7f3504a234aa53daff4 Mon Sep 17 00:00:00 2001 From: Nick Gasson Date: Sun, 4 Dec 2022 15:27:38 +0000 Subject: [PATCH] Make jit_irgen thread-safe --- src/cgen.c | 2 +- src/jit/jit-core.c | 17 ++++++----- src/jit/jit-exits.c | 8 ----- src/jit/jit-interp.c | 2 +- src/jit/jit-irgen.c | 71 +++++++++++++++++++++++++++++++++++--------- src/jit/jit-llvm.c | 12 ++++---- src/jit/jit-priv.h | 9 ++++-- src/thread.c | 6 ++++ src/thread.h | 1 + 9 files changed, 90 insertions(+), 38 deletions(-) diff --git a/src/cgen.c b/src/cgen.c index 48e3e8c6..70f4c3c5 100644 --- a/src/cgen.c +++ b/src/cgen.c @@ -78,7 +78,7 @@ typedef struct { static A(char *) link_args; static A(char *) cleanup_files = AINIT; -#define UNITS_PER_JOB 1000 +#define UNITS_PER_JOB 25 #if !CGEN_USE_JIT diff --git a/src/jit/jit-core.c b/src/jit/jit-core.c index 26a88752..5e43667b 100644 --- a/src/jit/jit-core.c +++ b/src/jit/jit-core.c @@ -249,6 +249,7 @@ jit_handle_t jit_lazy_compile(jit_t *j, ident_t name) f = xcalloc(sizeof(jit_func_t)); f->name = alias ?: name; + f->state = symbol ? JIT_FUNC_READY : JIT_FUNC_PLACEHOLDER; f->unit = vu; f->symbol = symbol; f->jit = j; @@ -261,7 +262,7 @@ jit_handle_t jit_lazy_compile(jit_t *j, ident_t name) jit_install(j, f); if (alias != NULL && alias != name) - chash_put(j->index, alias, f); + chash_put(j->index, name, f); return f->handle; } @@ -288,7 +289,7 @@ jit_handle_t jit_compile(jit_t *j, ident_t name) return handle; jit_func_t *f = jit_get_func(j, handle); - if (f->irbuf == NULL && f->symbol == NULL && f->entry == jit_interp) + if (load_acquire(&(f->state)) != JIT_FUNC_READY) jit_irgen(f); return handle; @@ -307,6 +308,7 @@ void jit_register(jit_t *j, ident_t name, jit_entry_fn_t fn, f = xcalloc(sizeof(jit_func_t)); f->name = name; + f->state = JIT_FUNC_READY; f->unit = vcode_find_unit(f->name); f->jit = j; f->handle = j->next_handle++; @@ -666,7 +668,7 @@ static bool jit_try_vcall(jit_t *j, jit_func_t *f, jit_scalar_t *result, static void jit_unpack_args(jit_func_t *f, jit_scalar_t *args, va_list ap) { - if (f->symbol == NULL && f->irbuf == NULL && f->entry == jit_interp) + if (load_acquire(&(f->state)) != JIT_FUNC_READY) jit_irgen(f); // Ensure FFI spec is set const int nargs = ffi_count_args(f->spec); @@ -736,7 +738,7 @@ bool jit_try_call_packed(jit_t *j, jit_handle_t handle, jit_scalar_t context, { jit_func_t *f = jit_get_func(j, handle); - if (f->symbol == NULL && f->irbuf == NULL && f->entry == jit_interp) + if (load_acquire(&(f->state)) != JIT_FUNC_READY) jit_irgen(f); // Ensure FFI spec is set assert(f->spec != 0); @@ -1063,6 +1065,7 @@ jit_handle_t jit_assemble(jit_t *j, ident_t name, const char *text) f = xcalloc(sizeof(jit_func_t)); f->name = name; + f->state = JIT_FUNC_READY; f->jit = j; f->handle = j->next_handle++; f->next_tier = j->tiers; @@ -1102,8 +1105,8 @@ jit_handle_t jit_assemble(jit_t *j, ident_t name, const char *text) { "C", JIT_CC_C }, }; - f->bufsz = 128; - f->irbuf = xcalloc_array(f->bufsz, sizeof(jit_ir_t)); + const unsigned bufsz = 128; + f->irbuf = xcalloc_array(bufsz, sizeof(jit_ir_t)); SCOPED_A(int) lpatch = AINIT; int *labels LOCAL = NULL, maxlabel = 0; @@ -1114,7 +1117,7 @@ jit_handle_t jit_assemble(jit_t *j, ident_t name, const char *text) again: switch (state) { case LABEL: - assert(f->nirs < f->bufsz); + assert(f->nirs < bufsz); ir = &(f->irbuf[f->nirs++]); if (tok[0] == 'L' && isdigit((int)tok[1])) { diff --git a/src/jit/jit-exits.c b/src/jit/jit-exits.c index 25bc9ebb..e3e49134 100644 --- a/src/jit/jit-exits.c +++ b/src/jit/jit-exits.c @@ -1479,9 +1479,6 @@ DLLEXPORT void __nvc_register(const char *name, jit_entry_fn_t fn, const uint8_t *debug, int32_t bufsz, object_t *obj, ffi_spec_t spec) { - printf("register! name=%s fn=%p bufsz=%d obj=%p spec=%lx\n", - name, fn, bufsz, obj, spec); - jit_t *j = jit_thread_local()->jit; jit_register(j, ident_new(name), fn, debug, bufsz, obj, spec); } @@ -1489,23 +1486,18 @@ void __nvc_register(const char *name, jit_entry_fn_t fn, const uint8_t *debug, DLLEXPORT jit_func_t *__nvc_get_func(const char *name) { - printf("get_func %s\n", name); - jit_t *j = jit_thread_local()->jit; jit_handle_t handle = jit_lazy_compile(j, ident_new(name)); if (handle == JIT_HANDLE_INVALID) fatal_trace("invalid function %s", name); - printf("...handle = %d\n", handle); - return jit_get_func(j, handle); } DLLEXPORT jit_foreign_t *__nvc_get_foreign(const char *name, ffi_spec_t spec) { - printf("get_foreign %s %lx\n", name, spec); ident_t id = ident_new(name); return jit_ffi_get(id) ?: jit_ffi_bind(id, spec, NULL); } diff --git a/src/jit/jit-interp.c b/src/jit/jit-interp.c index 98a3f807..41d8ccc9 100644 --- a/src/jit/jit-interp.c +++ b/src/jit/jit-interp.c @@ -843,7 +843,7 @@ void jit_interp(jit_func_t *f, jit_anchor_t *caller, jit_scalar_t *args) return; } - if (f->irbuf == NULL) + if (load_acquire(&(f->state)) != JIT_FUNC_READY || f->symbol /* XXX */) jit_irgen(f); if (f->next_tier && --(f->hotness) <= 0) diff --git a/src/jit/jit-irgen.c b/src/jit/jit-irgen.c index 3aedae5f..b2cebbf2 100644 --- a/src/jit/jit-irgen.c +++ b/src/jit/jit-irgen.c @@ -58,6 +58,7 @@ typedef struct { size_t cpoolptr; jit_value_t statereg; vcode_reg_t flags; + unsigned bufsz; } jit_irgen_t; //////////////////////////////////////////////////////////////////////////////// @@ -181,14 +182,15 @@ static jit_value_t jit_value_from_foreign(jit_foreign_t *ff) return (jit_value_t){ .kind = JIT_VALUE_FOREIGN, .foreign = ff }; } -static jit_ir_t *irgen_append(jit_func_t *f) +static jit_ir_t *irgen_append(jit_irgen_t *g) { - if (f->nirs == f->bufsz) { - f->bufsz = MAX(f->bufsz * 2, 1024); - f->irbuf = xrealloc_array(f->irbuf, f->bufsz, sizeof(jit_ir_t)); + if (g->func->nirs == g->bufsz) { + g->bufsz = MAX(g->bufsz * 2, 1024); + g->func->irbuf = xrealloc_array(g->func->irbuf, g->bufsz, + sizeof(jit_ir_t)); } - return &(f->irbuf[f->nirs++]); + return &(g->func->irbuf[g->func->nirs++]); } static jit_reg_t irgen_alloc_reg(jit_irgen_t *g) @@ -243,7 +245,7 @@ static jit_cc_t irgen_get_jit_cc(vcode_cmp_t cmp) static void irgen_emit_nullary(jit_irgen_t *g, jit_op_t op, jit_cc_t cc, jit_reg_t result) { - jit_ir_t *ir = irgen_append(g->func); + jit_ir_t *ir = irgen_append(g); ir->op = op; ir->size = JIT_SZ_UNSPEC; ir->cc = cc; @@ -257,7 +259,7 @@ static jit_ir_t *irgen_emit_unary(jit_irgen_t *g, jit_op_t op, jit_size_t sz, jit_cc_t cc, jit_reg_t result, jit_value_t arg) { - jit_ir_t *ir = irgen_append(g->func); + jit_ir_t *ir = irgen_append(g); ir->op = op; ir->size = sz; ir->cc = cc; @@ -273,7 +275,7 @@ static void irgen_emit_binary(jit_irgen_t *g, jit_op_t op, jit_size_t sz, jit_cc_t cc, jit_reg_t result, jit_value_t arg1, jit_value_t arg2) { - jit_ir_t *ir = irgen_append(g->func); + jit_ir_t *ir = irgen_append(g); ir->op = op; ir->size = sz; ir->cc = cc; @@ -1851,13 +1853,22 @@ static void irgen_op_var_upref(jit_irgen_t *g, int op) } // TODO: maybe we should cache these somewhere? - jit_handle_t handle = jit_compile(g->func->jit, vcode_unit_name()); + jit_handle_t handle = jit_lazy_compile(g->func->jit, vcode_unit_name()); jit_func_t *cf = jit_get_func(g->func->jit, handle); + // Handle potential circular dependency + // TODO: it would be better to avoid this entirely + const unsigned *varoff = load_acquire(&(cf->varoff)); + if (varoff == NULL) { + jit_irgen(cf); + varoff = load_acquire(&(cf->varoff)); + assert(varoff); + } + vcode_state_restore(&state); assert(address < cf->nvars); - const int offset = cf->varoff[address]; + const int offset = varoff[address]; g->map[vcode_get_result(op)] = jit_addr_from_value(context, offset); } @@ -3490,8 +3501,8 @@ static void irgen_locals(jit_irgen_t *g) { const int nvars = g->func->nvars = vcode_count_vars(); + unsigned *varoff = xmalloc_array(nvars, sizeof(unsigned)); g->vars = xmalloc_array(nvars, sizeof(jit_value_t)); - g->func->varoff = xmalloc_array(nvars, sizeof(unsigned)); bool on_stack = true; const vunit_kind_t kind = vcode_unit_kind(); @@ -3517,7 +3528,7 @@ static void irgen_locals(jit_irgen_t *g) const int align = irgen_align_of(vtype); sz = ALIGN_UP(sz, align); g->vars[i] = jit_value_from_frame_addr(sz); - g->func->varoff[i] = sz; + varoff[i] = sz; sz += irgen_size_bytes(vtype); } @@ -3534,7 +3545,7 @@ static void irgen_locals(jit_irgen_t *g) vcode_type_t vtype = vcode_var_type(i); const int align = irgen_align_of(vtype); sz = ALIGN_UP(sz, align); - g->func->varoff[i] = sz; + varoff[i] = sz; sz += irgen_size_bytes(vtype); } @@ -3547,8 +3558,12 @@ static void irgen_locals(jit_irgen_t *g) g->statereg = mem; for (int i = 0; i < nvars; i++) - g->vars[i] = jit_addr_from_value(g->statereg, g->func->varoff[i]); + g->vars[i] = jit_addr_from_value(g->statereg, varoff[i]); } + + // Publish the variable offset table early to handle circular references + // This may be read by another thread while in the COMPILING state + store_release(&(g->func->varoff), varoff); } static void irgen_params(jit_irgen_t *g, int first) @@ -3640,8 +3655,33 @@ static bool irgen_is_procedure(void) } } +static bool irgen_enter(jit_func_t *f) +{ + switch (load_acquire(&(f->state))) { + case JIT_FUNC_READY: + return f->symbol != NULL; // XXX: should be false + case JIT_FUNC_PLACEHOLDER: + if (atomic_cas(&(f->state), JIT_FUNC_PLACEHOLDER, JIT_FUNC_COMPILING)) + return true; + // Fall-through + case JIT_FUNC_COMPILING: + // Another thread is compiling this function + for (int timeout = 10000; load_acquire(&(f->state)) != JIT_FUNC_READY; ) { + if (timeout-- == 0) + fatal_trace("timeout waiting for %s", istr(f->name)); + thread_sleep(100); + } + return false; + default: + fatal_trace("illegal function state for %s", istr(f->name)); + } +} + void jit_irgen(jit_func_t *f) { + if (!irgen_enter(f)) + return; + assert(f->irbuf == NULL); vcode_select_unit(f->unit); @@ -3723,6 +3763,9 @@ void jit_irgen(jit_func_t *f) jit_do_lvn(f); jit_free_cfg(f); + // Function can be executed immediately after this store + store_release(&(f->state), JIT_FUNC_READY); + if (opt_get_verbose(OPT_JIT_VERBOSE, istr(f->name))) { #ifdef DEBUG jit_dump_interleaved(f); diff --git a/src/jit/jit-llvm.c b/src/jit/jit-llvm.c index 301ace84..2b47dd3f 100644 --- a/src/jit/jit-llvm.c +++ b/src/jit/jit-llvm.c @@ -2320,12 +2320,12 @@ void llvm_add_abi_version(llvm_obj_t *obj) void llvm_aot_compile(llvm_obj_t *obj, jit_t *j, jit_handle_t handle) { + DEBUG_ONLY(const uint64_t start_us = get_timestamp_us()); + jit_func_t *f = jit_get_func(j, handle); - if (f->irbuf == NULL) + if (load_acquire(&(f->state)) != JIT_FUNC_READY) jit_irgen(f); - const uint64_t start_us = get_timestamp_us(); - LOCAL_TEXT_BUF tb = tb_new(); tb_istr(tb, f->name); @@ -2336,9 +2336,11 @@ void llvm_aot_compile(llvm_obj_t *obj, jit_t *j, jit_handle_t handle) cgen_function(obj, &func); +#ifdef DEBUG const uint64_t end_us = get_timestamp_us(); - debugf("thread %d compiled %s [%"PRIi64" us]", thread_id(), - func.name, end_us - start_us); + if (end_us - start_us > 100000) + debugf("compiled %s [%"PRIi64" us]", func.name, end_us - start_us); +#endif free(func.name); } diff --git a/src/jit/jit-priv.h b/src/jit/jit-priv.h index fb150c21..270c7461 100644 --- a/src/jit/jit-priv.h +++ b/src/jit/jit-priv.h @@ -234,9 +234,15 @@ typedef struct { jit_block_t blocks[0]; } jit_cfg_t; +typedef enum { + JIT_FUNC_PLACEHOLDER, + JIT_FUNC_COMPILING, + JIT_FUNC_READY +} func_state_t; + typedef struct _jit_func { jit_entry_fn_t entry; // Must be first - nvc_lock_t lock; + func_state_t state; jit_t *jit; vcode_unit_t unit; ident_t name; @@ -246,7 +252,6 @@ typedef struct _jit_func { unsigned char *cpool; unsigned framesz; unsigned nirs; - unsigned bufsz; unsigned nregs; unsigned nvars; unsigned cpoolsz; diff --git a/src/thread.c b/src/thread.c index 4462730d..740648f1 100644 --- a/src/thread.c +++ b/src/thread.c @@ -321,6 +321,12 @@ int thread_id(void) return my_thread->id; } +void thread_sleep(int usec) +{ + usleep(usec); + // TODO: thread_poll here +} + static void *thread_wrapper(void *arg) { assert(my_thread == NULL); diff --git a/src/thread.h b/src/thread.h index b08903ee..f0387fa5 100644 --- a/src/thread.h +++ b/src/thread.h @@ -47,6 +47,7 @@ typedef struct _nvc_thread nvc_thread_t; void thread_init(void); int thread_id(void); +void thread_sleep(int usec); typedef void *(*thread_fn_t)(void *); -- 2.39.2