pekka.jaaskelainen added inline comments.

================
Comment at: lib/CodeGen/CGBuiltin.cpp:2033
@@ +2032,3 @@
+          *Arg1 = EmitScalarExpr(E->getArg(1));
+    llvm::Type *ReservedIDTy = ConvertType(getContext().OCLReserveIDTy);
+
----------------
Anastasia wrote:
> Why do we need to modify user defined functions? Only builtins will have this 
> extra parameter.
> 
> I think packet size would be useful for builtin implementation to know the 
> number of bytes to be read/written. I don't see how to implement it correctly 
> otherwise. As mentioned earlier relying on the metadata is not a conventional 
> compilation approach and should be avoided if possible.
The pipe struct can have the packet size in its header before the actual ring 
buffer or whatever, which can be used as a fallback unless the compiler can 
optimize it to a larger access. Correct implementation thus doesn't require a 
"hidden parameter". Adding it as a compile time hidden constant argument should 
help the optimizers, that's of course true, but I don't think it's strictly 
required.

If you think having a special behavior for the built-ins calls isn't 
problematic, then fine, I'm not having so strong opinion on this.


http://reviews.llvm.org/D15914



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

Reply via email to