Nonmoving: Ensure write barrier vanishes in non-threaded RTS
authorBen Gamari <ben@smart-cactus.org>
Sun, 19 May 2019 17:29:42 +0000 (13:29 -0400)
committerBen Gamari <ben@smart-cactus.org>
Wed, 19 Jun 2019 01:42:05 +0000 (21:42 -0400)
includes/Cmm.h
includes/rts/NonMoving.h
rts/Messages.c
rts/PrimOps.cmm
rts/STM.c
rts/Schedule.c
rts/ThreadPaused.c
rts/Threads.c
rts/Updates.h
rts/sm/NonMovingMark.h
rts/sm/Storage.c

index 48024ce..62e65b7 100644 (file)
     return (dst);
 
 
+//
+// Nonmoving write barrier helpers
+//
+// See Note [Update remembered set] in NonMovingMark.c.
+
 #if defined(THREADED_RTS)
-#define IF_WRITE_BARRIER_ENABLED                               \
+#define IF_NONMOVING_WRITE_BARRIER_ENABLED                     \
     if (W_[nonmoving_write_barrier_enabled] != 0) (likely: False)
 #else
 // A similar measure is also taken in rts/NonMoving.h, but that isn't visible from C--
-#define IF_WRITE_BARRIER_ENABLED                               \
+#define IF_NONMOVING_WRITE_BARRIER_ENABLED                     \
     if (0)
 #define nonmoving_write_barrier_enabled 0
 #endif
 
 // A useful helper for pushing a pointer to the update remembered set.
-// See Note [Update remembered set] in NonMovingMark.c.
 #define updateRemembSetPushPtr(p)                                    \
-    IF_WRITE_BARRIER_ENABLED {                                       \
+    IF_NONMOVING_WRITE_BARRIER_ENABLED {                             \
       ccall updateRemembSetPushClosure_(BaseReg "ptr", p "ptr");     \
     }
index 6a6d96b..7c07ab4 100644 (file)
@@ -21,4 +21,7 @@ void updateRemembSetPushClosure(Capability *cap, StgClosure *p);
 
 void updateRemembSetPushThunk_(StgRegTable *reg, StgThunk *p);
 
+// Note that RTS code should not condition on this directly by rather
+// use the IF_NONMOVING_WRITE_BARRIER_ENABLED macro to ensure that
+// the barrier is eliminated in the non-threaded RTS.
 extern StgWord DLL_IMPORT_DATA_VAR(nonmoving_write_barrier_enabled);
index 4283df4..88db926 100644 (file)
@@ -256,7 +256,7 @@ loop:
 
         // point to the BLOCKING_QUEUE from the BLACKHOLE
         write_barrier(); // make the BQ visible
-        if (RTS_UNLIKELY(nonmoving_write_barrier_enabled)) {
+        IF_NONMOVING_WRITE_BARRIER_ENABLED {
             updateRemembSetPushClosure(cap, (StgClosure*)p);
         }
         ((StgInd*)bh)->indirectee = (StgClosure *)bq;
@@ -287,7 +287,7 @@ loop:
         }
 #endif
 
-        if (RTS_UNLIKELY(nonmoving_write_barrier_enabled)) {
+        IF_NONMOVING_WRITE_BARRIER_ENABLED {
             // We are about to overwrite bq->queue; make sure its current value
             // makes it into the update remembered set
             updateRemembSetPushClosure(cap, (StgClosure*)bq->queue);
index a7036b1..65a56b4 100644 (file)
@@ -474,7 +474,7 @@ stg_copyArray_barrier ( W_ hdr_size, gcptr dst, W_ dst_off, W_ n)
     end = p + WDS(n);
 
 again:
-    IF_WRITE_BARRIER_ENABLED {
+    IF_NONMOVING_WRITE_BARRIER_ENABLED {
         ccall updateRemembSetPushClosure_(BaseReg "ptr", W_[p] "ptr");
     }
     p = p + WDS(1);
@@ -490,7 +490,7 @@ stg_copySmallArrayzh ( gcptr src, W_ src_off, gcptr dst, W_ dst_off, W_ n)
     W_ dst_p, src_p, bytes;
 
     if (n > 0) {
-        IF_WRITE_BARRIER_ENABLED {
+        IF_NONMOVING_WRITE_BARRIER_ENABLED {
             call stg_copyArray_barrier(SIZEOF_StgSmallMutArrPtrs,
                                       dst, dst_off, n);
         }
@@ -511,7 +511,7 @@ stg_copySmallMutableArrayzh ( gcptr src, W_ src_off, gcptr dst, W_ dst_off, W_ n
     W_ dst_p, src_p, bytes;
 
     if (n > 0) {
-        IF_WRITE_BARRIER_ENABLED {
+        IF_NONMOVING_WRITE_BARRIER_ENABLED {
             call stg_copyArray_barrier(SIZEOF_StgSmallMutArrPtrs,
                                       dst, dst_off, n);
         }
index c17f33a..1dde70b 100644 (file)
--- a/rts/STM.c
+++ b/rts/STM.c
@@ -297,8 +297,10 @@ static StgClosure *lock_tvar(Capability *cap,
   } while (cas((void *)&(s -> current_value),
                (StgWord)result, (StgWord)trec) != (StgWord)result);
 
-  if (RTS_UNLIKELY(nonmoving_write_barrier_enabled && result)) {
-      updateRemembSetPushClosure(cap, result);
+
+  IF_NONMOVING_WRITE_BARRIER_ENABLED {
+      if (result)
+          updateRemembSetPushClosure(cap, result);
   }
   return result;
 }
@@ -323,8 +325,9 @@ static StgBool cond_lock_tvar(Capability *cap,
   TRACE("%p : cond_lock_tvar(%p, %p)", trec, s, expected);
   w = cas((void *)&(s -> current_value), (StgWord)expected, (StgWord)trec);
   result = (StgClosure *)w;
-  if (RTS_UNLIKELY(nonmoving_write_barrier_enabled && result)) {
-      updateRemembSetPushClosure(cap, expected);
+  IF_NONMOVING_WRITE_BARRIER_ENABLED {
+      if (result)
+          updateRemembSetPushClosure(cap, expected);
   }
   TRACE("%p : %s", trec, result ? "success" : "failure");
   return (result == expected);
index d0ec444..0a5ac94 100644 (file)
@@ -2503,7 +2503,7 @@ resumeThread (void *task_)
     incall->suspended_tso = NULL;
     incall->suspended_cap = NULL;
     // we will modify tso->_link
-    if (RTS_UNLIKELY(nonmoving_write_barrier_enabled)) {
+    IF_NONMOVING_WRITE_BARRIER_ENABLED {
         updateRemembSetPushClosure(cap, (StgClosure *)tso->_link);
     }
     tso->_link = END_TSO_QUEUE;
index 4b24362..38dacb8 100644 (file)
@@ -330,15 +330,16 @@ threadPaused(Capability *cap, StgTSO *tso)
             }
 #endif
 
-            if (RTS_UNLIKELY(nonmoving_write_barrier_enabled
-                             && ip_THUNK(INFO_PTR_TO_STRUCT(bh_info)))) {
-                // We are about to replace a thunk with a blackhole.
-                // Add the free variables of the closure we are about to
-                // overwrite to the update remembered set.
-                // N.B. We caught the WHITEHOLE case above.
-                updateRemembSetPushThunkEager(cap,
-                                             THUNK_INFO_PTR_TO_STRUCT(bh_info),
-                                             (StgThunk *) bh);
+            IF_NONMOVING_WRITE_BARRIER_ENABLED {
+                if (ip_THUNK(INFO_PTR_TO_STRUCT(bh_info))) {
+                    // We are about to replace a thunk with a blackhole.
+                    // Add the free variables of the closure we are about to
+                    // overwrite to the update remembered set.
+                    // N.B. We caught the WHITEHOLE case above.
+                    updateRemembSetPushThunkEager(cap,
+                                                 THUNK_INFO_PTR_TO_STRUCT(bh_info),
+                                                 (StgThunk *) bh);
+                }
             }
 
             // The payload of the BLACKHOLE points to the TSO
index 488fcdc..03e9380 100644 (file)
@@ -711,7 +711,7 @@ threadStackUnderflow (Capability *cap, StgTSO *tso)
             barf("threadStackUnderflow: not enough space for return values");
         }
 
-        if (RTS_UNLIKELY(nonmoving_write_barrier_enabled)) {
+        IF_NONMOVING_WRITE_BARRIER_ENABLED {
             // ensure that values that we copy into the new stack are marked
             // for the nonmoving collector. Note that these values won't
             // necessarily form a full closure so we need to handle them
index 7776f92..de84791 100644 (file)
@@ -44,7 +44,7 @@
     W_ bd;                                                      \
                                                                 \
     OVERWRITING_CLOSURE(p1);                                    \
-    IF_WRITE_BARRIER_ENABLED {                                  \
+    IF_NONMOVING_WRITE_BARRIER_ENABLED {                        \
       ccall updateRemembSetPushThunk_(BaseReg, p1 "ptr");       \
     }                                                           \
     StgInd_indirectee(p1) = p2;                                 \
@@ -73,7 +73,7 @@ INLINE_HEADER void updateWithIndirection (Capability *cap,
     /* not necessarily true: ASSERT( !closure_IND(p1) ); */
     /* occurs in RaiseAsync.c:raiseAsync() */
     OVERWRITING_CLOSURE(p1);
-    if (RTS_UNLIKELY(nonmoving_write_barrier_enabled)) {
+    IF_NONMOVING_WRITE_BARRIER_ENABLED {
         updateRemembSetPushThunk(cap, (StgThunk*)p1);
     }
     ((StgInd *)p1)->indirectee = p2;
index aecad67..410a230 100644 (file)
@@ -143,6 +143,15 @@ extern StgIndStatic *debug_caf_list_snapshot;
 extern MarkQueue *current_mark_queue;
 extern bdescr *upd_rem_set_block_list;
 
+// A similar macro is defined in includes/Cmm.h for C-- code.
+#if defined(THREADED_RTS)
+#define IF_NONMOVING_WRITE_BARRIER_ENABLED \
+    if (RTS_UNLIKELY(nonmoving_write_barrier_enabled))
+#else
+#define IF_NONMOVING_WRITE_BARRIER_ENABLED \
+    if (0)
+#endif
+
 void nonmovingMarkInitUpdRemSet(void);
 
 void init_upd_rem_set(UpdRemSet *rset);
index 6871025..e651e99 100644 (file)
@@ -478,7 +478,7 @@ lockCAF (StgRegTable *reg, StgIndStatic *caf)
     // reference should be in SRTs
     ASSERT(orig_info_tbl->layout.payload.ptrs == 0);
     // Becuase the payload is empty we just push the SRT
-    if (RTS_UNLIKELY(nonmoving_write_barrier_enabled)) {
+    IF_NONMOVING_WRITE_BARRIER_ENABLED {
         StgThunkInfoTable *thunk_info = itbl_to_thunk_itbl(orig_info_tbl);
         if (thunk_info->i.srt) {
             updateRemembSetPushClosure(cap, GET_SRT(thunk_info));
@@ -1205,7 +1205,7 @@ dirty_MUT_VAR(StgRegTable *reg, StgMutVar *mvar, StgClosure *old)
     if (mvar->header.info == &stg_MUT_VAR_CLEAN_info) {
         mvar->header.info = &stg_MUT_VAR_DIRTY_info;
         recordClosureMutated(cap, (StgClosure *) mvar);
-        if (RTS_UNLIKELY(nonmoving_write_barrier_enabled != 0)) {
+        IF_NONMOVING_WRITE_BARRIER_ENABLED {
             updateRemembSetPushClosure_(reg, old);
         }
     }
@@ -1224,7 +1224,7 @@ dirty_TVAR(Capability *cap, StgTVar *p,
     if (p->header.info == &stg_TVAR_CLEAN_info) {
         p->header.info = &stg_TVAR_DIRTY_info;
         recordClosureMutated(cap,(StgClosure*)p);
-        if (RTS_UNLIKELY(nonmoving_write_barrier_enabled != 0)) {
+        IF_NONMOVING_WRITE_BARRIER_ENABLED {
             updateRemembSetPushClosure(cap, old);
         }
     }
@@ -1241,8 +1241,9 @@ setTSOLink (Capability *cap, StgTSO *tso, StgTSO *target)
     if (tso->dirty == 0) {
         tso->dirty = 1;
         recordClosureMutated(cap,(StgClosure*)tso);
-        if (RTS_UNLIKELY(nonmoving_write_barrier_enabled))
+        IF_NONMOVING_WRITE_BARRIER_ENABLED {
             updateRemembSetPushClosure(cap, (StgClosure *) tso->_link);
+        }
     }
     tso->_link = target;
 }
@@ -1253,8 +1254,9 @@ setTSOPrev (Capability *cap, StgTSO *tso, StgTSO *target)
     if (tso->dirty == 0) {
         tso->dirty = 1;
         recordClosureMutated(cap,(StgClosure*)tso);
-        if (RTS_UNLIKELY(nonmoving_write_barrier_enabled))
+        IF_NONMOVING_WRITE_BARRIER_ENABLED {
             updateRemembSetPushClosure(cap, (StgClosure *) tso->block_info.prev);
+        }
     }
     tso->block_info.prev = target;
 }
@@ -1267,8 +1269,9 @@ dirty_TSO (Capability *cap, StgTSO *tso)
         recordClosureMutated(cap,(StgClosure*)tso);
     }
 
-    if (RTS_UNLIKELY(nonmoving_write_barrier_enabled))
+    IF_NONMOVING_WRITE_BARRIER_ENABLED {
         updateRemembSetPushTSO(cap, tso);
+    }
 }
 
 void
@@ -1276,8 +1279,9 @@ dirty_STACK (Capability *cap, StgStack *stack)
 {
     // First push to upd_rem_set before we set stack->dirty since we
     // the nonmoving collector may already be marking the stack.
-    if (RTS_UNLIKELY(nonmoving_write_barrier_enabled))
+    IF_NONMOVING_WRITE_BARRIER_ENABLED {
         updateRemembSetPushStack(cap, stack);
+    }
 
     if (! (stack->dirty & STACK_DIRTY)) {
         stack->dirty = STACK_DIRTY;
@@ -1301,7 +1305,7 @@ void
 update_MVAR(StgRegTable *reg, StgClosure *p, StgClosure *old_val)
 {
     Capability *cap = regTableToCapability(reg);
-    if (RTS_UNLIKELY(nonmoving_write_barrier_enabled)) {
+    IF_NONMOVING_WRITE_BARRIER_ENABLED {
         StgMVar *mvar = (StgMVar *) p;
         updateRemembSetPushClosure(cap, old_val);
         updateRemembSetPushClosure(cap, (StgClosure *) mvar->head);