[PATCH] D60763: Prototype OpenCL BIFs using Tablegen

Pierre via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 25 02:52:20 PDT 2019


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





More information about the cfe-commits mailing list