[PATCH] D15914: [OpenCL] Pipe builtin functions

[Xiuli PAN via cfe-commits]

pxli168 added a comment.

I think we'd better to ask about to add this in. The pipe type support actually have no use without builtins to read or write it.


================
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
----------------
Anastasia wrote:
> pekka.jaaskelainen wrote:
> > 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!
I think that's a good idea!

================
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
----------------
Anastasia wrote:
> 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?
OK, I will. I tried to create an account to do, but it seems I could not receive the email. I will attach the bug report web here once I created one. 

================
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;
----------------
Anastasia wrote:
> 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. 
The return type of int and uint is set in the builtin.def.


http://reviews.llvm.org/D15914