[PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.

[Anastasia Stulova via cfe-commits]

Anastasia added inline comments.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:501
@@ -500,1 +500,3 @@
 def err_builtin_needs_feature : Error<"%0 needs target feature %1">;
+def err_builtin_needs_opencl_version
+    : Error<"%0 needs OpenCL version %1%select{| or above}2">;
----------------
Could we move this to all the other OpenCL diagnostics to have them grouped together?

================
Comment at: lib/Sema/SemaChecking.cpp:463
@@ +462,3 @@
+// \return True if a semantic error has been found, false otherwise.
+static bool SemaBuiltinToAddr(Sema &S, unsigned BuiltinID, CallExpr *Call) {
+  // OpenCL v2.0 s6.13.9 - Address space qualifier functions.
----------------
Could we rename this please to SemaOpenCLBuiltinToAddr.

I think Pipe functions should be better renamed too at some point.

================
Comment at: lib/Sema/SemaChecking.cpp:478
@@ +477,3 @@
+  auto RT = Call->getArg(0)->getType();
+  if (!RT->isPointerType() || RT->getPointeeType().getCanonicalType()
+      .getQualifiers().getAddressSpace() == LangAS::opencl_constant) {
----------------
Could the second check be written a bit simpler? I have a feeling something like this might work too:
  RT->getPointeeType().getAddressSpace()


================
Comment at: lib/Sema/SemaChecking.cpp:485
@@ +484,3 @@
+
+  RT = RT->getPointeeType().getCanonicalType();
+  auto Qual = RT.getQualifiers();
----------------
Is canonical type really necessary here?


http://reviews.llvm.org/D19932