[PATCH] D15914: [OpenCL] Pipe builtin functions

[Pekka Jääskeläinen via cfe-commits]

pekka.jaaskelainen added a comment.

I wonder could we squeeze this in before the next week's LLVM 3.8 branching? It would be great.


================
Comment at: lib/CodeGen/CGBuiltin.cpp:2033
@@ +2032,3 @@
+    else if (BuiltinID == Builtin::BIwork_group_reserve_write_pipe)
+      Name = "_Z29work_group_reserve_write_pipePU3AS110ocl_pipe_tji";
+    else if (BuiltinID == Builtin::BIsub_group_reserve_read_pipe)
----------------
Anastasia wrote:
> I am still not convinced we need to mangle here at all. It seems we mainly have only one prototype for those functions apart from read/write_pipe in which case we can generate two versions of that function:
> 
> read_pipe_2(...) - for two parameters case
> read_pipe_4(...) - for four parameters case
> 
> Having a mangler would complicate the implementation. And if used it would be better to adhere to conventional scheme  (i.e. calling getMangledName()). But that won't work here as we are generating parameters different from originally specified. I don't see any need or benefit in mangling at the moment since the function signatures are all known here.
> 
> Regarding the packet size, I prefer to pass it in. This way we can allow simple BIF implementation without using any metadata magic. Usage of metadata often implies adding non-conventional compilation mechanisms somewhere that are best to avoid.
We just need to remember that passing the packet size as an argument means touching all the user functions having pipes as arguments too.  

I'm still in the understanding that the compile time packet size info is needed only for optimization purposes, and thus could be a metadata which your implementation can just ignore for starters, or convert to a parameter before code emission? Touching the initial function finger prints for optimization reasons sounds a bit too intrusive. I'd rather leave the decision what to do with it to consumer of the Clang's output.

================
Comment at: lib/CodeGen/CGOpenCLRuntime.h:55
@@ -50,3 +54,3 @@
 };
 
 }
----------------
> Additionally how would definitions of builtin with user defined types appear in the BIF libraries?

This is a good question. read_pipe should just work for any type of any size, thus we cannot just generate a new function for all possible sizes in advance, thus what Anastasia suggests here makes sense to me:

> One approach would be to just generate calls that would always use generic types

If now there was an additional parameter (always a constant) that stores the type's size it would not help much as one would need to generate a big switch...case that optimizes the access based on the packet size in case of a software pipe or a compiler pass that looks into that argument and generates (a call to) an optimized version?

I think combining the Anastasia proposed generic read/write_pipe with the metadata (that points to the packet's inner type or its size?) would be the best solution (so far).


http://reviews.llvm.org/D15914