From 7b4f4427f819032ece6a13be71c5fc3e9829fa93 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Wed, 21 May 2025 14:31:23 +0200 Subject: [PATCH] Update for Whippet changes, VM stacks scanned partly-conservatively * libguile/trace.h (scm_from_ref, scm_to_ref): Helpers moved here; update all callers. * libguile/loader.c (scm_trace_loader_roots): * libguile/threads.c (scm_trace_thread_roots): * libguile/vm.c (scm_trace_vm_roots): Update for new pinned-roots prototype. * libguile/whippet-embedder.h (gc_extern_space_visit): Update for Whippet API changes. --- libguile/ephemerons.c | 18 +++------- libguile/finalizers.c | 14 ++------ libguile/loader.c | 16 ++++----- libguile/pairs.c | 6 +--- libguile/threads.c | 27 +++++++++----- libguile/trace.h | 72 ++++++++++++++++++++++--------------- libguile/vm.c | 58 ++++++++++++++++-------------- libguile/whippet-embedder.h | 63 ++++++++++++++++---------------- 8 files changed, 143 insertions(+), 131 deletions(-) diff --git a/libguile/ephemerons.c b/libguile/ephemerons.c index f8b7ecf54..f30ebdf49 100644 --- a/libguile/ephemerons.c +++ b/libguile/ephemerons.c @@ -38,6 +38,7 @@ #include "numbers.h" #include "ports.h" #include "threads.h" +#include "trace.h" #include "version.h" #include @@ -45,17 +46,6 @@ #include "ephemerons.h" - - -static inline SCM ref_to_scm (struct gc_ref ref) -{ - return SCM_PACK (gc_ref_value (ref)); -} -static inline struct gc_ref scm_to_ref (SCM scm) -{ - return gc_ref (SCM_UNPACK (scm)); -} - #define SCM_EPHEMERON_P(X) (SCM_HAS_TYP7 (X, scm_tc7_ephemeron)) @@ -95,14 +85,14 @@ SCM scm_c_ephemeron_key (struct gc_ephemeron *e) { struct gc_ref ret = gc_ephemeron_key (e); - return gc_ref_is_null (ret) ? SCM_BOOL_F : ref_to_scm (ret); + return gc_ref_is_null (ret) ? SCM_BOOL_F : scm_from_ref (ret); } SCM scm_c_ephemeron_value (struct gc_ephemeron *e) { struct gc_ref ret = gc_ephemeron_value (e); - return gc_ref_is_null (ret) ? SCM_BOOL_F : ref_to_scm (ret); + return gc_ref_is_null (ret) ? SCM_BOOL_F : scm_from_ref (ret); } void @@ -117,7 +107,7 @@ scm_c_ephemeron_swap_x (struct gc_ephemeron *e, SCM new_val) struct gc_ref ret = gc_ephemeron_swap_value (SCM_I_CURRENT_THREAD->mutator, e, scm_to_ref (new_val)); - return gc_ref_is_null (ret) ? SCM_BOOL_F : ref_to_scm (ret); + return gc_ref_is_null (ret) ? SCM_BOOL_F : scm_from_ref (ret); } struct gc_ephemeron* diff --git a/libguile/finalizers.c b/libguile/finalizers.c index 44648fe13..6fc76ce0b 100644 --- a/libguile/finalizers.c +++ b/libguile/finalizers.c @@ -45,6 +45,7 @@ #include "smob.h" #include "struct.h" #include "symbols.h" +#include "trace.h" #include "threads.h" #include "version.h" @@ -77,15 +78,6 @@ enum builtin_finalizer_kind FINALIZE_KIND_PORT, }; -static inline SCM ref_to_scm (struct gc_ref ref) -{ - return SCM_PACK (gc_ref_value (ref)); -} -static inline struct gc_ref scm_to_ref (SCM scm) -{ - return gc_ref (SCM_UNPACK (scm)); -} - static SCM add_finalizer (struct scm_thread *thread, SCM obj, SCM closure, enum finalizer_priority priority) @@ -213,8 +205,8 @@ run_finalizers (void *raw_data) finalizer; finalizer = gc_pop_finalizable (mut), data->count++) run_finalizer (thread, - ref_to_scm (gc_finalizer_object (finalizer)), - ref_to_scm (gc_finalizer_closure (finalizer))); + scm_from_ref (gc_finalizer_object (finalizer)), + scm_from_ref (gc_finalizer_closure (finalizer))); data->success = 1; return NULL; } diff --git a/libguile/loader.c b/libguile/loader.c index fd2a04609..5c6e14789 100644 --- a/libguile/loader.c +++ b/libguile/loader.c @@ -119,16 +119,16 @@ add_roots(char *lo, char *hi) } void -scm_trace_loader_conservative_roots (void (*trace_range)(uintptr_t lo, - uintptr_t hi, - int possibly_interior, - struct gc_heap *heap, - void *trace_data), - struct gc_heap *heap, - void *trace_data) +scm_trace_loader_roots (void (*trace_ambiguous)(uintptr_t lo, + uintptr_t hi, + int possibly_interior, + struct gc_heap *heap, + void *trace_data), + struct gc_heap *heap, + void *trace_data) { for (size_t i = 0; i < roots_count; i++) - trace_range(roots[i].lo, roots[i].hi, 0, heap, trace_data); + trace_ambiguous(roots[i].lo, roots[i].hi, 0, heap, trace_data); } /* The page size. */ diff --git a/libguile/pairs.c b/libguile/pairs.c index f61bc4009..a2c7a7d96 100644 --- a/libguile/pairs.c +++ b/libguile/pairs.c @@ -27,6 +27,7 @@ #include "boolean.h" #include "gc-internal.h" +#include "trace.h" #include "gsubr.h" #include "pairs.h" @@ -75,11 +76,6 @@ scm_cons2 (SCM w, SCM x, SCM y) return scm_cons (w, scm_cons (x, y)); } -static inline struct gc_ref scm_to_ref (SCM scm) -{ - return gc_ref (SCM_UNPACK (scm)); -} - int scm_is_mutable_pair (SCM x) { diff --git a/libguile/threads.c b/libguile/threads.c index 8dbd75982..5780a9481 100644 --- a/libguile/threads.c +++ b/libguile/threads.c @@ -74,6 +74,10 @@ +#if 0 +/* FIXME: For the moment, the bodies of thread objects are traced + conservatively; only bdw, heap-conservative-mmc, and + heap-conservative-parallel-mmc are supported. */ static void scm_trace_dynstack (scm_t_dynstack *dynstack, void (*trace_edge) (struct gc_edge edge, @@ -110,6 +114,7 @@ scm_trace_thread (struct scm_thread *thread, trace_edge (gc_edge (&thread->join_lock), heap, trace_data); trace_edge (gc_edge (&thread->join_results), heap, trace_data); } +#endif /* Guile-level thread objects are themselves GC-allocated. A thread object has two states: active and finished. A thread is active if it @@ -125,16 +130,20 @@ scm_trace_thread (struct scm_thread *thread, precise, threads need to be traced during root identification. */ void -scm_trace_thread_mutator_roots (struct scm_thread *thread, - void (*trace_edge) (struct gc_edge edge, - struct gc_heap *heap, - void *trace_data), - struct gc_heap *heap, void *trace_data) +scm_trace_thread_roots (struct scm_thread *thread, + void (*trace_pinned) (struct gc_ref ref, + struct gc_heap *heap, + void *trace_data), + void (*trace_ambiguous) (uintptr_t lo, + uintptr_t hi, + int possibly_interior, + struct gc_heap *heap, + void *trace_data), + struct gc_heap *heap, void *trace_data) { - scm_trace_vm (&thread->vm, trace_edge, heap, trace_data); - - /* FIXME: Call instead via gc_trace_object. */ - scm_trace_thread (thread, trace_edge, heap, trace_data); + trace_pinned (gc_ref_from_heap_object (thread), heap, trace_data); + scm_trace_vm_roots (&thread->vm, trace_pinned, trace_ambiguous, heap, + trace_data); } diff --git a/libguile/trace.h b/libguile/trace.h index d2705c112..4db9c5811 100644 --- a/libguile/trace.h +++ b/libguile/trace.h @@ -22,7 +22,7 @@ #include "libguile/scm.h" -#include "gc-edge.h" +#include "gc-ref.h" @@ -31,36 +31,52 @@ struct scm_vm; struct gc_heap; struct gc_heap_roots { int unused; }; -SCM_INTERNAL void -scm_trace_thread_mutator_roots (struct scm_thread *thread, - void (*trace_edge)(struct gc_edge edge, - struct gc_heap *heap, - void *trace_data), - struct gc_heap *heap, - void *trace_data); +static inline SCM +scm_from_ref (struct gc_ref ref) +{ + return SCM_PACK (gc_ref_value (ref)); +} -SCM_INTERNAL void scm_trace_thread (struct scm_thread *thread, - void (*trace_edge)(struct gc_edge edge, - struct gc_heap *heap, - void *trace_data), - struct gc_heap *heap, - void *trace_data); - -SCM_INTERNAL void scm_trace_vm (struct scm_vm *vp, - void (*trace_edge)(struct gc_edge edge, - struct gc_heap *heap, - void *trace_data), - struct gc_heap *heap, - void *trace_data); +static inline struct gc_ref +scm_to_ref (SCM scm) +{ + return gc_ref (SCM_UNPACK (scm)); +} SCM_INTERNAL void -scm_trace_loader_conservative_roots (void (*trace_range)(uintptr_t lo, - uintptr_t hi, - int possibly_interior, - struct gc_heap *heap, - void *trace_data), - struct gc_heap *heap, - void *trace_data); +scm_trace_thread_roots (struct scm_thread *thread, + void (*trace_pinned) (struct gc_ref ref, + struct gc_heap *heap, + void *trace_data), + void (*trace_ambiguous) (uintptr_t lo, + uintptr_t hi, + int possibly_interior, + struct gc_heap *heap, + void *trace_data), + struct gc_heap *heap, + void *trace_data); + +SCM_INTERNAL void +scm_trace_vm_roots (struct scm_vm *vp, + void (*trace_pinned) (struct gc_ref ref, + struct gc_heap *heap, + void *trace_data), + void (*trace_ambiguous) (uintptr_t lo, + uintptr_t hi, + int possibly_interior, + struct gc_heap *heap, + void *trace_data), + struct gc_heap *heap, + void *trace_data); + +SCM_INTERNAL void +scm_trace_loader_roots (void (*trace_ambiguous) (uintptr_t lo, + uintptr_t hi, + int possibly_interior, + struct gc_heap *heap, + void *trace_data), + struct gc_heap *heap, + void *trace_data); #endif /* SCM_THREADS_INTERNAL_H */ diff --git a/libguile/vm.c b/libguile/vm.c index cb46c54cc..1711a262a 100644 --- a/libguile/vm.c +++ b/libguile/vm.c @@ -707,11 +707,15 @@ enum slot_desc }; void -scm_trace_vm (struct scm_vm *vp, - void (*trace_edge) (struct gc_edge edge, - struct gc_heap *heap, - void *trace_data), - struct gc_heap *heap, void *trace_data) +scm_trace_vm_roots (struct scm_vm *vp, + void (*trace_pinned) (struct gc_ref ref, + struct gc_heap *heap, + void *trace_data), + void (*trace_ambiguous) (uintptr_t lo, uintptr_t hi, + int possibly_interior, + struct gc_heap *heap, + void *trace_data), + struct gc_heap *heap, void *trace_data) { union scm_vm_stack_element *sp, *fp; /* The first frame will be marked conservatively (without a slot map). @@ -730,29 +734,31 @@ scm_trace_vm (struct scm_vm *vp, { ptrdiff_t nlocals = SCM_FRAME_NUM_LOCALS (fp, sp); size_t slot = nlocals - 1; - for (slot = nlocals - 1; sp < fp; sp++, slot--) - { - enum slot_desc desc = SLOT_DESC_LIVE_GC; + if (slot_map) + for (slot = nlocals - 1; sp < fp; sp++, slot--) + { + enum slot_desc desc = + (slot_map[slot / 4U] >> ((slot % 4U) * 2)) & 3U; - if (slot_map) - desc = (slot_map[slot / 4U] >> ((slot % 4U) * 2)) & 3U; + switch (desc) + { + case SLOT_DESC_LIVE_RAW: + case SLOT_DESC_UNUSED: + break; + case SLOT_DESC_LIVE_GC: + trace_pinned (scm_to_ref (sp->as_scm), heap, trace_data); + break; + case SLOT_DESC_DEAD: + /* This value may become dead as a result of GC, + so we can't just leave it on the stack. */ + sp->as_scm = SCM_UNSPECIFIED; + break; + } + } + else + trace_ambiguous ((uintptr_t) sp, (uintptr_t) (sp + nlocals), 0, + heap, trace_data); - switch (desc) - { - case SLOT_DESC_LIVE_RAW: - break; - case SLOT_DESC_UNUSED: - case SLOT_DESC_LIVE_GC: - if (SCM_NIMP (sp->as_scm)) - trace_edge (gc_edge (sp), heap, trace_data); - break; - case SLOT_DESC_DEAD: - /* This value may become dead as a result of GC, - so we can't just leave it on the stack. */ - sp->as_scm = SCM_UNSPECIFIED; - break; - } - } sp = SCM_FRAME_PREVIOUS_SP (fp); /* Inner frames may have a dead slots map for precise marking. Note that there may be other reasons to not have a dead slots diff --git a/libguile/whippet-embedder.h b/libguile/whippet-embedder.h index c2b993c01..e81ac3e97 100644 --- a/libguile/whippet-embedder.h +++ b/libguile/whippet-embedder.h @@ -63,7 +63,6 @@ gc_is_valid_conservative_ref_displacement (uintptr_t displacement) { // FIXME: Here add tracing for SCM literals in .go files or .data // sections, perhaps. For now while we are using BDW-GC we can punt. static inline int gc_extern_space_visit (struct gc_extern_space *space, - struct gc_edge edge, struct gc_ref ref) { #if GC_CONSERVATIVE_TRACE return 0; @@ -78,6 +77,14 @@ static inline void gc_extern_space_finish_gc (struct gc_extern_space *space, int is_minor_gc) { } +static inline SCM scm_from_gc_ref (struct gc_ref ref) { + return SCM_PACK (gc_ref_value (ref)); +} + +static inline struct gc_ref scm_to_gc_ref (SCM scm) { + return gc_ref (SCM_UNPACK (scm)); +} + static inline void gc_trace_object (struct gc_ref ref, void (*trace_edge) (struct gc_edge edge, struct gc_heap *heap, @@ -101,8 +108,6 @@ static inline void gc_trace_mutator_roots (struct gc_mutator_roots *roots, void *trace_data), struct gc_heap *heap, void *trace_data) { - trace_edge (gc_edge (&roots->thread), heap, trace_data); - scm_trace_thread_mutator_roots (roots->thread, trace_edge, heap, trace_data); } static inline void gc_trace_heap_roots (struct gc_heap_roots *roots, @@ -114,36 +119,34 @@ static inline void gc_trace_heap_roots (struct gc_heap_roots *roots, } static inline void -gc_trace_mutator_conservative_roots (struct gc_mutator_roots *roots, - void (*trace_range) (uintptr_t lo, - uintptr_t hi, - int possibly_interior, - struct gc_heap *heap, - void *trace_data), - struct gc_heap *heap, - void *trace_data) { - /* FIXME: thread stack? Currently traced via the precise - gc_trace_mutator_roots. */ +gc_trace_mutator_pinned_roots (struct gc_mutator_roots *roots, + void (*trace_pinned) (struct gc_ref ref, + struct gc_heap *heap, + void *trace_data), + void (*trace_ambiguous) (uintptr_t lo, + uintptr_t hi, + int possibly_interior, + struct gc_heap *heap, + void *trace_data), + struct gc_heap *heap, + void *trace_data) { + scm_trace_thread_roots (roots->thread, trace_pinned, trace_ambiguous, + heap, trace_data); } static inline void -gc_trace_heap_conservative_roots (struct gc_heap_roots *roots, - void (*trace_range) (uintptr_t lo, - uintptr_t hi, - int possibly_interior, - struct gc_heap *heap, - void *trace_data), - struct gc_heap *heap, - void *trace_data) { - scm_trace_loader_conservative_roots(trace_range, heap, trace_data); -} - -static inline SCM scm_from_gc_ref (struct gc_ref ref) { - return SCM_PACK (gc_ref_value (ref)); -} - -static inline struct gc_ref scm_to_gc_ref (SCM scm) { - return gc_ref (SCM_UNPACK (scm)); +gc_trace_heap_pinned_roots (struct gc_heap_roots *roots, + void (*trace_pinned) (struct gc_ref ref, + struct gc_heap *heap, + void *trace_data), + void (*trace_ambiguous) (uintptr_t lo, + uintptr_t hi, + int possibly_interior, + struct gc_heap *heap, + void *trace_data), + struct gc_heap *heap, + void *trace_data) { + scm_trace_loader_roots (trace_ambiguous, heap, trace_data); } static inline scm_t_bits* scm_cell_type_loc (SCM scm) {