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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to