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@-16 {{use of undeclared identifier 'atomic_int'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_uint'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_long'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_ulong'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_float'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_double'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_flag'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_intptr_t'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_uintptr_t'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_size_t'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_ptrdiff_t'}}
-#elif !EXT
-// expected-error@-26 {{use of type 'atomic_long' (aka '_Atomic(long)')
requires cl_khr_int64_base_atomics support}}
-// expected-error@-27 {{use of type 'atomic_long' (aka '_Atomic(long)')
requires cl_khr_int64_extended_atomics support}}
-// expected-error@-27 {{use of type 'atomic_ulong' (aka '_Atomic(unsigned
long)') requires cl_khr_int64_base_atomics support}}
-// expected-error@-28 {{use of type 'atomic_ulong' (aka '_Atomic(unsigned
long)') requires cl_khr_int64_extended_atomics support}}
-// expected-error@-27 {{use of type 'atomic_double' (aka '_Atomic(double)')
requires cl_khr_int64_base_atomics support}}
-// expected-error@-28 {{use of type 'atomic_double' (aka '_Atomic(double)')
requires cl_khr_int64_extended_atomics support}}
-#if __LP64__
-// expected-error-re@-28 {{use of type 'atomic_intptr_t' (aka
'_Atomic({{.+}})') requires cl_khr_int64_base_atomics support}}
-// expected-error-re@-29 {{use of type 'atomic_intptr_t' (aka
'_Atomic({{.+}})') requires cl_khr_int64_extended_atomics support}}
-// expected-error-re@-29 {{use of type 'atomic_uintptr_t' (aka
'_Atomic({{.+}})') requires cl_khr_int64_base_atomics support}}
-// expected-error-re@-30 {{use of type 'atomic_uintptr_t' (aka
'_Atomic({{.+}})') requires cl_khr_int64_extended_atomics support}}
-// expected-error-re@-30 {{use of type 'atomic_size_t' (aka '_Atomic({{.+}})')
requires cl_khr_int64_base_atomics support}}
-// expected-error-re@-31 {{use of type 'atomic_size_t' (aka '_Atomic({{.+}})')
requires cl_khr_int64_extended_atomics support}}
-// expected-error-re@-31 {{use of type 'atomic_ptrdiff_t' (aka
'_Atomic({{.+}})') requires cl_khr_int64_base_atomics support}}
-// expected-error-re@-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@-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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
