rts: Use fences instead of explicit barriers

5 files changed, 44 insertions, 31 deletions

diff --git a/rts/PrimOps.cmm b/rts/PrimOps.cmm
index c20b4d471f..fc2b13962d 100644
--- a/rts/PrimOps.cmm
+++ b/rts/PrimOps.cmm
@@ -977,8 +977,7 @@ stg_deRefWeakzh ( gcptr w )
     W_ code, info;
     gcptr val;
 
-    info = GET_INFO(w);
-    prim_read_barrier;
+    info = GET_INFO_ACQUIRE(w);
 
     if (info == stg_WHITEHOLE_info) {
         // w is locked by another thread. Now it's not immediately clear if w is
@@ -1763,8 +1762,7 @@ loop:
         updateRemembSetPushPtr(val);
         return (val);
     }
-    qinfo = StgHeader_info(q);
-    prim_read_barrier;
+    qinfo = GET_INFO_ACQUIRE(q);
     if (qinfo == stg_IND_info ||
         qinfo == stg_MSG_NULL_info) {
         q = StgInd_indirectee(q);
@@ -1831,8 +1829,7 @@ loop:
         return (1, val);
     }
 
-    qinfo = StgHeader_info(q);
-    prim_read_barrier;
+    qinfo = GET_INFO_ACQUIRE(q);
 
     if (qinfo == stg_IND_info ||
         qinfo == stg_MSG_NULL_info) {
@@ -1934,8 +1931,7 @@ loop:
         return ();
     }
 
-    qinfo = StgHeader_info(q);
-    prim_read_barrier;
+    qinfo = GET_INFO_ACQUIRE(q);
 
     if (qinfo == stg_IND_info ||
         qinfo == stg_MSG_NULL_info) {
@@ -2024,8 +2020,7 @@ loop:
         return (1);
     }
 
-    qinfo = StgHeader_info(q);
-    prim_read_barrier;
+    qinfo = GET_INFO_ACQUIRE(q);
 
     if (qinfo == stg_IND_info ||
         qinfo == stg_MSG_NULL_info) {
diff --git a/rts/SMPClosureOps.h b/rts/SMPClosureOps.h
index 2df88db06c..f5242a9cf1 100644
--- a/rts/SMPClosureOps.h
+++ b/rts/SMPClosureOps.h
@@ -26,8 +26,7 @@
 #endif
 
 #define unlockClosure(ptr,info)                 \
-    prim_write_barrier;                         \
-    StgHeader_info(ptr) = info;
+    %release StgHeader_info(ptr) = info;
 
 #else
 
diff --git a/rts/StgMiscClosures.cmm b/rts/StgMiscClosures.cmm
index 013db8731b..374358028d 100644
--- a/rts/StgMiscClosures.cmm
+++ b/rts/StgMiscClosures.cmm
@@ -528,14 +528,19 @@ INFO_TABLE(stg_BLACKHOLE,1,0,BLACKHOLE,"BLACKHOLE","BLACKHOLE")
     TICK_ENT_DYN_IND(); /* tick */
 
 retry:
-    prim_read_barrier;
-    p = StgInd_indirectee(node);
+#if defined(TSAN_ENABLED)
+    // See Note [ThreadSanitizer and fences]
+    W_ unused; unused = %acquire GET_INFO(node);
+#endif
+    // Synchronizes with the release-store in updateWithIndirection.
+    // See Note [Heap memory barriers] in SMP.h.
+    ACQUIRE_FENCE;
+    p = %relaxed StgInd_indirectee(node);
     if (GETTAG(p) != 0) {
         return (p);
     }
 
-    info = StgHeader_info(p);
-    prim_read_barrier;
+    info = 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
diff --git a/rts/Updates.h b/rts/Updates.h
index 81ddca2684..b35790b536 100644
--- a/rts/Updates.h
+++ b/rts/Updates.h
@@ -47,8 +47,6 @@
  */
 #define updateWithIndirection(p1, p2, and_then) \
     W_ bd;                                                      \
-                                                                \
-    prim_write_barrier;                                         \
     bd = Bdescr(p1);                                            \
     if (bdescr_gen_no(bd) != 0 :: bits16) {                     \
       IF_NONMOVING_WRITE_BARRIER_ENABLED {                      \
@@ -59,8 +57,9 @@
     } else {                                                    \
       TICK_UPD_NEW_IND();                                       \
     }                                                           \
+                                                                \
     OVERWRITING_CLOSURE(p1);                                    \
-    StgInd_indirectee(p1) = p2;                                 \
+    %relaxed StgInd_indirectee(p1) = p2;                        \
     SET_INFO_RELEASE(p1, stg_BLACKHOLE_info);                   \
     LDV_RECORD_CREATE(p1);                                      \
     and_then;
diff --git a/rts/include/Cmm.h b/rts/include/Cmm.h
index f688a68e4d..fb267a5581 100644
--- a/rts/include/Cmm.h
+++ b/rts/include/Cmm.h
@@ -277,18 +277,20 @@
 // because LDV profiling relies on entering closures to mark them as
 // "used".
 
-#define LOAD_INFO(ret,x)                        \
-    info = %INFO_PTR(UNTAG(x));
+#define LOAD_INFO_ACQUIRE(ret,x)                \
+    info = %INFO_PTR(UNTAG(x));                 \
+    prim_read_barrier;
 
 #define UNTAG_IF_PROF(x) UNTAG(x)
 
 #else
 
-#define LOAD_INFO(ret,x)                        \
+#define LOAD_INFO_ACQUIRE(ret,x)                \
   if (GETTAG(x) != 0) {                         \
       ret(x);                                   \
   }                                             \
-  info = %INFO_PTR(x);
+  info = %INFO_PTR(x);                          \
+  prim_read_barrier;
 
 #define UNTAG_IF_PROF(x) (x) /* already untagged */
 
@@ -316,9 +318,8 @@
 #define ENTER_(ret,x)                                   \
  again:                                                 \
   W_ info;                                              \
-  LOAD_INFO(ret,x)                                      \
   /* See Note [Heap memory barriers] in SMP.h */        \
-  prim_read_barrier;                                    \
+  LOAD_INFO_ACQUIRE(ret,x);                             \
   switch [INVALID_OBJECT .. N_CLOSURE_TYPES]            \
          (TO_W_( %INFO_TYPE(%STD_INFO(info)) )) {       \
   case                                                  \
@@ -594,8 +595,9 @@
 
 /* Getting/setting the info pointer of a closure */
 #define SET_INFO(p,info) StgHeader_info(p) = info
-#define SET_INFO_RELEASE(p,info) prim_write_barrier; StgHeader_info(p) = info
+#define SET_INFO_RELEASE(p,info) %release StgHeader_info(p) = info
 #define GET_INFO(p) StgHeader_info(p)
+#define GET_INFO_ACQUIRE(p) %acquire GET_INFO(p)
 
 /* Determine the size of an ordinary closure from its info table */
 #define sizeW_fromITBL(itbl) \
@@ -663,19 +665,32 @@
 #define IS_STACK_CLEAN(stack) \
     ((TO_W_(StgStack_dirty(stack)) & STACK_DIRTY) == 0)
 
+/* Note [ThreadSanitizer and fences]
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * Sadly ThreadSanitizer currently doesn't support analysis of fences.
+ * Consequently, to avoid false-positive data-race reports cases which rely
+ * on fences for ordering should (when TSAN_ENABLED is defined) also provide
+ * explicit ordered accesses to make ordering apparent to TSAN.
+ */
+
 // Memory barriers.
 // For discussion of how these are used to fence heap object
 // accesses see Note [Heap memory barriers] in SMP.h.
 #if defined(THREADED_RTS)
 #define prim_read_barrier prim %read_barrier()
-#else
-#define prim_read_barrier /* nothing */
-#endif
-#if defined(THREADED_RTS)
 #define prim_write_barrier prim %write_barrier()
+
+// See Note [ThreadSanitizer and fences]
+#define RELEASE_FENCE prim %write_barrier()
+#define ACQUIRE_FENCE prim %read_barrier()
+
 #else
+
+#define prim_read_barrier /* nothing */
 #define prim_write_barrier /* nothing */
-#endif
+#define RELEASE_FENCE /* nothing */
+#define ACQUIRE_FENCE /* nothing */
+#endif /* THREADED_RTS */
 
 /* -----------------------------------------------------------------------------
    Ticky macros