Anastasia added a comment.
A few nitpicks here, otherwise it seems we are in a good shape. I hope this
will hit trunk soon. :)
================
Comment at: lib/CodeGen/CGBuiltin.cpp:1966
@@ -1965,1 +1965,3 @@
}
+
+ case Builtin::BIread_pipe:
----------------
Can you put a comment with the spec reference here.
================
Comment at: lib/CodeGen/CGBuiltin.cpp:1974
@@ +1973,3 @@
+ // Type of the generic packet parameter.
+ llvm::Type *I8PTy = llvm::PointerType::get(llvm::Type::getInt8Ty(Ctxt),
4U);
+
----------------
Please, avoid hard coded constants.
You can use getTargetAddressSpace() method of ASTContext to obtain a generic AS
number, something like:
unsigned GenericAS =
getContext().getTargetAddressSpace(LangAS::opencl_generic);
llvm::Type Int8Ty = ...(..., GenericAS);
================
Comment at: lib/CodeGen/CGBuiltin.cpp:1978
@@ +1977,3 @@
+ if (2U == E->getNumArgs()) {
+ // The name is hard-coded for now, since we would like to avoid the
+ // AST rewrite for the infinite user define types.
----------------
I don't think this comment is any relevant and can be removed!
================
Comment at: lib/CodeGen/CGBuiltin.cpp:1983
@@ +1982,3 @@
+ // Re-Creating the function type for this call, since the original type
+ // is variadic, we make them to be generic type.
+ llvm::Type *ArgTys[] = {Arg0->getType(), I8PTy};
----------------
the type is not really variadic! May be better to say:
"Creating a generic function type to be able to call with any builtin or user
defined type."
================
Comment at: lib/Sema/SemaChecking.cpp:286
@@ +285,3 @@
+ getOpenCLArgAccess(cast<DeclRefExpr>(Arg0)->getDecl());
+ // Validates the access modifier is compatible with the call.
+ // From OpenCL C Specification 6.13.16 the access qualifiers for
----------------
modifier -> qualifier
================
Comment at: lib/Sema/SemaChecking.cpp:394
@@ +393,3 @@
+
+ // check the reverse size
+ if (!Call->getArg(1)->getType()->isIntegerType() &&
----------------
reverse -> reserve
================
Comment at: lib/Sema/SemaChecking.cpp:434
@@ +433,3 @@
+// \return True if a semantic error was found, false otherwise.
+static bool SemaBuiltinPipePacketes(Sema &S, CallExpr *Call) {
+ if (checkArgCount(S, Call, 1))
----------------
Packetes -> Packets
================
Comment at: lib/Sema/SemaChecking.cpp:754
@@ +753,3 @@
+ case Builtin::BIwrite_pipe:
+ // Since those two functions are declared with var args, therefore we need
+ // a semantic check for the argument.
----------------
remove "therefore"
================
Comment at: lib/Sema/SemaChecking.cpp:768
@@ +767,3 @@
+ // Since return type of reserve_read/write_pipe built-in function is
+ // reserve_id_t, which is not defined in the builtin def, we used int as
+ // return type and need to override the return type of these functions
----------------
builtin def -> builtin def file
http://reviews.llvm.org/D15914
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
