[PATCH] D28691: Add OpenCL 2.0 atomic builtin functions as Clang builtin

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

The patch generally looks good, but if you need to handle non-constant scopes, you should submit a new patch to address that.



================
Comment at: lib/CodeGen/CGAtomic.cpp:896
+        return V;
+      auto DestAS = getContext().getTargetAddressSpace(LangAS::opencl_generic);
+      auto T = V->getType();
----------------
You can sink this line and the next.


================
Comment at: lib/Sema/SemaChecking.cpp:3103
+        Diag(Scope->getLocStart(),
+             diag::err_atomic_op_has_non_constant_synch_scope)
+            << Scope->getSourceRange();
----------------
t-tye wrote:
> IIRC OpenCL allows the scope to be a runtime value. So will doing this will likely cause failures in conformance?
Ah, if that's true, you'll need to emit a switch in IRGen, the same way we handle non-constant memory orders.


https://reviews.llvm.org/D28691