Remove origin arguments from write barrier functionswip/nonmoving-gc

Write barriers push old values of updated field, e.g. when we have

    *q = p;

we do

    updateRemembSetPushClosure(*q, q);
    *q = p;

Here the second argument ("origin") is not useful because by the time we
do the update we invalidate "origin" (`q` is no longer origin of old
`*q`).

In general it doesn't make sense to record origins in write barriers so
we remove all origin arguments from write barriers.

Fixes #170.

11 files changed, 37 insertions, 67 deletions

diff --git a/compiler/codeGen/StgCmmPrim.hs b/compiler/codeGen/StgCmmPrim.hs
index ee46bbf946..ddef4fa0fd 100644
--- a/compiler/codeGen/StgCmmPrim.hs
+++ b/compiler/codeGen/StgCmmPrim.hs
@@ -1623,7 +1623,7 @@ doWritePtrArrayOp addr idx val
            hdr_size = arrPtrsHdrSize dflags
        -- Update remembered set for non-moving collector
        whenUpdRemSetEnabled
-           $ emitUpdRemSetPush dflags (cmmLoadIndexOffExpr dflags hdr_size ty addr ty idx)
+           $ emitUpdRemSetPush (cmmLoadIndexOffExpr dflags hdr_size ty addr ty idx)
        -- This write barrier is to ensure that the heap writes to the object
        -- referred to by val have happened before we write val into the array.
        -- See #12469 for details.
@@ -2569,19 +2569,15 @@ whenUpdRemSetEnabled = id -- TODO
 
 -- | Emit code to add an entry to a now-overwritten pointer to the update
 -- remembered set.
-emitUpdRemSetPush :: DynFlags
-                  -> CmmExpr   -- ^ value of pointer which was overwritten
+emitUpdRemSetPush :: CmmExpr   -- ^ value of pointer which was overwritten
                   -> FCode ()
-emitUpdRemSetPush dflags ptr = do
+emitUpdRemSetPush ptr = do
     emitRtsCall
       rtsUnitId
       (fsLit "updateRemembSetPushClosure_")
       [(CmmReg (CmmGlobal BaseReg), AddrHint),
-       (ptr, AddrHint),
-       (origin, AddrHint)]
+       (ptr, AddrHint)]
       False
-  where
-    origin = zeroExpr dflags
 
 -- | Push a range of pointer-array elements that are about to be copied over to
 -- the update remembered set.
diff --git a/includes/Cmm.h b/includes/Cmm.h
index b8440204ad..b21c5d1773 100644
--- a/includes/Cmm.h
+++ b/includes/Cmm.h
@@ -840,9 +840,9 @@
  * object `origin` has been overwritten. This implements the nonmoving collector's
  * update remembered set write barrier.
  */
-#define recordMutatedPtr(p, origin)                             \
+#define recordMutatedPtr(p)                             \
     if (nonmoving_write_barrier_enabled != 0) (likely: False) { \
-        ccall updateRemembSetPushClosure_(BaseReg "ptr", (p) "ptr", (origin) "ptr"); \
+        ccall updateRemembSetPushClosure_(BaseReg "ptr", (p) "ptr"); \
     }
 
 /* -----------------------------------------------------------------------------
diff --git a/includes/rts/NonMoving.h b/includes/rts/NonMoving.h
index e0787297f5..a27fb86b13 100644
--- a/includes/rts/NonMoving.h
+++ b/includes/rts/NonMoving.h
@@ -15,11 +15,6 @@
 
 /* This is called by the code generator */
 extern DLL_IMPORT_RTS
-void updateRemembSetPushClosure_(StgRegTable *reg,
-                               StgClosure *p,
-                               StgClosure **origin);
+void updateRemembSetPushClosure_(StgRegTable *reg, StgClosure *p);
 
-
-void updateRemembSetPushClosure(Capability *cap,
-                              StgClosure *p,
-                              StgClosure **origin);
+void updateRemembSetPushClosure(Capability *cap, StgClosure *p);
diff --git a/rts/Messages.c b/rts/Messages.c
index 6a09cd898b..89415ea3d6 100644
--- a/rts/Messages.c
+++ b/rts/Messages.c
@@ -257,7 +257,7 @@ loop:
         // point to the BLOCKING_QUEUE from the BLACKHOLE
         write_barrier(); // make the BQ visible
         if (nonmoving_write_barrier_enabled) {
-            updateRemembSetPushClosure(cap, (StgClosure*)p, NULL);
+            updateRemembSetPushClosure(cap, (StgClosure*)p);
         }
         ((StgInd*)bh)->indirectee = (StgClosure *)bq;
         recordClosureMutated(cap,bh); // bh was mutated
@@ -290,7 +290,7 @@ loop:
         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, NULL);
+            updateRemembSetPushClosure(cap, (StgClosure*)bq->queue);
         }
         msg->link = bq->queue;
         bq->queue = msg;
diff --git a/rts/PrimOps.cmm b/rts/PrimOps.cmm
index cb5e8821f0..81282832b4 100644
--- a/rts/PrimOps.cmm
+++ b/rts/PrimOps.cmm
@@ -355,9 +355,7 @@ stg_casArrayzh ( gcptr arr, W_ ind, gcptr old, gcptr new )
 
         // Concurrent GC write barrier
         if (nonmoving_write_barrier_enabled != 0) (likely: False) {
-            ccall updateRemembSetPushClosure_(BaseReg "ptr",
-                                            old "ptr",
-                                            p "ptr");
+            ccall updateRemembSetPushClosure_(BaseReg "ptr", old "ptr");
         }
 
         return (0,new);
@@ -478,9 +476,7 @@ stg_copyArray_barrier ( W_ hdr_size, gcptr dst, W_ dst_off, W_ n)
     end = p + WDS(n);
 
 again:
-    ccall updateRemembSetPushClosure_(BaseReg "ptr",
-                                    W_[p] "ptr",
-                                    p "ptr");
+    ccall updateRemembSetPushClosure_(BaseReg "ptr", W_[p] "ptr");
     p = p + WDS(1);
     if (p < end) {
         goto again;
@@ -553,7 +549,7 @@ stg_casSmallArrayzh ( gcptr arr, W_ ind, gcptr old, gcptr new )
         SET_HDR(arr, stg_SMALL_MUT_ARR_PTRS_DIRTY_info, CCCS);
 
         // Concurrent GC write barrier
-        recordMutatedPtr(old, p);
+        recordMutatedPtr(old);
 
         return (0,new);
     }
@@ -801,8 +797,7 @@ stg_addCFinalizzerToWeakzh ( W_ fptr,   // finalizer
 
     // Write barrier for concurrent non-moving collector
     if (nonmoving_write_barrier_enabled != 0) (likely: False) {
-        ccall updateRemembSetPushClosure(MyCapability() "ptr",
-                                         StgWeak_cfinalizers(w) "ptr", NULL);
+        ccall updateRemembSetPushClosure(MyCapability() "ptr", StgWeak_cfinalizers(w) "ptr");
     }
 
     StgCFinalizerList_link(c) = StgWeak_cfinalizers(w);
@@ -886,8 +881,7 @@ stg_deRefWeakzh ( gcptr w )
         val = StgWeak_value(w);
         // See Note [Concurrent read barrier on deRefWeak#] in NonMovingMark.c
         if (nonmoving_write_barrier_enabled != 0) (likely: False) {
-            ccall updateRemembSetPushClosure(MyCapability() "ptr",
-                                           val "ptr", NULL);
+            ccall updateRemembSetPushClosure(MyCapability() "ptr", val "ptr");
         }
     } else {
         code = 0;
@@ -1599,8 +1593,7 @@ loop:
         // However, we do need to ensure that the nonmoving collector
         // knows about the reference to the value that we just removed...
         if (nonmoving_write_barrier_enabled != 0) (likely: False) {
-            ccall updateRemembSetPushClosure(MyCapability() "ptr",
-                                             val "ptr", NULL);
+            ccall updateRemembSetPushClosure(MyCapability() "ptr", val "ptr");
         }
         return (val);
     }
@@ -1993,9 +1986,7 @@ stg_makeStableNamezh ( P_ obj )
         if (nonmoving_write_barrier_enabled != 0) (likely: False) {
             // We only need to push obj itself to the update remembered set
             // since we know that sn_obj will be on the nursery anyways.
-            ccall updateRemembSetPushClosure_(BaseReg "ptr",
-                                              obj "ptr",
-                                              NULL "ptr");
+            ccall updateRemembSetPushClosure_(BaseReg "ptr", obj "ptr");
         }
     } else {
         sn_obj = snEntry_sn_obj(W_[stable_name_table] + index*SIZEOF_snEntry);
diff --git a/rts/STM.c b/rts/STM.c
index c78229e74d..9176f66e72 100644
--- a/rts/STM.c
+++ b/rts/STM.c
@@ -298,7 +298,7 @@ static StgClosure *lock_tvar(Capability *cap,
                (StgWord)result, (StgWord)trec) != (StgWord)result);
 
   if (nonmoving_write_barrier_enabled && result) {
-      updateRemembSetPushClosure(cap, result, NULL);
+      updateRemembSetPushClosure(cap, result);
   }
   return result;
 }
@@ -324,7 +324,7 @@ static StgBool cond_lock_tvar(Capability *cap,
   w = cas((void *)&(s -> current_value), (StgWord)expected, (StgWord)trec);
   result = (StgClosure *)w;
   if (nonmoving_write_barrier_enabled && result) {
-      updateRemembSetPushClosure(cap, expected, NULL);
+      updateRemembSetPushClosure(cap, expected);
   }
   TRACE("%p : %s", trec, result ? "success" : "failure");
   return (result == expected);
diff --git a/rts/Schedule.c b/rts/Schedule.c
index 0a240559f3..2edd7503e7 100644
--- a/rts/Schedule.c
+++ b/rts/Schedule.c
@@ -2500,7 +2500,7 @@ resumeThread (void *task_)
     incall->suspended_tso = NULL;
     incall->suspended_cap = NULL;
     // we will modify tso->_link
-    updateRemembSetPushClosure(cap, (StgClosure *)tso->_link, NULL);
+    updateRemembSetPushClosure(cap, (StgClosure *)tso->_link);
     tso->_link = END_TSO_QUEUE;
 
     traceEventRunThread(cap, tso);
diff --git a/rts/Threads.c b/rts/Threads.c
index 33cf907633..408304a6a6 100644
--- a/rts/Threads.c
+++ b/rts/Threads.c
@@ -717,9 +717,7 @@ threadStackUnderflow (Capability *cap, StgTSO *tso)
             // necessarily form a full closure so we need to handle them
             // specially.
             for (unsigned int i = 0; i < retvals; i++) {
-                updateRemembSetPushClosure(cap,
-                                         (StgClosure *) old_stack->sp[i],
-                                         NULL);
+                updateRemembSetPushClosure(cap, (StgClosure *) old_stack->sp[i]);
             }
         }
 
diff --git a/rts/sm/NonMovingMark.c b/rts/sm/NonMovingMark.c
index 8990ff7412..8fab83a765 100644
--- a/rts/sm/NonMovingMark.c
+++ b/rts/sm/NonMovingMark.c
@@ -517,32 +517,24 @@ void updateRemembSetPushThunkEager(Capability *cap,
     }
 }
 
-void updateRemembSetPushThunk_(StgRegTable *reg, StgThunk *origin)
+void updateRemembSetPushThunk_(StgRegTable *reg, StgThunk *p)
 {
     // TODO: Eliminate this conditional once it's folded into codegen
     if (!nonmoving_write_barrier_enabled) return;
-    updateRemembSetPushThunk(regTableToCapability(reg), origin);
+    updateRemembSetPushThunk(regTableToCapability(reg), p);
 }
 
-void updateRemembSetPushClosure(Capability *cap,
-                                StgClosure *p,
-                                StgClosure **origin)
+void updateRemembSetPushClosure(Capability *cap, StgClosure *p)
 {
     if (!nonmoving_write_barrier_enabled) return;
     if (!check_in_nonmoving_heap(p)) return;
     MarkQueue *queue = &cap->upd_rem_set.queue;
-    // We only shortcut things living in the nonmoving heap.
-    if (! check_in_nonmoving_heap((StgClosure *) origin))
-        origin = NULL;
-
-    push_closure(queue, p, origin);
+    push_closure(queue, p, NULL);
 }
 
-void updateRemembSetPushClosure_(StgRegTable *reg,
-                                 StgClosure *p,
-                                 StgClosure **origin)
+void updateRemembSetPushClosure_(StgRegTable *reg, StgClosure *p)
 {
-    updateRemembSetPushClosure(regTableToCapability(reg), p, origin);
+    updateRemembSetPushClosure(regTableToCapability(reg), p);
 }
 
 STATIC_INLINE bool needs_upd_rem_set_mark(StgClosure *p)
diff --git a/rts/sm/NonMovingMark.h b/rts/sm/NonMovingMark.h
index 4dd4632c93..730c7e2fd0 100644
--- a/rts/sm/NonMovingMark.h
+++ b/rts/sm/NonMovingMark.h
@@ -127,8 +127,8 @@ void nonmovingMarkInitUpdRemSet(void);
 
 void init_upd_rem_set(UpdRemSet *rset);
 void reset_upd_rem_set(UpdRemSet *rset);
-void updateRemembSetPushThunk(Capability *cap, StgThunk *origin);
-void updateRemembSetPushThunk_(StgRegTable *reg, StgThunk *origin);
+void updateRemembSetPushThunk(Capability *cap, StgThunk *p);
+void updateRemembSetPushThunk_(StgRegTable *reg, StgThunk *p);
 void updateRemembSetPushTSO(Capability *cap, StgTSO *tso);
 void updateRemembSetPushStack(Capability *cap, StgStack *stack);
 // Debug only -- count number of blocks in global UpdRemSet
diff --git a/rts/sm/Storage.c b/rts/sm/Storage.c
index dd253ade57..6fa6d777a8 100644
--- a/rts/sm/Storage.c
+++ b/rts/sm/Storage.c
@@ -433,7 +433,7 @@ lockCAF (StgRegTable *reg, StgIndStatic *caf)
     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), NULL);
+            updateRemembSetPushClosure(cap, GET_SRT(thunk_info));
         }
     }
 
@@ -1141,9 +1141,7 @@ dirty_MUT_VAR(StgRegTable *reg, StgMutVar *mvar, StgClosure *old)
         mvar->header.info = &stg_MUT_VAR_DIRTY_info;
         recordClosureMutated(cap, (StgClosure *) mvar);
         if (nonmoving_write_barrier_enabled != 0) {
-            updateRemembSetPushClosure_(reg,
-                                      old,
-                                      &mvar->var);
+            updateRemembSetPushClosure_(reg, old);
         }
     }
 }
@@ -1162,7 +1160,7 @@ dirty_TVAR(Capability *cap, StgTVar *p,
         p->header.info = &stg_TVAR_DIRTY_info;
         recordClosureMutated(cap,(StgClosure*)p);
         if (nonmoving_write_barrier_enabled != 0) {
-            updateRemembSetPushClosure(cap, old, NULL);
+            updateRemembSetPushClosure(cap, old);
         }
     }
 }
@@ -1179,7 +1177,7 @@ setTSOLink (Capability *cap, StgTSO *tso, StgTSO *target)
         tso->dirty = 1;
         recordClosureMutated(cap,(StgClosure*)tso);
         if (nonmoving_write_barrier_enabled)
-            updateRemembSetPushClosure(cap, (StgClosure *) tso->_link, NULL);
+            updateRemembSetPushClosure(cap, (StgClosure *) tso->_link);
     }
     tso->_link = target;
 }
@@ -1191,7 +1189,7 @@ setTSOPrev (Capability *cap, StgTSO *tso, StgTSO *target)
         tso->dirty = 1;
         recordClosureMutated(cap,(StgClosure*)tso);
         if (nonmoving_write_barrier_enabled)
-            updateRemembSetPushClosure(cap, (StgClosure *) tso->block_info.prev, NULL);
+            updateRemembSetPushClosure(cap, (StgClosure *) tso->block_info.prev);
     }
     tso->block_info.prev = target;
 }
@@ -1240,9 +1238,9 @@ update_MVAR(StgRegTable *reg, StgClosure *p, StgClosure *old_val)
     Capability *cap = regTableToCapability(reg);
     if (nonmoving_write_barrier_enabled) {
         StgMVar *mvar = (StgMVar *) p;
-        updateRemembSetPushClosure(cap, old_val, NULL);
-        updateRemembSetPushClosure(cap, (StgClosure *) mvar->head, NULL);
-        updateRemembSetPushClosure(cap, (StgClosure *) mvar->tail, NULL);
+        updateRemembSetPushClosure(cap, old_val);
+        updateRemembSetPushClosure(cap, (StgClosure *) mvar->head);
+        updateRemembSetPushClosure(cap, (StgClosure *) mvar->tail);
     }
 }