[PATCH] D15603: [OpenCL] Pipe type support

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

pekka.jaaskelainen added inline comments.

================
Comment at: lib/CodeGen/CGOpenCLRuntime.cpp:108
@@ +107,3 @@
+    PipeTy = llvm::PointerType::get(llvm::StructType::create(
+      CGM.getLLVMContext(), "opencl.pipe_t"), PipeAddrSpc);
+  }
----------------
pxli168 wrote:
> pekka.jaaskelainen wrote:
> > I'm not sure if touching the built-in fingerprints for this is a good idea. It might lead to problems and confusion. Cannot one pass pipes as arguments to user functions too? Are the fingerprints of those functions then modified accordingly? It might become messy.
> > 
> > Because the packet size is known at host side, the pipes can be implemented as structs which holds the packet size as one of the member variables. The problem with this approach is how to exploit wider reads/writes instead of a scalar read/write loop + unpack/pack in case of vector datatypes. 
> > 
> > If the size is known only at runtime, one cannot easily generate vector reads/writes even if the element is a vector datatype and it would be efficient to keep the packet in a vector register as long as possible. For helping this I'd add a metadata which can be utilized at compile time to make reading/writing from the pipe faster.  But in a way that is already an optimization, not a requirement, to make pipes work.
> > 
> > The reading itself is platform dependent as the pipe can be even a hardware FIFO accessed using special instructions.
> This is what I'm worry about, I don't think we need to give much information about an opaque type in OpenCL.
> 
> Actually we can get the element type from the metadata, and I think we can leave the optimization to the backend and let platform to choose which way to use for read/write pipe.
> 
> And I think the built-in function support for the pipe in OpenCL-C is not necessary in the clang, what do you think? Though it can do some check in Sema check, they can also be done in some llvm pass in backend. If the built-in is really needed, I will send another patch based on this included built-in support for pipe.
> 
> Thank you.
As far as I've understood, no there's no need to add built-in function awareness to the frontend for this case. Sema checking/diagnostics is needed to ensure pipes are used only as function arguments, not local variables, for example. Is this patch missing it?


http://reviews.llvm.org/D15603