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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 3 inline comments as done.
yaxunl added a comment.

Ping. Any other comments? Thanks.



================
Comment at: lib/AST/Expr.cpp:4000-4004
+  if (auto AT = T->getAs<AtomicType>()) {
+    return AT->getValueType();
+  } else {
+    return T;
+  }
----------------
bader wrote:
> No need in else branch after return:
> ```
> if (...) {
>   return AT->getValueType();
> }
> 
> return T;
> ```
> 
> http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
will fix when committing.


================
Comment at: test/SemaOpenCL/atomic-ops.cl:1
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -verify -fsyntax-only -triple=spir64
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -verify -fsyntax-only -triple=amdgcn-amdhsa-amd-opencl
----------------
bader wrote:
> It's a pity, we have to parse the whole opencl-c.h file to get two enums and one typedef...
Since there is change in opencl-c.h, using the header can test that.


https://reviews.llvm.org/D28691