[llvm-bugs] [Bug 60572] Is disallowing loading an `_Atomic(...)` type with `__atomic_load` intended?

[LLVM Bugs via llvm-bugs]

Issue	60572
Summary	Is disallowing loading an `_Atomic(...)` type with `__atomic_load` intended?
Labels	new issue
Assignees
Reporter	etcwilde

    I have the following example snippet, which compiles with GCC, but not with Clang. 
(https://godbolt.org/z/7PbjeTn1x)

```c
#include <stdatomic.h>
int main(int argc, char ** argv) {
  _Atomic int a;
 int value;
  __atomic_load(&a, &value, memory_order_relaxed);
  return 0;
}
```

Clang emits the following error message; `<source>:5:3: error: address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(int) *' invalid)`

This is being emitted from `Sema::BuildAtomicExpr` in `SemaChecking.cpp`;
```cpp
if (!IsC11 && !AtomTy.isTriviallyCopyableType(Context) &&
 !AtomTy->isScalarType()) {
    // For GNU atomics, require a trivially-copyable type. This is not part of
    // the GNU atomics specification, but we enforce it for sanity.
 Diag(ExprRange.getBegin(), diag::err_atomic_op_needs_trivial_copy)
 << Ptr->getType() << Ptr->getSourceRange();
    return ExprError();
 }
```
`AtomTy` is
```c
AtomicType 0x1258d10c0 '_Atomic(int)'
`-BuiltinType 0x1258417b0 'int'
```

I tried adding a clause to `QualType::isTriviallyCopyableType` and `Type::isScalarType` to look through the `AtomicType` at the underlying `ValueType`, but that resulted in some test failures. (Most are in C++ tests, which I'm less concerned with at the moment since I don't think the `_Atomic` spelling is even a thing in C++). The most prevalent is the failure in `Sema/atomic-ops.c`;
```c
void f(_Atomic(int) *i, const _Atomic(int) *ci,
       _Atomic(int*) *p, _Atomic(float) *f, _Atomic(double) *d,
 _Atomic(long double) *ld,
       int *I, const int *CI,
       int **P, float *D, struct S *s1, struct S *s2) {
  ...
  __atomic_load(i, I, memory_order_relaxed); // expected-error {{must be a pointer to a trivially-copyable type}}
  ...
}
```

Is this discrepancy intentional? I'm afraid I'm not familiar enough with the C atomic types to know what the desired behavior is. `IsC11` is defined as 

```cpp
bool IsC11 = (Op >= AtomicExpr::AO__c11_atomic_init &&
 Op <= AtomicExpr::AO__c11_atomic_fetch_min) ||
 IsOpenCL;
 ```

Would it make more sense to include a GNU atomics check instead of looking through the `_Atomic` type? 
```cpp
bool IsGNU = (Op >= AtomicExpr::AO__atomic_load &&
              Op <= AtomicExpr::AO__atomic_nand_fetch)
```

Thanks!

_______________________________________________
llvm-bugs mailing list
llvm-bugs@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs