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?
--- 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;
}
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
