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

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
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'}}
----------------
Anastasia wrote:
> mantognini wrote:
> > 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.
> I think for the purpose of this test it is enough that at least one extension is not enabled. So we could add another run line and check for `cl_khr_int64_extended_atomics` too and also another run line with both although I am not sure it adds a lot into testing at the moment.
> 
> We can add a comment, so something like:
> 
> ```
> Optional type identifiers are not added in earlier version or if at least one of the extensions is not supported. Here we check with `cl_khr_int64_base_atomics` only.
> ```
Does it make sense?


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

https://reviews.llvm.org/D100976