Anastasia added inline comments.

================
Comment at: clang/test/Parser/opencl-atomics-cl20.cl:7-8
 
-#ifdef EXT
-#pragma OPENCL EXTENSION cl_khr_int64_base_atomics:enable
-#pragma OPENCL EXTENSION cl_khr_int64_extended_atomics:enable
-#pragma OPENCL EXTENSION cl_khr_fp64:enable
-#if __OPENCL_C_VERSION__ >= CL_VERSION_1_2
-// expected-warning@-2{{OpenCL extension 'cl_khr_fp64' is core feature or 
supported optional core feature - ignoring}}
-#endif
+#if defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= CL_VERSION_1_2
+#define LANG_VER_OK
 #endif
----------------
azabaznov wrote:
> Mauby simply //-DLANG_VER_OK// when running tests?
I think defining it in the test source is better for future modifications. Then 
we could add as many extra RUN lines as possible without the need to pass extra 
-D flag. Also it is kind of redundant since we already have the language 
version defined while parsing.


================
Comment at: clang/test/Parser/opencl-atomics-cl20.cl:51-56
+// expected-error@-21 {{expected ';' after expression}}
+// expected-error@-22 {{use of undeclared identifier 's'}}
+// expected-error@-22 {{unknown type name 'atomic_intptr_t'; did you mean 
'atomic_int'?}}
+// expected-note@* {{'atomic_int' declared here}}
+// expected-error@-23 {{unknown type name 'atomic_uintptr_t'; did you mean 
'atomic_uint'?}}
+// expected-note@* {{'atomic_uint' declared here}}
----------------
azabaznov wrote:
> Well, this is exactly what I was afraid of last time, see 
> https://reviews.llvm.org/D97058#2602677. Is this for sure a right way to go 
> forward? Also, **declared //here//** for implicit type definitions is pretty 
> confusing. Maybe a way the diagnostics showed here is just a bug?
This is a standard diagnostic from clang fixit if the typename is similar to 
any it can find in a symbol table of valid identifiers.


```
opencl-atomics-cl20.cl:31:3 error: unknown type name 'atomic_intptr_t'; did you 
mean 'atomic_int'?
atomic_intptr_t ip;
^~~~~~~~~~~~~~~
atomic_int
note: 'atomic_uint' declared here
```

For such implicit typedefs we won't be able to display the source locations as 
there is no physical source. However this can occur in any kernel code compiled 
with type names similar to those defined by implicit typedefs i.e. I am not 
changing this in the patch. 

https://godbolt.org/z/j7vn6d4Kh

But I agree that this is not ideal - we could solve it by not printing such 
hints at all if source location doesn't exist but this would probably need to 
be agreed with the community as this should probably be done for all languages. 
We could create a clang bug to follow up on this? 


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

Reply via email to