Evaluate __{c11_,}atomic_is_lock_free to 0 (avoid libcall) if larger than MaxAtomicPromoteWidth

[MaskRay]

Summary

MaxAtomicPromoteWidth is defined as "the maximum width lock-free atomic
operation which will ever be supported for the given target", so an
oversized __c11_atomic_is_lock_free() or __atomic_is_lock_free() can be
evaluated to 0. This is advantageous in some scenarios (e.g. FreeBSD
powerpc, see D71600) to avoid the dependency on libatomic.

The behavior of __atomic_is_lock_free() will diverge from GCC as GCC
never evaluates it to 0 (gcc/builtins.c:fold_builtin_atomic_always_lock_free).

[merge_guards_bot]

Unit tests: pass. 61771 tests passed, 0 failed and 780 were skipped.

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[nemanjai]

If I understand this correctly, this just evaluates the query for lock free atomics at compile time if the size is larger than the maximum possible size for an atomic on the target. If that's the case, this looks fine to me. But of course, some of the other target maintainers might feel otherwise.
The incompatibility with GCC might be an issue for some, but I don't expect this to be an issue at least on PPC.

[jfb]

This changes the expression to a constant expression as well, right? You should point this out in the commit message.

The divergence with GCC is unfortunate, @jwakely do you think you'd be able to get GCC to match this behavior as well? It's technically a breaking change, but I doubt it matters in practice. It might also break code when newer architectures come in (if they have larger atomics) but again I think that's fine.

[efriedma]

Eagerly evaluating based on MaxAtomicPromoteWidth seems fine... assuming we're actually setting MaxAtomicPromoteWidth to something appropriate. The value on PowerPC looks wrong.

If we're worried about constant-evaluating it in contexts where gcc doesn't, we can generate a diagnostic to prevent that from happening. (See the handling of Builtin::BIstrcmp.)

[MaskRay]

From https://gcc.gnu.org/ml/gcc/2020-02/msg00116.html

We discussed this on IRC in #gcc. There was no motivation to change GCC. The platform that wants to avoid the libatomic dependency doesn't use GCC anyway. Relying on not getting the libatomic dependency seems fragile (it only works for a specific codebase, and if some other is_lock_free check is added to the codebase, the libatomic dependency will return anyway).

So they will not extend the interface. Clang has getMaxAtomicPromoteWidth(). Do we need the extension? I'm happy with either way.

[jyknight]

This isn't correct.

The atomic interface is designed to be forward-compatible with new CPUs that have wider atomic instructions. That clang has a MaxAtomicPromoteWidth value _at all_ is unfortunate, because it breaks this ability. We've locked ourselves into an unfortunate ABI here -- and what's worse, it's incompatible with GCC's ABI.

But, the MaxAtomicPromoteWidth value impactsaffects C11 _Atomic (and therefore libc++ std::atomic) variables' default alignment. But, the user can in any case specify additional alignment. For the __atomic_is_lock_free function, MaxAtomicPromoteWidth is irrelevant -- it can be used on variables of any size or alignment and needs to return correct answers, even on future CPUs.

It's not good that clang doesn't provide the ability to create a working libatomic, but the right answer is to fix that, not add hacks like this. I've started writing up a proposal on what we need to do there, but need to finish that up.

clang/lib/AST/ExprConstant.cpp

case Builtin::BI__c11_atomic_is_lock_free: {
     // maximum alignment where we promote atomics, we know it is not lock-free
     // (at least not in the sense of atomic_is_lock_free).  Otherwise,
     // the answer can only be determined at runtime; for example, 16-byte
     // atomics have lock-free implementations on some, but not all,
     // x86-64 processors.
 
     // Check power-of-two.
     CharUnits Size = CharUnits::fromQuantity(SizeVal.getZExtValue());
+    if (Size > Info.Ctx.toCharUnitsFromBits(
+                   Info.Ctx.getTargetInfo().getMaxAtomicPromoteWidth()))
+      return Success(0, E);
     if (Size.isPowerOfTwo()) {
       // Check against inlining width.
       unsigned InlineWidthBits =
           Info.Ctx.getTargetInfo().getMaxAtomicInlineWidth();
       if (Size <= Info.Ctx.toCharUnitsFromBits(InlineWidthBits)) {
         if (BuiltinOp == Builtin::BI__c11_atomic_is_lock_free ||
             Size == CharUnits::One() ||
             E->getArg(1)->isNullPointerConstant(Info.Ctx,

clang/test/CodeGen/atomic-ops.c

 struct Incomplete;
 
 int lock_free(struct Incomplete *incomplete) {
   // CHECK-LABEL: @lock_free
 
   // CHECK: call i32 @__atomic_is_lock_free(i32 3, i8* null)
   __c11_atomic_is_lock_free(3);
 
-  // CHECK: call i32 @__atomic_is_lock_free(i32 16, i8* {{.*}}@sixteen{{.*}})
-  __atomic_is_lock_free(16, &sixteen);
-
-  // CHECK: call i32 @__atomic_is_lock_free(i32 17, i8* {{.*}}@seventeen{{.*}})
-  __atomic_is_lock_free(17, &seventeen);
+  // CHECK: call i32 @__atomic_is_lock_free(i32 8, i8* {{.*}}@seventeen{{.*}})
+  __atomic_is_lock_free(8, &seventeen);
 
   // CHECK: call i32 @__atomic_is_lock_free(i32 4, {{.*}})
   __atomic_is_lock_free(4, incomplete);
 
   char cs[20];
   // CHECK: call i32 @__atomic_is_lock_free(i32 4, {{.*}})
   __atomic_is_lock_free(4, cs+1);
 
   // CHECK-NOT: call
+  __atomic_is_lock_free(9, &sixteen);
+  __atomic_is_lock_free(16, 0);
   __atomic_always_lock_free(3, 0);
   __atomic_always_lock_free(16, 0);
   __atomic_always_lock_free(17, 0);
   __atomic_always_lock_free(16, &sixteen);
   __atomic_always_lock_free(17, &seventeen);
 
   int n;
   __atomic_is_lock_free(4, &n);

clang/test/CodeGen/big-atomic-ops.c

int lock_free(struct Incomplete *incomplete) {
   // CHECK: @lock_free
 
   // CHECK: call i32 @__atomic_is_lock_free(i64 3, i8* null)
   __c11_atomic_is_lock_free(3);
 
   // CHECK: call i32 @__atomic_is_lock_free(i64 16, i8* {{.*}}@sixteen{{.*}})
   __atomic_is_lock_free(16, &sixteen);
 
-  // CHECK: call i32 @__atomic_is_lock_free(i64 17, i8* {{.*}}@seventeen{{.*}})
-  __atomic_is_lock_free(17, &seventeen);
-
   // CHECK: call i32 @__atomic_is_lock_free(i64 4, {{.*}})
   __atomic_is_lock_free(4, incomplete);
 
   char cs[20];
   // CHECK: call i32 @__atomic_is_lock_free(i64 4, {{.*}})
   __atomic_is_lock_free(4, cs+1);
 
   // CHECK-NOT: call
+  __atomic_is_lock_free(17, &seventeen);
+  __atomic_is_lock_free(32, 0);
   __atomic_always_lock_free(3, 0);
   __atomic_always_lock_free(16, 0);
   __atomic_always_lock_free(17, 0);
   __atomic_always_lock_free(16, &sixteen);
   __atomic_always_lock_free(17, &seventeen);
 
   int n;
   __atomic_is_lock_free(4, &n);

clang/test/Sema/atomic-ops.c

 _Static_assert(__GCC_ATOMIC_LLONG_LOCK_FREE == 2, "");
 #endif
 _Static_assert(__GCC_ATOMIC_LLONG_LOCK_FREE == __CLANG_ATOMIC_LLONG_LOCK_FREE, "");
 _Static_assert(__GCC_ATOMIC_POINTER_LOCK_FREE == 2, "");
 _Static_assert(__GCC_ATOMIC_POINTER_LOCK_FREE == __CLANG_ATOMIC_POINTER_LOCK_FREE, "");
 
 _Static_assert(__c11_atomic_is_lock_free(1), "");
 _Static_assert(__c11_atomic_is_lock_free(2), "");
 _Static_assert(__c11_atomic_is_lock_free(3), ""); // expected-error {{not an integral constant expression}}
 _Static_assert(__c11_atomic_is_lock_free(4), "");
 _Static_assert(__c11_atomic_is_lock_free(8), "");
-_Static_assert(__c11_atomic_is_lock_free(16), ""); // expected-error {{not an integral constant expression}}
-_Static_assert(__c11_atomic_is_lock_free(17), ""); // expected-error {{not an integral constant expression}}
+_Static_assert(!__c11_atomic_is_lock_free(16), "");
+_Static_assert(!__c11_atomic_is_lock_free(17), "");
 
 _Static_assert(__atomic_is_lock_free(1, 0), "");
 _Static_assert(__atomic_is_lock_free(2, 0), "");
 _Static_assert(__atomic_is_lock_free(3, 0), ""); // expected-error {{not an integral constant expression}}
 _Static_assert(__atomic_is_lock_free(4, 0), "");
 _Static_assert(__atomic_is_lock_free(8, 0), "");
-_Static_assert(__atomic_is_lock_free(16, 0), ""); // expected-error {{not an integral constant expression}}
-_Static_assert(__atomic_is_lock_free(17, 0), ""); // expected-error {{not an integral constant expression}}
+_Static_assert(!__atomic_is_lock_free(16, 0), "");
+_Static_assert(!__atomic_is_lock_free(17, 0), "");
 
 _Static_assert(atomic_is_lock_free((atomic_char*)0), "");
 _Static_assert(atomic_is_lock_free((atomic_short*)0), "");
 _Static_assert(atomic_is_lock_free((atomic_int*)0), "");
 _Static_assert(atomic_is_lock_free((atomic_long*)0), "");
 // expected-error@+1 {{__int128 is not supported on this target}}
-_Static_assert(atomic_is_lock_free((_Atomic(__int128)*)0), ""); // expected-error {{not an integral constant expression}}
+_Static_assert(!atomic_is_lock_free((_Atomic(__int128)*)0), "");
 _Static_assert(atomic_is_lock_free(0 + (atomic_char*)0), "");
 
 char i8;
 short i16;
 int i32;
 int __attribute__((vector_size(8))) i64;
 struct Incomplete *incomplete; // expected-note {{forward declaration of 'struct Incomplete'}}
 
 _Static_assert(__atomic_is_lock_free(1, &i8), "");
 _Static_assert(__atomic_is_lock_free(1, &i64), "");
 _Static_assert(__atomic_is_lock_free(2, &i8), ""); // expected-error {{not an integral constant expression}}
 _Static_assert(__atomic_is_lock_free(2, &i16), "");
 _Static_assert(__atomic_is_lock_free(2, &i64), "");
 _Static_assert(__atomic_is_lock_free(4, &i16), ""); // expected-error {{not an integral constant expression}}
 _Static_assert(__atomic_is_lock_free(4, &i32), "");
 _Static_assert(__atomic_is_lock_free(4, &i64), "");
 _Static_assert(__atomic_is_lock_free(8, &i32), ""); // expected-error {{not an integral constant expression}}
 _Static_assert(__atomic_is_lock_free(8, &i64), "");
 
 _Static_assert(__atomic_always_lock_free(1, 0), "");
 _Static_assert(__atomic_always_lock_free(2, 0), "");
 _Static_assert(!__atomic_always_lock_free(3, 0), "");
 _Static_assert(__atomic_always_lock_free(4, 0), "");
 _Static_assert(__atomic_always_lock_free(8, 0), "");
 _Static_assert(!__atomic_always_lock_free(16, 0), "");

 
 #define _AS1 __attribute__((address_space(1)))
 #define _AS2 __attribute__((address_space(2)))
 
 void f(_Atomic(int) *i, const _Atomic(int) *ci,
        _Atomic(int*) *p, _Atomic(float) *d,
        int *I, const int *CI,
        int **P, float *D, struct S *s1, struct S *s2) {
   __c11_atomic_init(I, 5); // expected-error {{pointer to _Atomic}}
   __c11_atomic_init(ci, 5); // expected-error {{address argument to atomic operation must be a pointer to non-const _Atomic type ('const _Atomic(int) *' invalid)}}
 
   __c11_atomic_load(0); // expected-error {{too few arguments to function}}
   __c11_atomic_load(0,0,0); // expected-error {{too many arguments to function}}
   __c11_atomic_store(0,0,0); // expected-error {{address argument to atomic builtin must be a pointer}}
   __c11_atomic_store((int*)0,0,0); // expected-error {{address argument to atomic operation must be a pointer to _Atomic}}
   __c11_atomic_store(i, 0, memory_order_relaxed);
   __c11_atomic_store(ci, 0, memory_order_relaxed); // expected-error {{address argument to atomic operation must be a pointer to non-const _Atomic type ('const _Atomic(int) *' invalid)}}
 
   __c11_atomic_load(i, memory_order_seq_cst);
   __c11_atomic_load(p, memory_order_seq_cst);
   __c11_atomic_load(d, memory_order_seq_cst);
   __c11_atomic_load(ci, memory_order_seq_cst);
 
   int load_n_1 = __atomic_load_n(I, memory_order_relaxed);
   int *load_n_2 = __atomic_load_n(P, memory_order_relaxed);
   float load_n_3 = __atomic_load_n(D, memory_order_relaxed); // expected-error {{must be a pointer to integer or pointer}}
   __atomic_load_n(s1, memory_order_relaxed); // expected-error {{must be a pointer to integer or pointer}}
   load_n_1 = __atomic_load_n(CI, memory_order_relaxed);
 
   __atomic_load(i, I, memory_order_relaxed); // expected-error {{must be a pointer to a trivially-copyable type}}
   __atomic_load(CI, I, memory_order_relaxed);
 
   __atomic_load(I, i, memory_order_relaxed); // expected-warning {{passing '_Atomic(int) *' to parameter of type 'int *'}}
   __atomic_load(I, *P, memory_order_relaxed);
   __atomic_load(I, *P, memory_order_relaxed, 42); // expected-error {{too many arguments}}