[PATCH] D15914: [OpenCL] Pipe builtin functions
Anastasia Stulova via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 14 03:42:55 PST 2016
Anastasia added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:1981
@@ +1980,3 @@
+ const char *Name =
+ (BuiltinID == Builtin::BIread_pipe) ? "read_pipe_2" : "write_pipe_2";
+ // Re-Creating the function type for this call, since the original type
> Shouldn't these have underscores, because they are not reserved OpenCL keywords/builtins? Cannot user functions be called read_pipe_2 or write_pipe_2?
Yes, definitely! Add prefix "__" to each generated name! I think this should be done for all of them!
Comment at: lib/Sema/SemaChecking.cpp:291
@@ +290,3 @@
+ bool isValid = true;
+ // TODO: Now we check for all the pipe builtin with access qualifier, but in
+ // OpenCL spec does not tell about group functions, need to fix after get
Please, remove this TODO. Have you created the bug to Khronos already?
It seems sensible enough the access qualifiers must match. How can reserve/commit_read_pipe be called with a write_only pipe?
Comment at: lib/Sema/SemaChecking.cpp:378
@@ +377,3 @@
+ S.Diag(Call->getLocStart(), diag::err_opencl_builtin_pipe_arg_num)
+ << getFunctionName(Call) << Call->getSourceRange();
+ return true;
I don't think we have any arguments there. It's all declared as variadic. But it seems to work without setting it. So I am not sure it's necessary to set it at all.
More information about the cfe-commits