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

[Luís Marques via Phabricator via llvm-commits]

luismarques created this revision.
luismarques added reviewers: asb, lenary, jfb, jyknight, eli.friedman.
Herald added subscribers: Sanitizers, s.egerton, PkmX, dexonsmith, simoncook, dberris.
Herald added a project: Sanitizers.
luismarques requested review of this revision.

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 <https://reviews.llvm.org/D86043>, due to issues with subscribing llvm-commits).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86281

Files:
  compiler-rt/lib/builtins/atomic.c


Index: compiler-rt/lib/builtins/atomic.c
===================================================================
--- compiler-rt/lib/builtins/atomic.c
+++ compiler-rt/lib/builtins/atomic.c
@@ -120,14 +120,12 @@
   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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D86281.286788.patch
Type: text/x-patch
Size: 1124 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200820/dcebbe1d/attachment.bin>