[PATCH] D17437: [OpenCL] Add Sema checks for types

[Xiuli PAN via cfe-commits]

pxli168 added inline comments.

================
Comment at: lib/Sema/SemaDecl.cpp:5725
@@ +5724,3 @@
+  // Pipes can only be passed as arguments to a function.
+  if (getLangOpts().OpenCL && getLangOpts().OpenCLVersion >= 200 &&
+      R->isPipeType()) {
----------------
Anastasia wrote:
> Remove 'getLangOpts().OpenCLVersion >= 200'. Parser will only allow pipe for CL2.0. Ideally we don't need to check getLangOpts().OpenCL either, but might be good to leave it for documentation purposes.
> 
> However could we merge with the previous:
>   if (getLangOpts().OpenCL) {
>    if (...) ... // image type
>    if (...) ... // pipe type
>   }
Ok, it should be more clear.

================
Comment at: lib/Sema/SemaDecl.cpp:10768
@@ -10746,1 +10767,3 @@
 
+  // OpenCL v2.0 s6.9b2 - An image type cannot be used to declare a variable, a
+  // structure or union field, an array of images, a pointer to an image, or the
----------------
Anastasia wrote:
> Here you only check the pointer and not the other bits. So please modify the comment according to what the code does.
> 
> Does the same restriction apply to other OpenCL types i.e. sampler, event, queue, etc? 
Good suggestion. I will have a look and try to add them.

================
Comment at: test/CodeGenOpenCL/opencl_types.cl:39
@@ -38,3 @@
-
-void __attribute__((overloadable)) bad1(image1d_t *b, image2d_t *c, image2d_t *d) {}
-// CHECK-LABEL: @{{_Z4bad1P11ocl_image1dP11ocl_image2dS2_|"\\01\?bad1@@\$\$J0YAXPE?APAUocl_image1d@@PE?APAUocl_image2d@@1 at Z"}}
----------------
Anastasia wrote:
> Could you please not remove this mangling test as it's not being tested elsewhere.
> 
> You can remove * from parameters though!
OK, I will try to make this mangle right for different target.

================
Comment at: test/SemaOpenCL/invalid-pipes-cl2.0.cl:9
@@ -8,1 +8,3 @@
 }
+void test4() {
+  pipe int p; // expected-error {{pipe can only be used as a function parameter}}
----------------
yaxunl wrote:
> Do we have a test for diagnosing pipe type for OCL < 2.0 ?
We have one called test/SemaOpenCL/pipes-1.2-negative.cl


http://reviews.llvm.org/D17437