Anastasia added inline comments.

================
Comment at: clang/test/Misc/opencl-c-3.0.incorrect_options.cl:21
+
 // CHECK-FP64: error: options cl_khr_fp64 and __opencl_c_fp64 are set to 
different values
 
----------------
Anastasia wrote:
> I can't remember if we have discussed this already, but could we use 
> `-verify` for these errors?
We should be able to remove `FileCheck` and replace `CHECK` directives with 
something like:
`//expected-error@*{{options cl_khr_fp64 and __opencl_c_fp64 are set to 
different values}}`


================
Comment at: clang/test/SemaOpenCL/invalid-device-enqueue-types-cl3.0.cl:5
+void f() {
+  clk_event_t e;
+  queue_t q;
----------------
azabaznov wrote:
> Anastasia wrote:
> > I know that many test have prefix "invalid" but I feel we have failed to 
> > establish the meaning for it because most of the tests in Sema are testing 
> > some sort of invalid behavior. But also here I feel that we should test 
> > that `enqueue_kernel` is not supported?
> > 
> > Do you think we chould merge this testing together with 
> > `SemaOpenCL/cl20-device-side-enqueue.cl` with some filename renaming?
> > 
> > Technically we should do the same testing even for CL1.x versions...
> I think it can. But `enqueu_kernel` is a `LangBuiltin` and still not 
> supported yet. Support for `LangBuiltins`  is going to be added in a separate 
> patch: with this patch all of the features that affect language built-ins are 
> supported.
Ok, then would it still work if we merge this functionality 
`SemaOpenCL/cl20-device-side-enqueue.cl` and the rest can be added later on...


================
Comment at: clang/test/SemaOpenCL/storageclass.cl:2
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL1.2
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 
-cl-ext=-__opencl_c_program_scope_global_variables,-__opencl_c_generic_address_space,-__opencl_c_pipes
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 
-cl-ext=+__opencl_c_program_scope_global_variables,-__opencl_c_generic_address_space,-__opencl_c_pipes
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 
-cl-ext=-__opencl_c_program_scope_global_variables,+__opencl_c_generic_address_space
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 
-cl-ext=-__opencl_c_program_scope_global_variables,-__opencl_c_generic_address_space,-__opencl_c_pipes,-__opencl_c_device_enqueue
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 
-cl-ext=+__opencl_c_program_scope_global_variables,-__opencl_c_generic_address_space,-__opencl_c_pipes,-__opencl_c_device_enqueue
----------------
Anastasia wrote:
> These lines are getting a bit longer. Do we actually need to set 
> `-__opencl_c_device_enqueue` for this test? Same for some other tests...
ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115640

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to