[compiler-rt][builtins] Tweak checks for lock-free atomics

[luismarques]

Summary

The IS_LOCK_FREE_* macros in atomic.c were using the __c11_atomic_is_lock_free builtin. Those specific builtin calls would typically be lowered to compile-time constants (probably for all of the scenarios where compiler-rt was being used and providing atomic builtins). In some cases, though, the builtin can be lowered to a call to the runtime function __atomic_is_lock_free. An example is when targeting RISC-V rv32imac and it calls __c11_atomic_is_lock_free(8) by using the IS_LOCK_FREE_8 macro. Yet, atomic.c doesn't provide an implementation of __atomic_is_lock_free. In those scenarios you aren't able to link your program. To address this issue, this patch changes the macros to be implemented in terms of the __atomic_always_lock_free builtin, which is always lowered to a compile-time constant.

An alternative approach would be to implement the run-time function __atomic_is_lock_free, but right now that would only add run-time overhead. atomic.c implements the builtin runtime functions in terms of other atomic builtins, but the only relevant builtin here is __atomic_always_lock_free, so you might as well just use that builtin in the first place. If compiler-rt started to deviate from the current implementation approach and providing architecture-specific logic it's theoretically possible that a more fine-grained implementation of __atomic_is_lock_free could allow covering additional cases with lock-free implementations. If that ever comes to pass, we can revisit the implementation of the macros changed by this patch.

(This review supersedes D86043, due to issues with subscribing llvm-commits).

[arichardson]

Only just saw this after posting my own fix (D86510). This LGTM, and I can rebase my change to enable the 16-byte case on top of this.

[arichardson]

compiler-rt/lib/builtins/atomic.c

124

Actually, I'm not sure if passing NULL is correct, can __atomic_load be called with an under-aligned pointer?

[luismarques]

In D86281#2235413, @arichardson wrote:

Only just saw this after posting my own fix (D86510). This LGTM, and I can rebase my change to enable the 16-byte case on top of this.

Great minds think alike ;)
Your patch is a superset of mine, so we can just apply yours. I'll abandon this one.

Abandoning. Superceeded by D86510.

[luismarques]

compiler-rt/lib/builtins/atomic.c

124

Good question. At the time when I reasoned about that issue I concluded that it was correct. I remember checking that the assembly ouput of compiling atomic.c didn't change at all for e.g. X86 (that doesn't prove anything for the general case, but it's still a good sanity check). But thinking again about it I think you may be right -- or maybe I'm just forgetting the reasons why I had concluded it was fine. It doesn't matter now :)

compiler-rt/lib/builtins/atomic.c

static __inline Lock *lock_for_pointer(void *ptr) {
   // Now use the high(er) set of bits to perturb the hash, so that we don't
   // get collisions from atomic fields in a single object
   hash >>= 16;
   hash ^= low;
   // Return a pointer to the word to use
   return locks + (hash & SPINLOCK_MASK);
 }
 
-/// Macros for determining whether a size is lock free.  Clang can not yet
-/// codegen __atomic_is_lock_free(16), so for now we assume 16-byte values are
-/// not lock free.
-#define IS_LOCK_FREE_1 __c11_atomic_is_lock_free(1)
-#define IS_LOCK_FREE_2 __c11_atomic_is_lock_free(2)
-#define IS_LOCK_FREE_4 __c11_atomic_is_lock_free(4)
-#define IS_LOCK_FREE_8 __c11_atomic_is_lock_free(8)
-#define IS_LOCK_FREE_16 0
+/// Macros for determining whether a size is lock free.
+#define IS_LOCK_FREE_1 __atomic_always_lock_free(1, NULL)
+#define IS_LOCK_FREE_2 __atomic_always_lock_free(2, NULL)
+#define IS_LOCK_FREE_4 __atomic_always_lock_free(4, NULL)
+#define IS_LOCK_FREE_8 __atomic_always_lock_free(8, NULL)
+#define IS_LOCK_FREE_16 __atomic_always_lock_free(16, NULL)
 
 /// Macro that calls the compiler-generated lock-free versions of functions
 /// when they exist.
 #define LOCK_FREE_CASES()                                                      \
   do {                                                                         \
     switch (size) {                                                            \
     case 1:                                                                    \
       if (IS_LOCK_FREE_1) {                                                    \