[PATCH] D46597: [sanitizer] Correct 64-bit atomic_store on 32-bit "other" platforms

[Dmitry Vyukov via Phabricator via llvm-commits]

dvyukov added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_atomic_clang_other.h:89
       cur = __sync_val_compare_and_swap(&a->val_dont_use, cmp, v);
-      if (cmp == v)
+      if (cur == v)
         break;
----------------
The current is not just slow, it looks broken to me. Is it even used anywhere? Maybe we should just drop it. I think we generally require C++11 for building compiler-rt.

The canonical way to do this is `cur == cmp` which should accomplish the operation with 1 CAS in common case. The old check was probably an attempt to optimize for the case when the variable already contains the value we want to store (and we obtained an atomic snapshot of the current value with CAS). So perhaps this could be `cur == cmp || cur == v`?


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D46597