[PATCH] D17436: [OpenCL] Add Sema checks for OpenCL 2.0 block

[Xiuli PAN via cfe-commits]

pxli168 marked 11 inline comments as done.
pxli168 added a comment.

Block is an OpenCL v2.0 feature, I think all test should be handled only for CL2.0 or newer version.


================
Comment at: lib/Sema/SemaDecl.cpp:6714
@@ +6713,3 @@
+  // supported in OpenCL C: Blocks with variadic arguments.
+  if (getLangOpts().OpenCL && LangOpts.OpenCLVersion >= 200 &&
+      T->isBlockPointerType()) {
----------------
Anastasia wrote:
> getLangOpts() -> LangOpts
> 
> Could you remove 'LangOpts.OpenCLVersion >= 200' because we can also enable Blocks with -fblocks flag.
But this is a OpenCL v2.0 restriction, if we do not check about OpenCL version it seems meaningless.

================
Comment at: lib/Sema/SemaExpr.cpp:6525
@@ +6524,3 @@
+    // should output error for both LHS and RHS, use | instead ||
+    if (checkBlockType(*this, LHS.get()) | checkBlockType(*this, RHS.get()))
+      return QualType();
----------------
Anastasia wrote:
> I suggest to unify with another check for OpenCL on line 6497 to have OpenCL bits all in one place.
It seems move it to there may change the order of checking condition first.

================
Comment at: test/SemaOpenCL/invalid-block.cl:16
@@ +15,3 @@
+
+void f2(BlkInt *BlockPtr) {
+  BlkInt B = ^int(int I) {return 1;};
----------------
Anastasia wrote:
> Actually I think pointers to blocks have to be disallowed too, but spec doesn't mention that although it forbids the dereferencing.
> 
> I will submit a bug to Khronos to clarify this.
I think it is reasonable, please add me in cc list. I will try to make a change here. But this may make deference test case hard to write.

BTW, the qualifier to workgroup pipe builtin functions is resolved by https://cvs.khronos.org/bugzilla/show_bug.cgi?id=15541.



http://reviews.llvm.org/D17436