[PATCH] D60763: Prototype OpenCL BIFs using Tablegen

[Pierre via Phabricator via cfe-commits]

Pierre added a comment.

Other comments:
1-
When a header file is included, its function declarations are decorated with the "nounwind" attribute, meaning that the function is not supposed to throw an exception. This decorator is currently not added with the new mechanism.
The "readnone" decorator is also present for these builtin functions and added somewhere y clang, but not added with the new mechanism.
This can be tested with the following command line, on an .cl file containing a function calling a builtin function (e.g.: acos):

  clang -cc1 -cl-std=CL2.0 -emit-llvm -O0 -I<path_to>/llvm-project/clang/lib/Headers <your_file.cl>  [-fadd-opencl-builtins|-finclude-default-header]

2-
When the function definition is inserted, it seems to be actually just resolving the identifier for the current lookup. Calling the same builtin function multiple times currently result in multiple lookup. Maybe there is a way to add the function declaration for the scope/file, so the lookup is only performed one time for each function. This part is done around the code taken from Sema::LazilyCreateBuiltin function, and I will spend more time on it.
3-
I haven't looked at the test file yet.
4-
If you have any suggestion/comment, feel free to share.



================
Comment at: clang/include/clang/Basic/OpenCLBuiltins.td:65
+  def float_t  : Type<"float">;
+  def double_t : Type<"double">;
+}
----------------
Anastasia wrote:
> half?
The signature of OpenCL builtin functions taking/returning half types are not part of OpenCL by default, this is part of OpenCL 2.0 extensions (cf https://www.khronos.org/registry/OpenCL/specs/opencl-2.0-extensions.pdf)


================
Comment at: clang/lib/Sema/SemaLookup.cpp:675
 
+// TODO: Auto-generate this from tablegen
+static QualType OCL2Qual(ASTContext &Context, OpenCLType Ty) {
----------------
Anastasia wrote:
> Does this mean we have to produce this in `ClangOpenCLBuiltinEmitter.cpp`?
Yes I think so, I have made a change in this way


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

https://reviews.llvm.org/D60763