pxli168 added inline comments. ================ Comment at: include/clang/Basic/Builtins.def:1260 @@ +1259,3 @@ + +LANGBUILTIN(reserve_read_pipe, "i.", "tn", OCLC_LANG) +LANGBUILTIN(reserve_write_pipe, "i.", "tn", OCLC_LANG) ---------------- Anastasia wrote: > That's right! I am just saying that we don't have to make it variadic, so you > can change "v." -> "v" and "i." -> "i" If that means there is no arguments, and may be misleading.
================ 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) ---------------- pekka.jaaskelainen wrote: > 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. That could be fine I think, the function name do not need mangle if we use generic type. ================ Comment at: lib/Sema/SemaChecking.cpp:268 @@ +267,3 @@ +static OpenCLImageAccessAttr *getOpenCLImageAcces(const Decl *D) { + if (D->hasAttr<OpenCLImageAccessAttr>()) + return D->getAttr<OpenCLImageAccessAttr>(); ---------------- Anastasia wrote: > Let's rename this new function in this commit and I agree with renaming an > attribute later on. May be leaving TODO explaining that would be good. OK, it will make the function more easy to understand. And I will add a FIXME or TODO here to explain the misleading imageaccess. And how do you think about the access check for the work_group functions? The specification does not say clearly, but it seems they should also obey the same rule with read/write_pipe. http://reviews.llvm.org/D15914 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits