Ben Gamari pushed to branch wip/T26053 at Glasgow Haskell Compiler / GHC Commits: 189a368b by Ben Gamari at 2025-02-06T10:32:27+01:00 compiler: Fix CPP guards around ghc_unique_counter64 The `ghc_unique_counter64` symbol was introduced in the RTS in the 64-bit unique refactor (!10568) which has been backported to %9.6.7 and %9.8.4. Update the CPP to reflect this. Fixes #25576. (cherry-picked from commit 63d74f4d15ecaa8d711bd12fa64f77fce72b5f79) - - - - - 81fcda40 by Ben Gamari at 2025-07-15T12:33:36-04:00 rts/nonmoving: Fix comment spelling - - - - - 978b3689 by Ben Gamari at 2025-07-15T12:39:43-04:00 rts/nonmoving: Use atomic operations to update bd->flags - - - - - d77c7954 by Ben Gamari at 2025-07-15T12:39:43-04:00 Debug output - - - - - 4 changed files: - compiler/cbits/genSym.c - rts/include/stg/SMP.h - rts/sm/NonMoving.c - rts/sm/NonMovingMark.c Changes: ===================================== compiler/cbits/genSym.c ===================================== @@ -9,11 +9,20 @@ // // The CPP is thus about the RTS version GHC is linked against, and not the // version of the GHC being built. -#if !MIN_VERSION_GLASGOW_HASKELL(9,8,4,0) -HsWord64 ghc_unique_counter64 = 0; + +#if MIN_VERSION_GLASGOW_HASKELL(9,9,0,0) +// Unique64 patch was present in 9.10 and later +#define HAVE_UNIQUE64 1 +#elif !MIN_VERSION_GLASGOW_HASKELL(9,9,0,0) && MIN_VERSION_GLASGOW_HASKELL(9,8,4,0) +// Unique64 patch was backported to 9.8.4 +#define HAVE_UNIQUE64 1 +#elif !MIN_VERSION_GLASGOW_HASKELL(9,7,0,0) && MIN_VERSION_GLASGOW_HASKELL(9,6,7,0) +// Unique64 patch was backported to 9.6.7 +#define HAVE_UNIQUE64 1 #endif -#if !MIN_VERSION_GLASGOW_HASKELL(9,3,0,0) -HsInt ghc_unique_inc = 1; + +#if !defined(HAVE_UNIQUE64) +HsWord64 ghc_unique_counter64 = 0; #endif // This function has been added to the RTS. Here we pessimistically assume ===================================== rts/include/stg/SMP.h ===================================== @@ -516,6 +516,14 @@ atomic_dec(StgVolatilePtr p) #endif } +EXTERN_INLINE StgWord +atomic_or(StgVolatilePtr p, StgWord v) +{ + // TODO: This ordering is stronger than necessary for many users (e.g. + // setting flags). + return __atomic_or_fetch(p, v, __ATOMIC_SEQ_CST); +} + /* * Some architectures have a way to tell the CPU that we're in a * busy-wait loop, and the processor should look for something else to ===================================== rts/sm/NonMoving.c ===================================== @@ -560,7 +560,7 @@ Mutex concurrent_coll_finished_lock; * arbitrary allocator sizes, we need to do some precomputation and make * use of the integer division by constants optimisation. * - * We currenlty try to balance these considerations by adopting the following scheme. + * We currently try to balance these considerations by adopting the following scheme. * We have nonmoving_alloca_dense_cnt "dense" allocators starting with size * NONMOVING_ALLOCA0, and incrementing by NONMOVING_ALLOCA_DENSE_INCREMENT. * These service the vast majority of allocations. ===================================== rts/sm/NonMovingMark.c ===================================== @@ -722,13 +722,14 @@ STATIC_INLINE bool needs_upd_rem_set_mark(StgClosure *p) { // TODO: Deduplicate with mark_closure bdescr *bd = Bdescr((StgPtr) p); + uint16_t flags = RELAXED_LOAD(&bd->flags); if (bd->gen != oldest_gen) { return false; - } else if (bd->flags & BF_LARGE) { - if (! (bd->flags & BF_NONMOVING_SWEEPING)) { + } else if (flags & BF_LARGE) { + if (! (flags & BF_NONMOVING_SWEEPING)) { return false; } else { - return ! (bd->flags & BF_MARKED); + return ! (flags & BF_MARKED); } } else { struct NonmovingSegment *seg = nonmovingGetSegment((StgPtr) p); @@ -740,8 +741,8 @@ STATIC_INLINE bool needs_upd_rem_set_mark(StgClosure *p) static void finish_upd_rem_set_mark_large(bdescr* bd) { // Someone else may have already marked it. ACQUIRE_LOCK(&nonmoving_large_objects_mutex); - if (! (bd->flags & BF_MARKED)) { - bd->flags |= BF_MARKED; + if (! (RELAXED_LOAD(&bd->flags) & BF_MARKED)) { + atomic_or((StgPtr) &bd->flags, BF_MARKED); dbl_link_remove(bd, &nonmoving_large_objects); dbl_link_onto(bd, &nonmoving_marked_large_objects); n_nonmoving_large_blocks -= bd->blocks; @@ -785,18 +786,28 @@ void updateRemembSetPushStack(Capability *cap, StgStack *stack) debugTrace(DEBUG_nonmoving_gc, "upd_rem_set: STACK %p", stack->sp); trace_stack(&cap->upd_rem_set.queue, stack); finish_upd_rem_set_mark((StgClosure *) stack); - return; } else { // The concurrent GC has claimed the right to mark the stack. // Wait until it finishes marking before proceeding with // mutation. - while (needs_upd_rem_set_mark((StgClosure *) stack)) + uint64_t iters = 0; + while (needs_upd_rem_set_mark((StgClosure *) stack)) { + iters++; + if (iters > 100000000) { + bdescr *bd = Bdescr((StgPtr) stack); + debugBelch("updateRemSetPushStack: stuck: %p\n", stack); + debugBelch(" bd->flags: %x\n", bd->flags); + debugBelch(" epoch : %x\n", nonmovingMarkEpoch); + debugBelch(" marking: %x\n", stack->marking); + abort(); + } + #if defined(PARALLEL_GC) busy_wait_nop(); // TODO: Spinning here is unfortunate #else ; #endif - return; + } } } } @@ -1371,35 +1382,36 @@ mark_closure (MarkQueue *queue, const StgClosure *p0, StgClosure **origin) // N.B. only the first block of a compact region is guaranteed to carry // BF_NONMOVING; consequently we must separately check for BF_COMPACT. - if (bd->flags & (BF_COMPACT | BF_NONMOVING)) { + const uint16_t flags = RELAXED_LOAD(&bd->flags); + if (flags & (BF_COMPACT | BF_NONMOVING)) { - if (bd->flags & BF_COMPACT) { + if (flags & BF_COMPACT) { StgCompactNFData *str = objectGetCompact((StgClosure*)p); bd = Bdescr((P_)str); - if (! (bd->flags & BF_NONMOVING_SWEEPING)) { + if (! (flags & BF_NONMOVING_SWEEPING)) { // Not in the snapshot return; } - if (! (bd->flags & BF_MARKED)) { + if (! (flags & BF_MARKED)) { dbl_link_remove(bd, &nonmoving_compact_objects); dbl_link_onto(bd, &nonmoving_marked_compact_objects); StgWord blocks = str->totalW / BLOCK_SIZE_W; n_nonmoving_compact_blocks -= blocks; n_nonmoving_marked_compact_blocks += blocks; - bd->flags |= BF_MARKED; + atomic_or(&bd->flags, BF_MARKED); } // N.B. the object being marked is in a compact region so by // definition there is no need to do any tracing here. goto done; - } else if (bd->flags & BF_LARGE) { - if (! (bd->flags & BF_NONMOVING_SWEEPING)) { + } else if (flags & BF_LARGE) { + if (! (flags & BF_NONMOVING_SWEEPING)) { // Not in the snapshot goto done; } - if (bd->flags & BF_MARKED) { + if (flags & BF_MARKED) { goto done; } } else { @@ -1731,24 +1743,25 @@ mark_closure (MarkQueue *queue, const StgClosure *p0, StgClosure **origin) * the object's pointers since in the case of marking stacks there may be a * mutator waiting for us to finish so it can start execution. */ - if (bd->flags & BF_LARGE) { + uint16_t bd_flags = RELAXED_LOAD(&bd->flags); + if (bd_flags & BF_LARGE) { /* Marking a large object isn't idempotent since we move it to * nonmoving_marked_large_objects; to ensure that we don't repeatedly * mark a large object, we only set BF_MARKED on large objects in the * nonmoving heap while holding nonmoving_large_objects_mutex */ ACQUIRE_LOCK(&nonmoving_large_objects_mutex); - if (! (bd->flags & BF_MARKED)) { + if (! (bd_flags & BF_MARKED)) { // Remove the object from nonmoving_large_objects and link it to // nonmoving_marked_large_objects dbl_link_remove(bd, &nonmoving_large_objects); dbl_link_onto(bd, &nonmoving_marked_large_objects); n_nonmoving_large_blocks -= bd->blocks; n_nonmoving_marked_large_blocks += bd->blocks; - bd->flags |= BF_MARKED; + RELAXED_STORE(&bd->flags, flags | BF_MARKED); } RELEASE_LOCK(&nonmoving_large_objects_mutex); - } else if (bd->flags & BF_NONMOVING) { + } else if (bd_flags & BF_NONMOVING) { // TODO: Kill repetition struct NonmovingSegment *seg = nonmovingGetSegment((StgPtr) p); nonmoving_block_idx block_idx = nonmovingGetBlockIdx((StgPtr) p); @@ -1927,7 +1940,7 @@ static bool nonmovingIsNowAlive (StgClosure *p) bdescr *bd = Bdescr((P_)p); - const uint16_t flags = bd->flags; + const uint16_t flags = RELAXED_LOAD(&bd->flags); if (flags & BF_LARGE) { if (flags & BF_PINNED && !(flags & BF_NONMOVING)) { // In this case we have a pinned object living in a non-full @@ -1937,15 +1950,15 @@ static bool nonmovingIsNowAlive (StgClosure *p) return true; } - ASSERT(bd->flags & BF_NONMOVING); - return (bd->flags & BF_NONMOVING_SWEEPING) == 0 + ASSERT(flags & BF_NONMOVING); + return (flags & BF_NONMOVING_SWEEPING) == 0 // the large object wasn't in the snapshot and therefore wasn't marked - || (bd->flags & BF_MARKED) != 0; + || (flags & BF_MARKED) != 0; // The object was marked } else { // All non-static objects in the non-moving heap should be marked as // BF_NONMOVING. - ASSERT(bd->flags & BF_NONMOVING); + ASSERT(flags & BF_NONMOVING); struct NonmovingSegment *seg = nonmovingGetSegment((StgPtr) p); StgClosure *snapshot_loc = View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/1c70dab6e65f911b915d9367e3d22fa... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/1c70dab6e65f911b915d9367e3d22fa... You're receiving this email because of your account on gitlab.haskell.org.