Allow build without __c11_atomic_fetch_nand

[brooks]

Summary

The atomic_fetch_nand libcalls added in 6ea2431d3f10 require
the c11_atomic_fetch_nand builtin which means compiler-rt
can't be built with an older verison of clang. Change this
so the locked implementation is used any time the builtin is
not available.

[brooks]

This pops up trying to build FreeBSD with LLVM 14 integrated using clang from LLVM 13 and not building a bootstrap toolchain.

[lkail]

I'm not sure if it's appropriate to use clang and compiler-rt in different versions. Added more reviewers for discussion.

[efriedma]

The current version doesn't have the right semantics: if the pointer is lock-free, you have to use a lock-free implementation (e.g. a cmpxchg loop).

In terms of whether we'd accept a patch, this code isn't really portable in the first place, but if it's necessary to build FreeBSD, we could add a small hack, I guess.

(On a more general note, FreeBSD should probably consider pursuing a different solution to atomic libcalls; this implementation doesn't work correctly if you try to share an atomic variable across multiple shared objects.)

[brooks]

In D126395#3539727, @lkail wrote:

I'm not sure if it's appropriate to use clang and compiler-rt in different versions. Added more reviewers for discussion.

As consumers of compiler-rt in FreeBSD, we expect to be able to build compiler-rt (or previously libgcc) with an external compiler which might not match exactly and can generally be somewhat older than the integrated one (e.g., we'd been building with llvm 13 and we support building with gcc 9). We've been doing this since we started our switch to clang well over a decade ago.

[brooks]

The other alternative I considered was to not generate __atomic_fetch_nand_* symbols at all if __c11_atomic_fetch_nand isn't available. It sounds like this might be the more correct approach. It has the downside that the symbols in FreeBSD's libraries can variety depending on which compiler we're using, but doesn't emit incorrect libcalls.

[efriedma]

The other alternative I considered was to not generate atomic_fetch_nand_* symbols at all if c11_atomic_fetch_nand isn't available

I'd accept that, sure.

[theraven]

I don't believe this version is correct. It unconditionally does the lock. If code using this is mixed with code where the compiler inlines the operation then the two operations will not be atomic with respect to eachother. Worse, an atomic-fetch-nand is not atomic with respect to any other operation (e.g. an atomic increment). This needs to have the same if (lockfree(ptr) guard and do a CAS loop in the is-lock-free case.

Edit: In between writing this and realising that Fabricator hadn't actually submitted the comment, I notice that efriedma has made the same observation.

[brooks]

I've added D126710 as an alternative that just doesn't create the libcall functions.

[brooks]

D126710 was committed

compiler-rt/lib/builtins/atomic.c

type __atomic_fetch_##opname##_##n(type *ptr, type val, int model) {         \
     Lock *l = lock_for_pointer(ptr);                                           \
     lock(l);                                                                   \
     type tmp = *ptr;                                                           \
     *ptr = tmp op val;                                                         \
     unlock(l);                                                                 \
     return tmp;                                                                \
   }
 
+#if __has_builtin(__c11_atomic_fetch_nand)
 #define ATOMIC_RMW_NAND(n, lockfree, type)                                     \
   type __atomic_fetch_nand_##n(type *ptr, type val, int model) {               \
     if (lockfree(ptr))                                                         \
       return __c11_atomic_fetch_nand((_Atomic(type) *)ptr, val, model);        \
     Lock *l = lock_for_pointer(ptr);                                           \
     lock(l);                                                                   \
     type tmp = *ptr;                                                           \
     *ptr = ~(tmp & val);                                                       \
     unlock(l);                                                                 \
     return tmp;                                                                \
   }
+#else
+#define ATOMIC_RMW_NAND(n, lockfree, type)                                     \
+  type __atomic_fetch_nand_##n(type *ptr, type val, int model) {               \
+    Lock *l = lock_for_pointer(ptr);                                           \
+    lock(l);                                                                   \
+    type tmp = *ptr;                                                           \
+    *ptr = ~(tmp & val);                                                       \
+    unlock(l);                                                                 \
+    return tmp;                                                                \
+  }
+#endif
 
 #define OPTIMISED_CASE(n, lockfree, type) ATOMIC_RMW(n, lockfree, type, add, +)
 OPTIMISED_CASES
 #undef OPTIMISED_CASE
 #define OPTIMISED_CASE(n, lockfree, type) ATOMIC_RMW(n, lockfree, type, sub, -)
 OPTIMISED_CASES
 #undef OPTIMISED_CASE
 #define OPTIMISED_CASE(n, lockfree, type) ATOMIC_RMW(n, lockfree, type, and, &)