From patchwork Sat Nov 24 08:11:19 2012
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [tsan] Instrument atomics
Date: Fri, 23 Nov 2012 22:11:19 -0000
From: Jakub Jelinek <jakub@redhat.com>
X-Patchwork-Id: 201443
Message-Id: <20121124081119.GH2315@tucnak.redhat.com>
To: Dmitry Vyukov <dvyukov@google.com>
Cc: Kostya Serebryany <kcc@google.com>, Dodji Seketeli <dseketel@redhat.com>, 
 GCC Patches <gcc-patches@gcc.gnu.org>

On Fri, Nov 23, 2012 at 08:10:39PM +0400, Dmitry Vyukov wrote:
> I've just committed a patch to llvm with failure_memory_order (r168518).

Shouldn't it be something like this instead?


The failure model is for when the CAS fails, success model for when it
succeeds.  Or perhaps the IsReleaseOrder/Release lines should be also after
the __sync_val_compare_and_swap (that doesn't matter, right, one thing is
the actual accesses, another thing is tsan events?) and also depend on the
corresponding model?

	Jakub


--- tsan_interface_atomic.cc.jj	2012-11-23 17:20:49.000000000 +0100
+++ tsan_interface_atomic.cc	2012-11-23 19:06:05.917147568 +0100
@@ -199,15 +199,17 @@ static T AtomicFetchXor(ThreadState *thr
 template<typename T>
 static bool AtomicCAS(ThreadState *thr, uptr pc,
     volatile T *a, T *c, T v, morder mo, morder fmo) {
-  (void)fmo;
   if (IsReleaseOrder(mo))
     Release(thr, pc, (uptr)a);
   T cc = *c;
   T pr = __sync_val_compare_and_swap(a, cc, v);
-  if (IsAcquireOrder(mo))
-    Acquire(thr, pc, (uptr)a);
-  if (pr == cc)
+  if (pr == cc) {
+    if (IsAcquireOrder(mo))
+      Acquire(thr, pc, (uptr)a);
     return true;
+  }
+  if (IsAcquireOrder(fmo))
+    Acquire(thr, pc, (uptr)a);
   *c = pr;
   return false;
 }