mantognini 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@-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'}}
----------------
Anastasia wrote:
> 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?
Yes, I think it does.
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
