[Git][ghc/ghc][wip/tsan/codegen] 11 commits: updremset

Ben Gamari (@bgamari) gitlab at gitlab.haskell.org
Wed Nov 16 14:48:51 UTC 2022



Ben Gamari pushed to branch wip/tsan/codegen at Glasgow Haskell Compiler / GHC


Commits:
22c71381 by Ben Gamari at 2022-11-15T23:46:27-05:00
updremset

- - - - -
cefb3d2c by Ben Gamari at 2022-11-16T00:14:11-05:00
pending_sync

- - - - -
c35b553f by Ben Gamari at 2022-11-16T00:14:24-05:00
nonmoving shutdown

- - - - -
06c4919a by Ben Gamari at 2022-11-16T00:14:32-05:00
nonmoving bitmap

- - - - -
3316692e by Ben Gamari at 2022-11-16T00:14:38-05:00
upd rem set load

- - - - -
ce52429b by Ben Gamari at 2022-11-16T08:49:02-05:00
note: Document rationale for lack of ABA problem

- - - - -
6d084f74 by Ben Gamari at 2022-11-16T09:36:26-05:00
StgToCmm: Ensure that MutVars have necessary barriers

- - - - -
6acc152b by Ben Gamari at 2022-11-16T09:44:52-05:00
Note race with wakeBlockingQueue

- - - - -
161a7a67 by Ben Gamari at 2022-11-16T09:46:10-05:00
nonmoving: HI

- - - - -
39aac38a by Ben Gamari at 2022-11-16T09:47:07-05:00
nonmovingMarkQueue

- - - - -
18b2a0a1 by Ben Gamari at 2022-11-16T09:48:19-05:00
nonmoving/shortcut: Fix race

- - - - -


8 changed files:

- compiler/GHC/StgToCmm/Prim.hs
- rts/Schedule.c
- rts/StgMiscClosures.cmm
- rts/include/rts/storage/ClosureMacros.h
- rts/sm/NonMoving.c
- rts/sm/NonMoving.h
- rts/sm/NonMovingMark.c
- rts/sm/NonMovingShortcut.c


Changes:

=====================================
compiler/GHC/StgToCmm/Prim.hs
=====================================
@@ -283,7 +283,8 @@ emitPrimOp cfg primop =
     emitAssign (CmmLocal res) currentTSOExpr
 
   ReadMutVarOp -> \[mutv] -> opIntoRegs $ \[res] ->
-    emitAssign (CmmLocal res) (cmmLoadIndexW platform mutv (fixedHdrSizeW profile) (gcWord platform))
+    emitPrimCall [res] (MO_AtomicRead (wordWidth platform) MemOrderAcquire)
+        [ cmmOffsetW platform mutv (fixedHdrSizeW profile) ]
 
   WriteMutVarOp -> \[mutv, var] -> opIntoRegs $ \res@[] -> do
     old_val <- CmmLocal <$> newTemp (cmmExprType platform var)
@@ -294,8 +295,8 @@ emitPrimOp cfg primop =
     -- Note that this also must come after we read the old value to ensure
     -- that the read of old_val comes before another core's write to the
     -- MutVar's value.
-    emitPrimCall res MO_WriteBarrier []
-    emitStore (cmmOffsetW platform mutv (fixedHdrSizeW profile)) var
+    emitPrimCall [] (MO_AtomicWrite (wordWidth platform) MemOrderRelease)
+        [ cmmOffsetW platform mutv (fixedHdrSizeW profile), var ]
 
     platform <- getPlatform
     mkdirtyMutVarCCall <- getCode $! emitCCall


=====================================
rts/Schedule.c
=====================================
@@ -1414,7 +1414,7 @@ void stopAllCapabilitiesWith (Capability **pCap, Task *task, SyncType sync_type)
 
     acquireAllCapabilities(pCap ? *pCap : NULL, task);
 
-    pending_sync = 0;
+    RELAXED_STORE(&pending_sync, 0);
     signalCondition(&sync_finished_cond);
 }
 #endif
@@ -1860,7 +1860,7 @@ delete_threads_and_gc:
 #if defined(THREADED_RTS)
     // reset pending_sync *before* GC, so that when the GC threads
     // emerge they don't immediately re-enter the GC.
-    pending_sync = 0;
+    RELAXED_STORE(&pending_sync, 0);
     signalCondition(&sync_finished_cond);
     GarbageCollect(collect_gen, heap_census, is_overflow_gc, deadlock_detect, gc_type, cap, idle_cap);
 #else


=====================================
rts/StgMiscClosures.cmm
=====================================
@@ -540,7 +540,8 @@ retry:
         return (p);
     }
 
-    info = GET_INFO(p);
+    // May race with OVERWRITE_INFO in wakeBlockingQueue()
+    info = %relaxed GET_INFO(p);
     if (info == stg_IND_info) {
         // This could happen, if e.g. we got a BLOCKING_QUEUE that has
         // just been replaced with an IND by another thread in


=====================================
rts/include/rts/storage/ClosureMacros.h
=====================================
@@ -97,6 +97,12 @@ EXTERN_INLINE const StgInfoTable *get_itbl(const StgClosure *c)
     return INFO_PTR_TO_STRUCT(RELAXED_LOAD(&c->header.info));
 }
 
+EXTERN_INLINE const StgInfoTable *get_itbl_acquire(const StgClosure *c);
+EXTERN_INLINE const StgInfoTable *get_itbl_acquire(const StgClosure *c)
+{
+    return INFO_PTR_TO_STRUCT(ACQUIRE_LOAD(&c->header.info));
+}
+
 EXTERN_INLINE const StgRetInfoTable *get_ret_itbl(const StgClosure *c);
 EXTERN_INLINE const StgRetInfoTable *get_ret_itbl(const StgClosure *c)
 {


=====================================
rts/sm/NonMoving.c
=====================================
@@ -87,6 +87,10 @@ Mutex concurrent_coll_finished_lock;
  *  - A set of *filled* segments, which contain no unallocated blocks and will
  *    be collected during the next major GC cycle
  *
+ * These sets are maintained as atomic singly-linked lists. This is not
+ * susceptible to the ABA problem since we are guaranteed to push a given
+ * segment to a list only once per garbage collection cycle.
+ *
  * Storage for segments is allocated using the block allocator using an aligned
  * group of NONMOVING_SEGMENT_BLOCKS blocks. This makes the task of locating
  * the segment header for a clone a simple matter of bit-masking (as
@@ -545,7 +549,7 @@ void nonmovingPushFreeSegment(struct NonmovingSegment *seg)
 static struct NonmovingSegment *nonmovingPopFreeSegment(void)
 {
     while (true) {
-        struct NonmovingSegment *seg = RELAXED_LOAD(&nonmovingHeap.free);
+        struct NonmovingSegment *seg = ACQUIRE_LOAD(&nonmovingHeap.free);
         if (seg == NULL) {
             return NULL;
         }
@@ -643,7 +647,8 @@ static bool advance_next_free(struct NonmovingSegment *seg, const unsigned int b
 static struct NonmovingSegment *pop_active_segment(struct NonmovingAllocator *alloca)
 {
     while (true) {
-        struct NonmovingSegment *seg = RELAXED_LOAD(&alloca->active);
+        // Synchronizes with CAS in nonmovingPushActiveSegment
+        struct NonmovingSegment *seg = ACQUIRE_LOAD(&alloca->active);
         if (seg == NULL) {
             return NULL;
         }
@@ -750,7 +755,7 @@ void nonmovingStop(void)
                    "waiting for nonmoving collector thread to terminate");
         ACQUIRE_LOCK(&concurrent_coll_finished_lock);
         waitCondition(&concurrent_coll_finished, &concurrent_coll_finished_lock);
-        ACQUIRE_LOCK(&nonmoving_collection_mutex);
+        RELEASE_LOCK(&concurrent_coll_finished_lock);
     }
 #endif
 }
@@ -763,6 +768,9 @@ void nonmovingExit(void)
     nonmovingStop();
 
 #if defined(THREADED_RTS)
+    ACQUIRE_LOCK(&nonmoving_collection_mutex);
+    RELEASE_LOCK(&nonmoving_collection_mutex);
+
     closeMutex(&concurrent_coll_finished_lock);
     closeCondition(&concurrent_coll_finished);
     closeMutex(&nonmoving_collection_mutex);


=====================================
rts/sm/NonMoving.h
=====================================
@@ -170,7 +170,7 @@ INLINE_HEADER void nonmovingPushActiveSegment(struct NonmovingSegment *seg)
     SET_SEGMENT_STATE(seg, ACTIVE);
     while (true) {
         struct NonmovingSegment *current_active = RELAXED_LOAD(&alloc->active);
-        RELAXED_STORE(&seg->link, current_active);
+        seg->link = current_active;
         if (cas((StgVolatilePtr) &alloc->active, (StgWord) current_active, (StgWord) seg) == (StgWord) current_active) {
             break;
         }
@@ -276,12 +276,12 @@ extern uint8_t nonmovingMarkEpoch;
 
 INLINE_HEADER void nonmovingSetMark(struct NonmovingSegment *seg, nonmoving_block_idx i)
 {
-    seg->bitmap[i] = nonmovingMarkEpoch;
+    RELAXED_STORE(&seg->bitmap[i], nonmovingMarkEpoch);
 }
 
 INLINE_HEADER uint8_t nonmovingGetMark(struct NonmovingSegment *seg, nonmoving_block_idx i)
 {
-    return seg->bitmap[i];
+    return RELAXED_LOAD(&seg->bitmap[i]);
 }
 
 INLINE_HEADER void nonmovingSetClosureMark(StgPtr p)


=====================================
rts/sm/NonMovingMark.c
=====================================
@@ -267,7 +267,7 @@ static void nonmovingAddUpdRemSetBlocks_(MarkQueue *rset)
 {
     if (markQueueIsEmpty(rset)) return;
 
-    // find the tail of the queue
+    // find the tail of the remembered set mark queue
     bdescr *start = rset->blocks;
     bdescr *end = start;
     while (end->link != NULL)
@@ -293,9 +293,8 @@ static void nonmovingAddUpdRemSetBlocks_lock(MarkQueue *rset)
     // Reset the state of the remembered set.
     ACQUIRE_SM_LOCK;
     init_mark_queue_(rset);
-    rset->is_upd_rem_set = true;
-    nonmovingResetUpdRemSetQueue(rset);
     RELEASE_SM_LOCK;
+    rset->is_upd_rem_set = true;
 }
 
 /*
@@ -310,7 +309,8 @@ static void nonmovingAddUpdRemSetBlocks_lock(MarkQueue *rset)
 void nonmovingAddUpdRemSetBlocks(UpdRemSet *rset)
 {
     nonmovingAddUpdRemSetBlocks_(&rset->queue);
-    nonmovingResetUpdRemSet(rset);
+    init_mark_queue_(&rset->queue);
+    rset->queue.is_upd_rem_set = true;
 }
 
 #if defined(THREADED_RTS)
@@ -804,7 +804,7 @@ void markQueuePushClosure (MarkQueue *q,
 /* TODO: Do we really never want to specify the origin here? */
 void markQueueAddRoot (MarkQueue* q, StgClosure** root)
 {
-    markQueuePushClosure(q, *root, NULL);
+    markQueuePushClosureGC(q, *root);
 }
 
 /* Push a closure to the mark queue without origin information */
@@ -1772,7 +1772,7 @@ nonmovingMark (MarkQueue *queue)
         }
         case NULL_ENTRY:
             // Perhaps the update remembered set has more to mark...
-            if (upd_rem_set_block_list) {
+            if (RELAXED_LOAD(&upd_rem_set_block_list) != NULL) {
                 ACQUIRE_LOCK(&upd_rem_set_lock);
                 bdescr *old = queue->blocks;
                 queue->blocks = upd_rem_set_block_list;


=====================================
rts/sm/NonMovingShortcut.c
=====================================
@@ -153,7 +153,8 @@ selectee_changed:
     // Selectee is a non-moving object, mark it.
     markQueuePushClosure(queue, selectee, NULL);
 
-    const StgInfoTable *selectee_info_tbl = get_itbl(selectee);
+    // This may synchronize with the release in updateWithIndirection.
+    const StgInfoTable *selectee_info_tbl = get_itbl_acquire(selectee);
     switch (selectee_info_tbl->type) {
         case WHITEHOLE: {
             // Probably a loop. Abort.



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/5e3ff9e71f6219a5a8ae2bb6709a34dd79783243...18b2a0a1e613ee26d892baca93abdb5c87ab75a4

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/5e3ff9e71f6219a5a8ae2bb6709a34dd79783243...18b2a0a1e613ee26d892baca93abdb5c87ab75a4
You're receiving this email because of your account on gitlab.haskell.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20221116/c9bf7326/attachment-0001.html>


More information about the ghc-commits mailing list