[PATCH] D16876: [OpenCL] Refine pipe builtin support

Anastasia Stulova via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 5 09:44:06 PST 2016


Anastasia added a comment.

I think some comments from Richad's feedback are missing, specifically:

- Missing space after comma.
- Why are these conversions performed here rather than in Sema?
- You should attempt to implicitly convert to the desired type here, rather than demanding the right type, to match the normal call semantics. Likewise elsewhere in this patch.

Do you plan to commit them in a separate patch?

Also please add Richard to reviewers list here for the final Ok.

Thanks!


================
Comment at: lib/Sema/SemaChecking.cpp:294
@@ +293,3 @@
+             diag::err_opencl_builtin_pipe_invalid_access_modifier)
+          << "read_only" << Arg0->getSourceRange();
+      return true;
----------------
Could we use getName() instead?

We could then also move this statement after the switch and just set an error flag here.


http://reviews.llvm.org/D16876





More information about the cfe-commits mailing list