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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
