[sanitizer] Use __atomic_load/store() built-ins for generic 32-bit targets

[glaubitz]

Summary

Simplifies the code and fixes the build on SPARC.

See discussion in: http://lists.llvm.org/pipermail/llvm-dev/2020-October/145937.html

[dvyukov]

Looks good with nits.
Please also upload with full file context, I wonder what's before that "} else {" on line 80.

compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang_other.h

54

Also remove this comment.

82

Also remove this comment.

[glaubitz]

Addressed all comments and suggestions.

[glaubitz]

Replace tabs with spaces to fix indentation.

[jrtc27]

I suggested this initially as a quick fix, not the final solution. Really if we want to fix this properly we should just always use __atomic_store with an appropriately-translated memory order, as is done in libcxx (this is really just reimplementing a subset of libcxx as a standalone library).

[glaubitz]

This patch unbreaks the build on bi-arch systems which are not x86 or MIPS, so I think it's okay to pick up the change and rewrite the code in cleaner terms later.

[glaubitz]

Could someone with access commit this patch for me?

[dvyukov]

Merged as: https://github.com/llvm/llvm-project/commit/26c1ced41c262bb87619cfa8a03c1879c63fb5f7
Thanks!

compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang_other.h

 inline typename T::Type atomic_load(
     const volatile T *a, memory_order mo) {
   DCHECK(mo & (memory_order_relaxed | memory_order_consume
       | memory_order_acquire | memory_order_seq_cst));
   DCHECK(!((uptr)a % sizeof(*a)));
   typename T::Type v;
 
   if (sizeof(*a) < 8 || sizeof(void*) == 8) {
     // Assume that aligned loads are atomic.
     if (mo == memory_order_relaxed) {
       v = a->val_dont_use;
     } else if (mo == memory_order_consume) {
       // Assume that processor respects data dependencies
       // (and that compiler won't break them).
       __asm__ __volatile__("" ::: "memory");
       v = a->val_dont_use;
       __asm__ __volatile__("" ::: "memory");
     } else if (mo == memory_order_acquire) {
       __asm__ __volatile__("" ::: "memory");
       v = a->val_dont_use;
       __sync_synchronize();
     } else {  // seq_cst
       // E.g. on POWER we need a hw fence even before the store.
       __sync_synchronize();
       v = a->val_dont_use;
       __sync_synchronize();
     }
   } else {
-    // 64-bit load on 32-bit platform.
-    // Gross, but simple and reliable.
-    // Assume that it is not in read-only memory.
-    v = __sync_fetch_and_add(
-        const_cast<typename T::Type volatile *>(&a->val_dont_use), 0);
+    __atomic_load(const_cast<typename T::Type volatile *>(&a->val_dont_use), &v,
+                  __ATOMIC_SEQ_CST);
   }
   return v;
 }

 inline void atomic_store(volatile T *a, typename T::Type v, memory_order mo) {
   DCHECK(mo & (memory_order_relaxed | memory_order_release
       | memory_order_seq_cst));
   DCHECK(!((uptr)a % sizeof(*a)));
 
   if (sizeof(*a) < 8 || sizeof(void*) == 8) {
     // Assume that aligned loads are atomic.
     if (mo == memory_order_relaxed) {
       a->val_dont_use = v;
     } else if (mo == memory_order_release) {
       __sync_synchronize();
       a->val_dont_use = v;
       __asm__ __volatile__("" ::: "memory");
     } else {  // seq_cst
       __sync_synchronize();
       a->val_dont_use = v;
       __sync_synchronize();
     }
   } else {
-    // 64-bit store on 32-bit platform.
-    // Gross, but simple and reliable.
-    typename T::Type cmp = a->val_dont_use;
-    typename T::Type cur;
-    for (;;) {
-      cur = __sync_val_compare_and_swap(&a->val_dont_use, cmp, v);
-      if (cur == cmp || cur == v)
-        break;
-      cmp = cur;
-    }
+    __atomic_store(&a->val_dont_use, &v, __ATOMIC_SEQ_CST);
   }
 }
 
 }  // namespace __sanitizer
 
 #endif  // #ifndef SANITIZER_ATOMIC_CLANG_OTHER_H