From ccc2f697f784203d5cffe49732aadbfd403e8b23 Mon Sep 17 00:00:00 2001 From: Nick Gasson Date: Mon, 5 Dec 2022 14:21:57 +0000 Subject: [PATCH] Refactor mptr_t interface --- src/cgen.c | 2 +- src/jit/jit-core.c | 10 ++-- src/rt/model.c | 12 ++--- src/rt/mspace.c | 121 ++++++++++++++++++++++++++------------------- src/rt/mspace.h | 7 ++- test/test_mspace.c | 8 +-- 6 files changed, 88 insertions(+), 72 deletions(-) diff --git a/src/cgen.c b/src/cgen.c index 70f4c3c5..060b3e40 100644 --- a/src/cgen.c +++ b/src/cgen.c @@ -374,7 +374,7 @@ static LLVMTypeRef llvm_tlab_type(void) llvm_void_ptr(), // Base pointer llvm_void_ptr(), // Allocation pointer llvm_void_ptr(), // Limit pointer - llvm_int32_type(), // Mptr object + llvm_void_ptr(), // Mptr object }; return LLVMStructTypeInContext(llvm_context(), struct_elems, ARRAY_LEN(struct_elems), false); diff --git a/src/jit/jit-core.c b/src/jit/jit-core.c index d159d7d1..fa07b82b 100644 --- a/src/jit/jit-core.c +++ b/src/jit/jit-core.c @@ -383,7 +383,7 @@ void *jit_link(jit_t *j, jit_handle_t handle) jit_func_t *f = jit_get_func(j, handle); if (f->privdata != MPTR_INVALID) - return mptr_get(j->mspace, f->privdata); + return *mptr_get(f->privdata); f->privdata = mptr_new(j->mspace, "privdata"); @@ -406,7 +406,7 @@ void *jit_link(jit_t *j, jit_handle_t handle) vcode_state_restore(&state); // Initialisation should save the context pointer - assert(result.pointer == mptr_get(j->mspace, f->privdata)); + assert(result.pointer == *mptr_get(f->privdata)); return result.pointer; } @@ -416,13 +416,13 @@ void *jit_get_privdata(jit_t *j, jit_func_t *f) if (f->privdata == MPTR_INVALID) f->privdata = mptr_new(j->mspace, "privdata"); - return mptr_get(j->mspace, f->privdata); + return *mptr_get(f->privdata); } void jit_put_privdata(jit_t *j, jit_func_t *f, void *ptr) { assert(f->privdata != MPTR_INVALID); - mptr_put(j->mspace, f->privdata, ptr); + *mptr_get(f->privdata) = ptr; } void *jit_get_frame_var(jit_t *j, jit_handle_t handle, uint32_t var) @@ -432,7 +432,7 @@ void *jit_get_frame_var(jit_t *j, jit_handle_t handle, uint32_t var) fatal_trace("%s not linked", istr(f->name)); assert(var < f->nvars); - return (char *)mptr_get(j->mspace, f->privdata) + f->varoff[var]; + return *mptr_get(f->privdata) + f->varoff[var]; } static void jit_emit_trace(diag_t *d, const loc_t *loc, tree_t enclosing, diff --git a/src/rt/model.c b/src/rt/model.c index 941122be..e94d0dc1 100644 --- a/src/rt/model.c +++ b/src/rt/model.c @@ -788,13 +788,13 @@ static void reset_process(rt_model_t *m, rt_proc_t *proc) active_scope = proc->scope; jit_scalar_t context = { - .pointer = mptr_get(m->mspace, proc->scope->privdata) + .pointer = *mptr_get(proc->scope->privdata) }; jit_scalar_t state = { .pointer = NULL }; jit_scalar_t result; if (jit_fastcall(m->jit, proc->handle, &result, state, context, NULL)) - mptr_put(m->mspace, proc->privdata, result.pointer); + *mptr_get(proc->privdata) = result.pointer; else m->force_stop = true; } @@ -823,12 +823,12 @@ static void run_process(rt_model_t *m, rt_proc_t *proc) // Stateless processes have NULL privdata so pass a dummy pointer // value in so it can be distinguished from a reset jit_scalar_t state = { - .pointer = mptr_get(m->mspace, proc->privdata) ?: (void *)-1 + .pointer = *mptr_get(proc->privdata) ?: (void *)-1 }; jit_scalar_t result; jit_scalar_t context = { - .pointer = mptr_get(m->mspace, proc->scope->privdata) + .pointer = *mptr_get(proc->scope->privdata) }; if (!jit_fastcall(m->jit, proc->handle, &result, state, context, tlab)) @@ -870,10 +870,10 @@ static void reset_scope(rt_model_t *m, rt_scope_t *s) jit_scalar_t p2 = { .integer = 0 }; if (s->parent != NULL) - context.pointer = mptr_get(m->mspace, s->parent->privdata); + context.pointer = *mptr_get(s->parent->privdata); if (jit_fastcall(m->jit, handle, &result, context, p2, NULL)) - mptr_put(m->mspace, s->privdata, result.pointer); + *mptr_get(s->privdata) = result.pointer; else { m->force_stop = true; return; diff --git a/src/rt/mspace.c b/src/rt/mspace.c index 9c46a4b7..bdce1dd1 100644 --- a/src/rt/mspace.c +++ b/src/rt/mspace.c @@ -41,10 +41,16 @@ #define LINE_SIZE 32 #define LINE_WORDS (LINE_SIZE / sizeof(intptr_t)) -typedef A(mptr_t) mptr_list_t; -typedef A(void *) root_list_t; typedef A(uint64_t) work_list_t; -typedef A(const char *) str_list_t; + +struct _mptr { + void *ptr; + mptr_t prev; + mptr_t next; +#ifdef DEBUG + const char *name; +#endif +}; typedef struct { bit_mask_t markmask; @@ -64,15 +70,14 @@ struct _mspace { unsigned maxlines; char *space; bit_mask_t headmask; - root_list_t roots; - mptr_list_t free_mptrs; + mptr_t roots; + mptr_t free_mptrs; mspace_oom_fn_t oomfn; free_list_t *free_list; uint64_t create_us; unsigned total_gc; unsigned num_cycles; #ifdef DEBUG - str_list_t mptr_names; bool stress; #endif }; @@ -97,9 +102,6 @@ mspace_t *mspace_new(size_t size) mask_init(&(m->headmask), m->maxlines); mask_setall(&(m->headmask)); - APUSH(m->roots, NULL); // Dummy MPTR_INVALID root - DEBUG_ONLY(APUSH(m->mptr_names, NULL)); - free_list_t *f = xmalloc(sizeof(free_list_t)); f->next = NULL; f->ptr = m->space; @@ -113,15 +115,13 @@ mspace_t *mspace_new(size_t size) void mspace_destroy(mspace_t *m) { -#ifndef NDEBUG - if (m->free_mptrs.count != m->roots.count - 1) { +#ifdef DEBUG + if (m->roots != NULL) { LOCAL_TEXT_BUF tb = tb_new(); - for (int i = 0, n = 0; i < m->mptr_names.count; i++) { - if (m->mptr_names.items[i] != NULL) - tb_printf(tb, "%s%s", n++ > 0 ? ", " : "", m->mptr_names.items[i]); - } - fatal_trace("destroying mspace with %d live mptrs: %s", - m->roots.count - m->free_mptrs.count - 1, tb_get(tb)); + int n = 0; + for (mptr_t p = m->roots; p; p = p->next) + tb_printf(tb, "%s%s", n++ > 0 ? ", " : "", p->name); + fatal_trace("destroying mspace with %d live mptrs: %s", n, tb_get(tb)); } #endif @@ -137,9 +137,11 @@ void mspace_destroy(mspace_t *m) free(it); } - ACLEAR(m->roots); - DEBUG_ONLY(ACLEAR(m->mptr_names)); - ACLEAR(m->free_mptrs); + for (mptr_t p = m->free_mptrs, tmp; p; p = tmp) { + tmp = p->next; + free(p); + } + mask_free(&(m->headmask)); nvc_munmap(m->space, m->maxsize); free(m); @@ -266,19 +268,25 @@ void mspace_set_oom_handler(mspace_t *m, mspace_oom_fn_t fn) mptr_t mptr_new(mspace_t *m, const char *name) { - if (m->free_mptrs.count > 0) { - mptr_t ptr = APOP(m->free_mptrs); - assert(AGET(m->roots, ptr) == NULL); - assert(AGET(m->mptr_names, ptr) == NULL); - DEBUG_ONLY(m->mptr_names.items[ptr] = name); - return ptr; + mptr_t ptr; + if (m->free_mptrs != NULL) { + ptr = m->free_mptrs; + m->free_mptrs = ptr->next; } - else { - mptr_t ptr = m->roots.count; - APUSH(m->roots, NULL); - DEBUG_ONLY(APUSH(m->mptr_names, name)); - return ptr; + else + ptr = xmalloc(sizeof(struct _mptr)); + + if (m->roots != NULL) { + assert(m->roots->prev == NULL); + m->roots->prev = ptr; } + + ptr->ptr = NULL; + ptr->next = m->roots; + ptr->prev = NULL; + DEBUG_ONLY(ptr->name = name); + + return (m->roots = ptr); } void mptr_free(mspace_t *m, mptr_t *ptr) @@ -286,31 +294,30 @@ void mptr_free(mspace_t *m, mptr_t *ptr) if (*ptr == MPTR_INVALID) return; - assert(*ptr < m->roots.count); - assert(*ptr < m->mptr_names.count); - assert(m->free_mptrs.count < m->roots.count); + if ((*ptr)->next != NULL) + (*ptr)->next->prev = (*ptr)->prev; - m->roots.items[*ptr] = NULL; - DEBUG_ONLY(m->mptr_names.items[*ptr] = NULL); - APUSH(m->free_mptrs, *ptr); - *ptr = MPTR_INVALID; -} + if ((*ptr)->prev != NULL) + (*ptr)->prev->next = (*ptr)->next; + else { + assert(m->roots == *ptr); + m->roots = (*ptr)->next; + } -void *mptr_get(mspace_t *m, mptr_t ptr) -{ - assert(ptr != MPTR_INVALID); - assert(ptr < m->roots.count); + (*ptr)->prev = NULL; + (*ptr)->ptr = NULL; + DEBUG_ONLY((*ptr)->name = NULL); + + (*ptr)->next = m->free_mptrs; + m->free_mptrs = *ptr; - return m->roots.items[ptr]; + *ptr = NULL; } -void mptr_put(mspace_t *m, mptr_t ptr, void *value) +void **mptr_get(mptr_t ptr) { assert(ptr != MPTR_INVALID); - assert(ptr < m->roots.count); - assert(value == NULL || is_mspace_ptr(m, value)); - - m->roots.items[ptr] = value; + return &(ptr->ptr); } void tlab_acquire(mspace_t *m, tlab_t *t) @@ -324,7 +331,7 @@ void tlab_acquire(mspace_t *m, tlab_t *t) t->mptr = mptr_new(m, "tlab"); // This ensures the TLAB is kept alive over GCs - mptr_put(m, t->mptr, t->base); + *mptr_get(t->mptr) = t->base; } void tlab_release(tlab_t *t) @@ -381,6 +388,16 @@ static void mspace_mark_root(mspace_t *m, intptr_t p, gc_state_t *state) } } +static void mspace_mark_mptr(mspace_t *m, mptr_t ptr, gc_state_t *state) +{ +#ifdef DEBUG + if (unlikely(ptr->ptr != NULL && !is_mspace_ptr(m, ptr->ptr))) + fatal_trace("mptr %s points to unknown address %p", ptr->name, ptr->ptr); +#endif + + mspace_mark_root(m, (intptr_t)ptr->ptr, state); +} + __attribute__((no_sanitize_address, noinline)) static void mspace_gc(mspace_t *m) { @@ -392,8 +409,8 @@ static void mspace_gc(mspace_t *m) gc_state_t state = {}; mask_init(&(state.markmask), m->maxlines); - for (int i = 0; i < m->roots.count; i++) - mspace_mark_root(m, (intptr_t)m->roots.items[i], &state); + for (mptr_t p = m->roots; p; p = p->next) + mspace_mark_mptr(m, p, &state); struct cpu_state cpu; capture_registers(&cpu); diff --git a/src/rt/mspace.h b/src/rt/mspace.h index c4954f80..c2d10c76 100644 --- a/src/rt/mspace.h +++ b/src/rt/mspace.h @@ -20,8 +20,8 @@ #include "prim.h" -typedef uint32_t mptr_t; -#define MPTR_INVALID 0 +#define MPTR_INVALID NULL +typedef struct _mptr *mptr_t; typedef void (*mspace_oom_fn_t)(mspace_t *, size_t); @@ -63,8 +63,7 @@ void *tlab_alloc(tlab_t *t, size_t size); mptr_t mptr_new(mspace_t *m, const char *name); void mptr_free(mspace_t *m, mptr_t *ptr); -void *mptr_get(mspace_t *m, mptr_t ptr); -void mptr_put(mspace_t *m, mptr_t ptr, void *value); +void **mptr_get(mptr_t ptr); #define MSPACE_CURRENT_FRAME __builtin_frame_address(0) diff --git a/test/test_mspace.c b/test/test_mspace.c index 74adf15b..a733ab21 100644 --- a/test/test_mspace.c +++ b/test/test_mspace.c @@ -108,16 +108,16 @@ START_TEST(test_mptr) generate_garbage(m, 5, sizeof(int)); mptr_t p = mptr_new(m, "test"); - mptr_put(m, p, mspace_alloc(m, sizeof(int))); - *(int *)mptr_get(m, p) = 42; + *mptr_get(p) = mspace_alloc(m, sizeof(int)); + *(int *)*mptr_get(p) = 42; // Do enough allocations to trigger a GC generate_garbage(m, 1000, sizeof(int)); - ck_assert_int_eq(*(int *)mptr_get(m, p), 42); + ck_assert_int_eq(*(int *)*mptr_get(p), 42); mptr_free(m, &p); - ck_assert_int_eq(p, MPTR_INVALID); + ck_assert_ptr_eq(p, MPTR_INVALID); mspace_destroy(m); } -- 2.39.2