[PATCH] D118605: [OpenCL] Add support of language builtins for OpenCL C 3.0

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: clang/lib/Basic/Builtins.cpp:79
+  bool OclBlocksUnsupported =
+      (LangOpts.getOpenCLCompatibleVersion() < 200 || !LangOpts.Blocks) &&
+      (BuiltinInfo.Langs & OCL_BLOCKS);
----------------
svenvh wrote:
> azabaznov wrote:
> > This check is needed as //-cl-std=CL1.2// can be used together with //-fblocks//. But in 3.0 block support requires //__opencl_c_device_enqueue// feature.
> > This check is needed as -cl-std=CL1.2 can be used together with -fblocks.
> 
> That surprised me, I cannot find anything in the OpenCL 1.2 spec about this. @Anastasia do you know if this is an (undocumented?) Clang extension?
> 
> There are tests checking for this (e.g. `clang/test/Frontend/opencl.cl`), so we need this check to preserve the existing behavior indeed.
I think blocks is a clang language feature that can be enabled using `-fblocks` and activate extra functionality of blocks used as lambda only in C-based languages. But the `enqueue_kernel` is only supported from OpenCL 2.0 onwards since it requires functionality outside of frontend so I think this check is reasonable. I feel perhaps we should rename to something like:

`OCL_BLOCKS` -> `OCL_KERNEL_ENQUEUE` or `OCL_DSE`?

Since the builtins are not about blocks but about enqueueing kernels from device...

Also I would suggest a comment explaining that we check for `Blocks` because it is coupled with enqueue kernel functionality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118605