From 7c11e5a9d61530248aaddad05e9cdc8230f518a5 Mon Sep 17 00:00:00 2001 From: Nick Gasson Date: Sun, 4 Dec 2022 11:39:59 +0000 Subject: [PATCH] Make jit_lazy_compile and jit_get_func thread-safe --- src/cgen.c | 3 ++ src/jit/jit-core.c | 103 +++++++++++++++++++++++++++++++++------------ src/jit/jit-llvm.c | 13 +++--- src/jit/jit-llvm.h | 1 + src/jit/jit-priv.h | 2 + src/thread.c | 9 ++++ src/thread.h | 7 +++ 7 files changed, 106 insertions(+), 32 deletions(-) diff --git a/src/cgen.c b/src/cgen.c index 25cb396d..48e3e8c6 100644 --- a/src/cgen.c +++ b/src/cgen.c @@ -5609,6 +5609,9 @@ static void cgen_async_work(void *context, void *arg) llvm_obj_t *obj = llvm_obj_new(job->module_name); + if (job->index == 0) + llvm_add_abi_version(obj); + for (int i = 0; i < job->units.count; i++) { vcode_unit_t vu = job->units.items[i]; vcode_select_unit(vu); diff --git a/src/jit/jit-core.c b/src/jit/jit-core.c index 1f7c3657..26a88752 100644 --- a/src/jit/jit-core.c +++ b/src/jit/jit-core.c @@ -51,7 +51,8 @@ #include #endif -typedef A(jit_func_t *) func_array_t; +#define FUNC_HASH_SZ 1024 +#define FUNC_LIST_SZ 512 typedef struct _jit_tier { jit_tier_t *next; @@ -60,9 +61,13 @@ typedef struct _jit_tier { void *context; } jit_tier_t; +typedef struct { + size_t length; + jit_func_t *items[0]; +} func_array_t; + typedef struct _jit { - func_array_t funcs; - hash_t *index; + chash_t *index; mspace_t *mspace; jit_lower_fn_t lower_fn; void *lower_ctx; @@ -73,6 +78,9 @@ typedef struct _jit { int exit_status; jit_tier_t *tiers; jit_dll_t *aotlib; + func_array_t *funcs; + unsigned next_handle; + nvc_lock_t lock; } jit_t; static void jit_oom_cb(mspace_t *m, size_t size) @@ -104,9 +112,13 @@ jit_thread_local_t *jit_thread_local(void) jit_t *jit_new(void) { jit_t *j = xcalloc(sizeof(jit_t)); - j->index = hash_new(256); + j->index = chash_new(FUNC_HASH_SZ); j->mspace = mspace_new(opt_get_int(OPT_HEAP_SIZE)); + j->funcs = xcalloc_flex(sizeof(func_array_t), + FUNC_LIST_SZ, sizeof(jit_func_t *)); + j->funcs->length = FUNC_LIST_SZ; + mspace_set_oom_handler(j->mspace, jit_oom_cb); // Ensure we can resolve symbols from the executable @@ -130,9 +142,9 @@ void jit_free(jit_t *j) if (j->aotlib != NULL) ffi_unload_dll(j->aotlib); - for (int i = 0; i < j->funcs.count; i++) - jit_free_func(j->funcs.items[i]); - ACLEAR(j->funcs); + for (int i = 0; i < j->next_handle; i++) + jit_free_func(j->funcs->items[i]); + free(j->funcs); if (j->layouts != NULL) { hash_iter_t it = HASH_BEGIN; @@ -150,7 +162,7 @@ void jit_free(jit_t *j) } mspace_destroy(j->mspace); - hash_free(j->index); + chash_free(j->index); free(j); } @@ -159,12 +171,44 @@ mspace_t *jit_get_mspace(jit_t *j) return j->mspace; } +static void jit_install(jit_t *j, jit_func_t *f) +{ + assert_lock_held(&(j->lock)); + + if (f->handle >= j->funcs->length) { + const size_t newlen = MAX(f->handle + 1, j->funcs->length * 2); + func_array_t *new = xcalloc_flex(sizeof(func_array_t), + newlen, sizeof(jit_func_t *)); + new->length = newlen; + for (size_t i = 0; i < j->funcs->length; i++) + new->items[i] = j->funcs->items[i]; + + // Synchronises with load_acquire in jit_get_func + store_release(&(j->funcs), new); + + // XXX: the old list leaks here: it should be cleaned up when + // no more threads can reference it + } + + // Synchronises with load_acquire in jit_get_func + store_release(&(j->funcs->items[f->handle]), f); + + chash_put(j->index, f->name, f); + if (f->unit) chash_put(j->index, f->unit, f); +} + jit_handle_t jit_lazy_compile(jit_t *j, ident_t name) { - jit_func_t *f = hash_get(j->index, name); + jit_func_t *f = chash_get(j->index, name); if (f != NULL) return f->handle; + SCOPED_LOCK(j->lock); + + // Second check with lock held + if ((f = chash_get(j->index, name))) + return f->handle; + void *symbol = NULL; if (j->aotlib != NULL) { LOCAL_TEXT_BUF tb = safe_symbol(name); @@ -200,7 +244,7 @@ jit_handle_t jit_lazy_compile(jit_t *j, ident_t name) if (vu == NULL && symbol == NULL) return JIT_HANDLE_INVALID; - assert(vu == NULL || hash_get(j->index, vu) == NULL); + assert(vu == NULL || chash_get(j->index, vu) == NULL); f = xcalloc(sizeof(jit_func_t)); @@ -208,26 +252,33 @@ jit_handle_t jit_lazy_compile(jit_t *j, ident_t name) f->unit = vu; f->symbol = symbol; f->jit = j; - f->handle = j->funcs.count; + f->handle = j->next_handle++; f->next_tier = j->tiers; f->hotness = f->next_tier ? f->next_tier->threshold : 0; f->entry = jit_interp; f->object = vu ? vcode_unit_object(vu) : NULL; - if (vu) hash_put(j->index, vu, f); - hash_put(j->index, name, f); + jit_install(j, f); if (alias != NULL && alias != name) - hash_put(j->index, alias, f); + chash_put(j->index, alias, f); - APUSH(j->funcs, f); return f->handle; } jit_func_t *jit_get_func(jit_t *j, jit_handle_t handle) { assert(handle != JIT_HANDLE_INVALID); - return AGET(j->funcs, handle); + + // Synchronises with store_release in jit_install + func_array_t *list = load_acquire(&(j->funcs)); + assert(handle < list->length); + + // Synchronises with store_release in jit_install + jit_func_t *f = load_acquire(&(list->items[handle])); + if (f == NULL) printf("get func %d is null\n", handle); + assert(f != NULL); + return f; } jit_handle_t jit_compile(jit_t *j, ident_t name) @@ -247,16 +298,18 @@ void jit_register(jit_t *j, ident_t name, jit_entry_fn_t fn, const uint8_t *debug, size_t bufsz, object_t *obj, ffi_spec_t spec) { - jit_func_t *f = hash_get(j->index, name); + jit_func_t *f = chash_get(j->index, name); if (f != NULL) fatal_trace("attempt to register existing function %s", istr(name)); + SCOPED_LOCK(j->lock); + f = xcalloc(sizeof(jit_func_t)); f->name = name; f->unit = vcode_find_unit(f->name); f->jit = j; - f->handle = j->funcs.count; + f->handle = j->next_handle++; f->next_tier = j->tiers; f->hotness = f->next_tier ? f->next_tier->threshold : 0; f->entry = fn; @@ -319,10 +372,7 @@ void jit_register(jit_t *j, ident_t name, jit_entry_fn_t fn, } assert(pos == bufsz); - if (f->unit) hash_put(j->index, f->unit, f); - hash_put(j->index, f->name, f); - - APUSH(j->funcs, f); + jit_install(j, f); } void *jit_link(jit_t *j, jit_handle_t handle) @@ -1004,21 +1054,22 @@ ident_t jit_get_name(jit_t *j, jit_handle_t handle) jit_handle_t jit_assemble(jit_t *j, ident_t name, const char *text) { - jit_func_t *f = hash_get(j->index, name); + jit_func_t *f = chash_get(j->index, name); if (f != NULL) return f->handle; + SCOPED_LOCK(j->lock); + f = xcalloc(sizeof(jit_func_t)); f->name = name; f->jit = j; - f->handle = j->funcs.count; + f->handle = j->next_handle++; f->next_tier = j->tiers; f->hotness = f->next_tier ? f->next_tier->threshold : 0; f->entry = jit_interp; - hash_put(j->index, name, f); - APUSH(j->funcs, f); + jit_install(j, f); enum { LABEL, INS, CCSIZE, RESULT, ARG1, ARG2, NEWLINE } state = LABEL; diff --git a/src/jit/jit-llvm.c b/src/jit/jit-llvm.c index 384280f3..301ace84 100644 --- a/src/jit/jit-llvm.c +++ b/src/jit/jit-llvm.c @@ -2304,6 +2304,11 @@ llvm_obj_t *llvm_obj_new(const char *name) LLVMConstArray(obj->types[LLVM_CTOR], ctors, CTOR_MAX_ORDER); LLVMSetInitializer(global, array); + return obj; +} + +void llvm_add_abi_version(llvm_obj_t *obj) +{ LLVMValueRef abi_version = LLVMAddGlobal(obj->module, obj->types[LLVM_INT32], "__nvc_abi_version"); LLVMSetInitializer(abi_version, llvm_int32(obj, RT_ABI_VERSION)); @@ -2311,8 +2316,6 @@ llvm_obj_t *llvm_obj_new(const char *name) #ifdef IMPLIB_REQUIRED LLVMSetDLLStorageClass(abi_version, LLVMDLLExportStorageClass); #endif - - return obj; } void llvm_aot_compile(llvm_obj_t *obj, jit_t *j, jit_handle_t handle) @@ -2334,10 +2337,8 @@ void llvm_aot_compile(llvm_obj_t *obj, jit_t *j, jit_handle_t handle) cgen_function(obj, &func); const uint64_t end_us = get_timestamp_us(); - static __thread uint64_t slowest = 0; - if (end_us - start_us > slowest) - debugf("compiled %s [%"PRIi64" us]", func.name, - (slowest = end_us - start_us)); + debugf("thread %d compiled %s [%"PRIi64" us]", thread_id(), + func.name, end_us - start_us); free(func.name); } diff --git a/src/jit/jit-llvm.h b/src/jit/jit-llvm.h index b5585620..12d0e504 100644 --- a/src/jit/jit-llvm.h +++ b/src/jit/jit-llvm.h @@ -28,6 +28,7 @@ void jit_register_llvm_plugin(jit_t *j); typedef struct _llvm_obj llvm_obj_t; llvm_obj_t *llvm_obj_new(const char *name); +void llvm_add_abi_version(llvm_obj_t *obj); void llvm_aot_compile(llvm_obj_t *obj, jit_t *j, jit_handle_t handle); void llvm_obj_emit(llvm_obj_t *obj, const char *path); diff --git a/src/jit/jit-priv.h b/src/jit/jit-priv.h index 787399ee..fb150c21 100644 --- a/src/jit/jit-priv.h +++ b/src/jit/jit-priv.h @@ -23,6 +23,7 @@ #include "jit/jit-ffi.h" #include "mask.h" #include "rt/mspace.h" +#include "thread.h" #include #include @@ -235,6 +236,7 @@ typedef struct { typedef struct _jit_func { jit_entry_fn_t entry; // Must be first + nvc_lock_t lock; jit_t *jit; vcode_unit_t unit; ident_t name; diff --git a/src/thread.c b/src/thread.c index fcdae1d5..4462730d 100644 --- a/src/thread.c +++ b/src/thread.c @@ -509,6 +509,15 @@ void nvc_unlock(nvc_lock_t *lock) TSAN_POST_UNLOCK(lock); } +#ifdef DEBUG +void assert_lock_held(nvc_lock_t *lock) +{ + int8_t state = relaxed_load(lock); + if (unlikely(!(state & IS_LOCKED))) + fatal_trace("expected lock at %p to be held", lock); +} +#endif + void __scoped_unlock(nvc_lock_t **plock) { nvc_unlock(*plock); diff --git a/src/thread.h b/src/thread.h index c3158f04..b08903ee 100644 --- a/src/thread.h +++ b/src/thread.h @@ -36,6 +36,7 @@ }) #define relaxed_add(p, n) __atomic_add_fetch((p), (n), __ATOMIC_RELAXED) +#define relaxed_fetch_add(p, n) __atomic_fetch_add((p), (n), __ATOMIC_RELAXED) #define relaxed_load(p) __atomic_load_n((p), __ATOMIC_RELAXED) #define relaxed_store(p, v) __atomic_store_n((p), (v), __ATOMIC_RELAXED) @@ -62,6 +63,12 @@ typedef int8_t nvc_lock_t; void nvc_lock(nvc_lock_t *lock); void nvc_unlock(nvc_lock_t *lock); +#ifdef DEBUG +void assert_lock_held(nvc_lock_t *lock); +#else +#define assert_lock_held(lock) +#endif + void __scoped_unlock(nvc_lock_t **plock); #define SCOPED_LOCK(lock) \ -- 2.39.2