[PATCH] D86510: [compiler-rt] Fix atomic support functions on 32-bit architectures

[Alexander Richardson via Phabricator via llvm-commits]

arichardson added a comment.

In D86510#2237437 <https://reviews.llvm.org/D86510#2237437>, @efriedma wrote:

>> This problem was found while compiling atomic.c for MIPS32 since the -Watomic-alignment warning was being triggered and objdump showed an undefined reference to _atomic_is_lock_free.
>
> Are you sure this fixes the issue properly?  If we're triggering -Watomic-alignment, probably the types are wrong somewhere.  (In particular, some targets support lock-free operations on `_Atomic long long`, but the ABI alignment of `long long` is lower than 8.)

We are triggering -Watomic-alignment because `__c11_atomic_is_lock_free(8)` expands to a `__atomic_is_lock_free` call for mips32 since the property is not known statically, so the branch inside the switch emitted and contains a call to `__atomic_load_8` which triggers the warning. With this patch we know that 8-byte atomics are not always lock free so the branch is not emitted and we fall back to the locking code.

> ------
>
> Have you seen https://reviews.llvm.org/D45321 ?

Thanks, I had not seen that review and it seems like it does almost the same thing, but uses the lock-free code in more cases.
It seems like both clang and GCC assume that atomic operations on pointers with unknown alignment are not always lock free: https://godbolt.org/z/T3oj1r
Therefore, this approach should be safe? And the optimization that checks the alignment and uses the atomic instructions instead of locks in the aligned case could be added later?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86510/new/

https://reviews.llvm.org/D86510