Skip to content

Commit

Permalink
Disable use of _ExtInt with '__atomic' builtins
Browse files Browse the repository at this point in the history
We're (temporarily) disabling ExtInt for the '__atomic' builtins so we can better design their behavior later. The idea is until we do an audit/design for the way atomic builtins are supposed to work with _ExtInt, we should leave them restricted so they don't limit our future options, such as by binding us to a sub-optimal implementation via ABI.

Example after this change:

    $ cat test.c

        void f(_ExtInt(64) *ptr) {
          __atomic_fetch_add(ptr, 1, 0);
        }

    $ clang -c test.c

        test.c:2:22: error: argument to atomic builtin of type '_ExtInt' is not supported
          __atomic_fetch_add(ptr, 1, 0);
                             ^
        1 error generated.

Differential Revision: https://reviews.llvm.org/D84049
  • Loading branch information
jtmott-intel committed Aug 18, 2020
1 parent 3471520 commit ca77ab4
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 9 deletions.
7 changes: 4 additions & 3 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -6038,9 +6038,8 @@ def err_func_def_incomplete_result : Error<
def err_atomic_specifier_bad_type
: Error<"_Atomic cannot be applied to "
"%select{incomplete |array |function |reference |atomic |qualified "
"|sizeless ||integer |integer }0type "
"%1 %select{|||||||which is not trivially copyable|with less than "
"1 byte of precision|with a non power of 2 precision}0">;
"|sizeless ||integer }0type "
"%1 %select{|||||||which is not trivially copyable|}0">;

// Expressions.
def ext_sizeof_alignof_function_type : Extension<
Expand Down Expand Up @@ -7967,6 +7966,8 @@ def err_atomic_exclusive_builtin_pointer_size : Error<
" 1,2,4 or 8 byte type (%0 invalid)">;
def err_atomic_builtin_ext_int_size : Error<
"Atomic memory operand must have a power-of-two size">;
def err_atomic_builtin_ext_int_prohibit : Error<
"argument to atomic builtin of type '_ExtInt' is not supported">;
def err_atomic_op_needs_atomic : Error<
"address argument to atomic operation must be a pointer to _Atomic "
"type (%0 invalid)">;
Expand Down
5 changes: 5 additions & 0 deletions clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5050,6 +5050,11 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
? 0
: 1);

if (ValType->isExtIntType()) {
Diag(Ptr->getExprLoc(), diag::err_atomic_builtin_ext_int_prohibit);
return ExprError();
}

return AE;
}

Expand Down
5 changes: 1 addition & 4 deletions clang/lib/Sema/SemaType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8963,11 +8963,8 @@ QualType Sema::BuildAtomicType(QualType T, SourceLocation Loc) {
else if (!T.isTriviallyCopyableType(Context))
// Some other non-trivially-copyable type (probably a C++ class)
DisallowedKind = 7;
else if (auto *ExtTy = T->getAs<ExtIntType>()) {
if (ExtTy->getNumBits() < 8)
else if (T->isExtIntType()) {
DisallowedKind = 8;
else if (!llvm::isPowerOf2_32(ExtTy->getNumBits()))
DisallowedKind = 9;
}

if (DisallowedKind != -1) {
Expand Down
4 changes: 4 additions & 0 deletions clang/test/Sema/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -285,12 +285,16 @@ void test_ei_i42i(_ExtInt(42) *ptr, int value) {
__sync_fetch_and_add(ptr, value); // expected-error {{Atomic memory operand must have a power-of-two size}}
// expected-warning@+1 {{the semantics of this intrinsic changed with GCC version 4.4 - the newer semantics are provided here}}
__sync_nand_and_fetch(ptr, value); // expected-error {{Atomic memory operand must have a power-of-two size}}

__atomic_fetch_add(ptr, 1, 0); // expected-error {{argument to atomic builtin of type '_ExtInt' is not supported}}
}

void test_ei_i64i(_ExtInt(64) *ptr, int value) {
__sync_fetch_and_add(ptr, value); // expect success
// expected-warning@+1 {{the semantics of this intrinsic changed with GCC version 4.4 - the newer semantics are provided here}}
__sync_nand_and_fetch(ptr, value); // expect success

__atomic_fetch_add(ptr, 1, 0); // expected-error {{argument to atomic builtin of type '_ExtInt' is not supported}}
}

void test_ei_ii42(int *ptr, _ExtInt(42) value) {
Expand Down
5 changes: 3 additions & 2 deletions clang/test/SemaCXX/ext-int.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,11 @@ typedef _ExtInt(32) __attribute__((vector_size(16))) VecTy;
_Complex _ExtInt(3) Cmplx;

// Reject cases of _Atomic:
// expected-error@+1{{_Atomic cannot be applied to integer type '_ExtInt(4)' with less than 1 byte of precision}}
// expected-error@+1{{_Atomic cannot be applied to integer type '_ExtInt(4)'}}
_Atomic _ExtInt(4) TooSmallAtomic;
// expected-error@+1{{_Atomic cannot be applied to integer type '_ExtInt(9)' with a non power of 2 precision}}
// expected-error@+1{{_Atomic cannot be applied to integer type '_ExtInt(9)'}}
_Atomic _ExtInt(9) NotPow2Atomic;
// expected-error@+1{{_Atomic cannot be applied to integer type '_ExtInt(128)'}}
_Atomic _ExtInt(128) JustRightAtomic;

// Test result types of Unary/Bitwise/Binary Operations:
Expand Down
11 changes: 11 additions & 0 deletions libcxx/test/libcxx/atomics/ext-int.verify.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// REQUIRES: clang-11

#include <atomic>

int main(int, char**)
{
// expected-error@atomic:*1 {{_Atomic cannot be applied to integer type '_ExtInt(32)'}}
std::atomic<_ExtInt(32)> x {42};

return 0;
}

0 comments on commit ca77ab4

Please sign in to comment.