Anastasia added a comment.
Do you have any tests? We won't be able to commit without. Also having them
would help understanding what this modification does.
================
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)
----------------
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"
================
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)
----------------
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.
================
Comment at: lib/Sema/SemaChecking.cpp:268
@@ +267,3 @@
+static OpenCLImageAccessAttr *getOpenCLImageAcces(const Decl *D) {
+ if (D->hasAttr<OpenCLImageAccessAttr>())
+ return D->getAttr<OpenCLImageAccessAttr>();
----------------
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.
================
Comment at: lib/Sema/SemaChecking.cpp:282
@@ +281,3 @@
+ }
+ OpenCLImageAccessAttr *AccessQual =
+ getOpenCLImageAcces(cast<DeclRefExpr>(Arg0)->getDecl());
----------------
Ah yes, I see. It should be fine!
http://reviews.llvm.org/D15914
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
