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

2017-08-04 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. yaxunl marked 2 inline comments as done. Closed by commit rL310082: Add OpenCL 2.0 atomic builtin functions as Clang builtin (authored by yaxunl). Changed prior to commit:

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

2017-08-04 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Still LGTM. https://reviews.llvm.org/D28691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

2017-08-04 Thread 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()) { +return AT->getValueType(); + } else { +return T; + } bader wrote: > No need in

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

2017-08-04 Thread Alexey Bader via Phabricator via cfe-commits
bader accepted this revision. bader added inline comments. Comment at: lib/AST/Expr.cpp:4000-4004 + if (auto AT = T->getAs()) { +return AT->getValueType(); + } else { +return T; + } No need in else branch after return: ``` if (...) { return

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

2017-08-04 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked 10 inline comments as done. yaxunl added inline comments. Comment at: include/clang/Basic/SyncScope.h:21 +/// \brief Defines the synch scope values used by the atomic builtins and +/// expressions +enum class SyncScope { rjmccall wrote: > If the

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

2017-08-03 Thread Tony Tye via Phabricator via cfe-commits
t-tye accepted this revision. t-tye added a comment. LGTM https://reviews.llvm.org/D28691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-08-03 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 109612. yaxunl marked an inline comment as done. yaxunl added a comment. Added documentation about __OPENCL_MEMORY_SCOPE_* by Tony's comments. https://reviews.llvm.org/D28691 Files: docs/LanguageExtensions.rst include/clang/AST/Expr.h

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

2017-08-03 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked an inline comment as done. yaxunl added inline comments. Comment at: docs/LanguageExtensions.rst:1935 +builtin function, and are named with a ``__opencl_`` prefix.) Low-level ARM exclusive memory builtins t-tye wrote: > Should it also say: > >

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

2017-08-03 Thread Tony Tye via Phabricator via cfe-commits
t-tye added inline comments. Comment at: docs/LanguageExtensions.rst:1935 +builtin function, and are named with a ``__opencl_`` prefix.) Low-level ARM exclusive memory builtins Should it also say: ``` The macros ``__OPENCL_MEMORY_SCOPE_WORK_ITEM``,

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

2017-08-03 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 109591. yaxunl added a comment. Add comments to SyncScope.h https://reviews.llvm.org/D28691 Files: docs/LanguageExtensions.rst include/clang/AST/Expr.h include/clang/Basic/Builtins.def include/clang/Basic/DiagnosticSemaKinds.td

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

2017-08-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. LGTM. I'm fine with the plan to handle potentially non-constant scopes in a follow-up patch. Comment at: include/clang/Basic/SyncScope.h:21 +/// \brief Defines the synch scope values used by the atomic builtins and +/// expressions +enum class

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

2017-08-03 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 109556. yaxunl added a comment. Add assert to make sure pre-defined macros __OPENCL_MEMORY_SCOP_* to be consistent with SyncScope enum. https://reviews.llvm.org/D28691 Files: docs/LanguageExtensions.rst include/clang/AST/Expr.h

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

2017-08-03 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 109542. yaxunl marked 5 inline comments as done. yaxunl added a comment. Herald added subscribers: aheejin, dschuff, jfb. Revised by Tony's and John's comments. https://reviews.llvm.org/D28691 Files: docs/LanguageExtensions.rst include/clang/AST/Expr.h

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

2017-08-03 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked 6 inline comments as done. yaxunl added inline comments. Comment at: include/clang/Basic/Builtins.def:717 +ATOMIC_BUILTIN(__opencl_atomic_fetch_max, "v.", "t") + #undef ATOMIC_BUILTIN t-tye wrote: > Will the OpenCL 2.0 memory fences also be

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

2017-08-02 Thread 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 =

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

2017-08-02 Thread Tony Tye via Phabricator via cfe-commits
t-tye added inline comments. Comment at: include/clang/Basic/SyncScope.h:23 +enum class SyncScope { + OpenCLWorkItem = 0, + OpenCLWorkGroup = 1, The OpenCL workitem scope is only used for image fences and does not apply to atomic operations so not sure that

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

2017-08-02 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 109406. yaxunl marked 29 inline comments as done. yaxunl added a comment. Revised by reviewers' comments. https://reviews.llvm.org/D28691 Files: docs/LanguageExtensions.rst include/clang/AST/Expr.h include/clang/Basic/Builtins.def

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

2017-07-28 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: include/clang/Basic/SyncScope.h:1 +//===--- SyncScope.h - atomic synchronization scopes *- C++ -*-===// +// Capitalization. Comment at: include/clang/Basic/SyncScope.h:20 + +namespace

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

2017-07-28 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D28691#823810, @Anastasia wrote: > In https://reviews.llvm.org/D28691#820684, @rjmccall wrote: > > > In https://reviews.llvm.org/D28691#820641, @b-sumner wrote: > > > > > In https://reviews.llvm.org/D28691#820595, @rjmccall wrote: > > > > > >

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

2017-07-28 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment. In https://reviews.llvm.org/D28691#820684, @rjmccall wrote: > In https://reviews.llvm.org/D28691#820641, @b-sumner wrote: > > > In https://reviews.llvm.org/D28691#820595, @rjmccall wrote: > > > > > In https://reviews.llvm.org/D28691#820541, @b-sumner wrote: > > > > > >

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

2017-07-25 Thread Tony Tye via Phabricator via cfe-commits
t-tye added inline comments. Comment at: include/clang/Basic/Builtins.def:717 +ATOMIC_BUILTIN(__opencl_atomic_fetch_max, "v.", "t") + #undef ATOMIC_BUILTIN Will the OpenCL 2.0 memory fences also be supported which also have a memory order and memory scope?

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

2017-07-25 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D28691#820641, @b-sumner wrote: > In https://reviews.llvm.org/D28691#820595, @rjmccall wrote: > > > In https://reviews.llvm.org/D28691#820541, @b-sumner wrote: > > > > > There are other languages for heterogeneous compute that have scopes, >

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

2017-07-25 Thread Brian Sumner via Phabricator via cfe-commits
b-sumner added a comment. In https://reviews.llvm.org/D28691#820595, @rjmccall wrote: > In https://reviews.llvm.org/D28691#820541, @b-sumner wrote: > > > There are other languages for heterogeneous compute that have scopes, > > although not exposed quite as explicitly as OpenCL. For example

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

2017-07-25 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D28691#820541, @b-sumner wrote: > There are other languages for heterogeneous compute that have scopes, > although not exposed quite as explicitly as OpenCL. For example AMD's "HC" > language. And any language making use of clang and

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

2017-07-25 Thread Brian Sumner via Phabricator via cfe-commits
b-sumner added a comment. In https://reviews.llvm.org/D28691#820526, @rjmccall wrote: > In https://reviews.llvm.org/D28691#820489, @yaxunl wrote: > > > In https://reviews.llvm.org/D28691#820466, @b-sumner wrote: > > > > > Can we drop the "opencl" part of the name and use something like > > >

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

2017-07-25 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D28691#820489, @yaxunl wrote: > In https://reviews.llvm.org/D28691#820466, @b-sumner wrote: > > > Can we drop the "opencl" part of the name and use something like > > __scoped_atomic_*? Also, it may not make sense to support non-constant

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

2017-07-25 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment. In https://reviews.llvm.org/D28691#820466, @b-sumner wrote: > Can we drop the "opencl" part of the name and use something like > __scoped_atomic_*? Also, it may not make sense to support non-constant > scope here since we can't predict what other scopes may be added

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

2017-07-25 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 108127. yaxunl added a comment. Add min/max and missing file. https://reviews.llvm.org/D28691 Files: docs/LanguageExtensions.rst include/clang/AST/Expr.h include/clang/Basic/Builtins.def include/clang/Basic/DiagnosticSemaKinds.td

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

2017-07-25 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked an inline comment as done. yaxunl added a comment. In https://reviews.llvm.org/D28691#820375, @kzhuravl wrote: > Seems like SyncScope.h file is missing? Right. I will add it. https://reviews.llvm.org/D28691 ___ cfe-commits mailing

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

2017-07-25 Thread Konstantin Zhuravlyov via Phabricator via cfe-commits
kzhuravl added a comment. Seems like SyncScope.h file is missing? https://reviews.llvm.org/D28691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-07-25 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked 2 inline comments as done. yaxunl added inline comments. Comment at: include/clang/Basic/Builtins.def:713 +ATOMIC_BUILTIN(__opencl_atomic_fetch_or, "v.", "t") +ATOMIC_BUILTIN(__opencl_atomic_fetch_xor, "v.", "t") + b-sumner wrote: > yaxunl wrote: >

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

2017-07-25 Thread Brian Sumner via Phabricator via cfe-commits
b-sumner added inline comments. Comment at: include/clang/Basic/Builtins.def:713 +ATOMIC_BUILTIN(__opencl_atomic_fetch_or, "v.", "t") +ATOMIC_BUILTIN(__opencl_atomic_fetch_xor, "v.", "t") + yaxunl wrote: > Anastasia wrote: > > What about min/max? I believe they

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

2017-07-25 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 108073. yaxunl marked 16 inline comments as done. yaxunl edited the summary of this revision. yaxunl added a reviewer: kzhuravl. yaxunl added a comment. Herald added a subscriber: nhaehnle. Rebased to ToT and revised by Anastasia's comments.

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

2017-07-25 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked 16 inline comments as done. yaxunl added inline comments. Comment at: include/clang/Basic/Builtins.def:713 +ATOMIC_BUILTIN(__opencl_atomic_fetch_or, "v.", "t") +ATOMIC_BUILTIN(__opencl_atomic_fetch_xor, "v.", "t") + Anastasia wrote: > What about

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

2017-06-20 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments. Comment at: include/clang/Basic/Builtins.def:713 +ATOMIC_BUILTIN(__opencl_atomic_fetch_or, "v.", "t") +ATOMIC_BUILTIN(__opencl_atomic_fetch_xor, "v.", "t") + What about min/max? I believe they will need to have the scope too.

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

2017-06-13 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 102374. yaxunl marked 11 inline comments as done. yaxunl edited the summary of this revision. yaxunl added a comment. Revised by John's comments. https://reviews.llvm.org/D28691 Files: docs/LanguageExtensions.rst include/clang/AST/Expr.h

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

2017-06-09 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Looks like you haven't introduced proper constants in the header for scopes. Comment at: docs/LanguageExtensions.rst:1855 +atomic builtins are in ``explicit`` form of the corresponding OpenCL 2.0 +builtin function, and are named with a ``__opencl__``

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

2017-06-08 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 101955. yaxunl retitled this revision from "Support synchronisation scope in Clang atomic builtin functions" to "Add OpenCL 2.0 atomic builtin functions as Clang builtin". yaxunl edited the summary of this revision. yaxunl added a comment. Add