[PATCH] D100976: [OpenCL] Simplify use of C11 atomic types

[Marco Antognini via Phabricator via cfe-commits]

mantognini added a comment.

> This change removes the requirement on pragma when atomic types from the extensions are supported because the behavior is not conformant. With this change, the developers can use atomic types from the extensions if they are supported without enabling the pragma because disabling the pragma doesn't do anything useful but only prevents the use of already available identifiers for types and issues extra diagnostics. This makes semantics consistent with the atomic functions that are also available when the extension is supported without any need for the pragma.

That sounds reasonable to me. Decreasing code complexity and reducing the "surprise-factor" for users by being more consistent is good indeed.



================
Comment at: clang/test/Parser/opencl-atomics-cl20.cl:12
 void atomic_types_test() {
-// OpenCL v2.0 s6.13.11.6 defines supported atomic types.
+  // OpenCL v2.0 s6.13.11.6 defines supported atomic types.
+
----------------
nit: there are a few tabs in this files you may want to remove before submitting.


================
Comment at: clang/test/Parser/opencl-atomics-cl20.cl:34
   atomic_ptrdiff_t pd;
-// OpenCL v2.0 s6.13.11.8, _Atomic type specifier and _Atomic type qualifier
-// are not supported by OpenCL.
-  _Atomic int i; // expected-error {{use of undeclared identifier '_Atomic'}}
-}
-#ifndef CL20
-// expected-error at -16 {{use of undeclared identifier 'atomic_int'}}
-// expected-error at -16 {{use of undeclared identifier 'atomic_uint'}}
-// expected-error at -16 {{use of undeclared identifier 'atomic_long'}}
-// expected-error at -16 {{use of undeclared identifier 'atomic_ulong'}}
-// expected-error at -16 {{use of undeclared identifier 'atomic_float'}}
-// expected-error at -16 {{use of undeclared identifier 'atomic_double'}}
-// expected-error at -16 {{use of undeclared identifier 'atomic_flag'}}
-// expected-error at -16 {{use of undeclared identifier 'atomic_intptr_t'}}
-// expected-error at -16 {{use of undeclared identifier 'atomic_uintptr_t'}}
-// expected-error at -16 {{use of undeclared identifier 'atomic_size_t'}}
-// expected-error at -16 {{use of undeclared identifier 'atomic_ptrdiff_t'}}
-#elif !EXT
-// expected-error at -26 {{use of type 'atomic_long' (aka '_Atomic(long)') requires cl_khr_int64_base_atomics support}}
-// expected-error at -27 {{use of type 'atomic_long' (aka '_Atomic(long)') requires cl_khr_int64_extended_atomics support}}
-// expected-error at -27 {{use of type 'atomic_ulong' (aka '_Atomic(unsigned long)') requires cl_khr_int64_base_atomics support}}
-// expected-error at -28 {{use of type 'atomic_ulong' (aka '_Atomic(unsigned long)') requires cl_khr_int64_extended_atomics support}}
-// expected-error at -27 {{use of type 'atomic_double' (aka '_Atomic(double)') requires cl_khr_int64_base_atomics support}}
-// expected-error at -28 {{use of type 'atomic_double' (aka '_Atomic(double)') requires cl_khr_int64_extended_atomics support}}
-#if __LP64__
-// expected-error-re at -28 {{use of type 'atomic_intptr_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_base_atomics support}}
-// expected-error-re at -29 {{use of type 'atomic_intptr_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics support}}
-// expected-error-re at -29 {{use of type 'atomic_uintptr_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_base_atomics support}}
-// expected-error-re at -30 {{use of type 'atomic_uintptr_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics support}}
-// expected-error-re at -30 {{use of type 'atomic_size_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_base_atomics support}}
-// expected-error-re at -31 {{use of type 'atomic_size_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics support}}
-// expected-error-re at -31 {{use of type 'atomic_ptrdiff_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_base_atomics support}}
-// expected-error-re at -32 {{use of type 'atomic_ptrdiff_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics support}}
+#if !defined(LANG_VER_OK) || !defined(cl_khr_int64_base_atomics)
+// expected-error at -8 {{use of undeclared identifier 'atomic_long'}}
----------------
Shouldn't that be 
```
!(defined(cl_khr_int64_extended_atomics) && defined(cl_khr_int64_base_atomics))
```
for atomic_long and atomic_ulong, and covering double for atomic_double?

Alternatively, if the goal isn't to have 100% coverage in this test of all these variations (which would be fine I believe), then a comment could clarify the intent.


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

https://reviews.llvm.org/D100976