GitLab

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
```

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 =