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

Fri Feb 4 07:33:48 PST 2022

Anastasia added inline comments.

================
Comment at: clang/include/clang/Basic/Builtins.h:39
+  COR_LANG = 0x80,     // builtin requires use of 'fcoroutine-ts' option.
+  OCLC2P_LANG = 0x100, // builtin for OpenCL C 2.0+ versions.
+  OCLC1X_LANG = 0x200, // builtin for OpenCL C 1.x only.
----------------
I am thinking that we probably don't need this combined mode now... I think it is enough to just have GAS, PIPES and BLOCKS separately. Then in `Builtin::Context::builtinIsSupported` we will have 3 separate checks against `LangOpts.OpenCLGenericAddressSpace`, `LangOpts.OpenCLPipes` and `LangOpts.Blocks` as they can be used for both OpenCL 2.0 and OpenCL 3.0. This could simplify this list.

Then we can also remove `OCL2P_WITH_GAS`, `OCL2P_WITH_PIPES` and `OCL2P_WITH_BLOCKS`.

================
Comment at: clang/include/clang/Basic/Builtins.h:40
+  OCLC2P_LANG = 0x100, // builtin for OpenCL C 2.0+ versions.
+  OCLC1X_LANG = 0x200, // builtin for OpenCL C 1.x only.
+  OCL_GAS = 0x400,     // builtin requires OpenCL generic address space.
----------------
Btw is this field needed? I can't see any builtins that are using this... If it's unused we should just remove this entry.

================
Comment at: clang/include/clang/Basic/Builtins.h:43
+  OCL_PIPE = 0x800,    // builtin requires OpenCL pipe.
+  BLOCKS = 0x1000,     // builtin requires blocks.
   ALL_LANGUAGES = C_LANG | CXX_LANG | OBJC_LANG, // builtin for all languages.
----------------
let's use `OCL_BLOCKS` since `enqueue_kernel`, etc are OpenCL-only builtins.

================
Comment at: clang/include/clang/Basic/Builtins.h:53
+      OCLC2P_LANG | BLOCKS, // builtin requires OpenCL C 2.0+ and blocks.
+  ALL_OCLC_LANGUAGES = OCLC1X_LANG | OCLC2P_LANG // builtin for OCLC languages.
 };
----------------
For consistency `ALL_OCLC_LANGUAGES` -> `ALL_OCL_LANGUAGES`

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118605