[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

2018-05-15 Thread Al Grant via Phabricator via cfe-commits
algrant added a comment. There is a (stalled) proposal to add atomic integer max/min to C++: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0493r0.pdf . The proposal has memory semantics similar to these builtins, i.e. unconditional RMW. There is no limitation to 32-bit types though,

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

2018-05-13 Thread Elena Demikhovsky via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL332193: Added atomic_fetch_min, max, umin, umax intrinsics to clang. (authored by delena, committed by ). Changed prior to commit: https://reviews.llvm.org/D46386?vs=146462&id=146502#toc Repository:

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

2018-05-12 Thread Elena Demikhovsky via Phabricator via cfe-commits
delena added a comment. In https://reviews.llvm.org/D46386#1096833, @rjmccall wrote: > The actual semantic parts of the diff seem to have disappeared from the patch > posted to Phabricator, for what it's worth. It is not disappeared by itself, I removed it. I understood that you don't see any

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

2018-05-11 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. Thanks, that works for me. The actual semantic parts of the diff seem to have disappeared from the patch posted to Phabricator, for what it's worth. Repository: rC Clang https://revie

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

2018-05-11 Thread Elena Demikhovsky via Phabricator via cfe-commits
delena updated this revision to Diff 146462. delena added a comment. Added a line about *load-store* semantics of these two intrinsics. Removed the common description of memory modeling. Repository: rC Clang https://reviews.llvm.org/D46386 Files: LanguageExtensions.rst Index: LanguageExt

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

2018-05-10 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: docs/LanguageExtensions.rst:1998 +``__ATOMIC_CONSUME``, ``__ATOMIC_ACQUIRE``, ``__ATOMIC_RELEASE``, +``__ATOMIC_ACQ_REL``, or ``__ATOMIC_SEQ_CST`` following C++11 memory model semantics. + rjmccall wrote: > Thank you f

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

2018-05-10 Thread Elena Demikhovsky via Phabricator via cfe-commits
delena updated this revision to Diff 146080. delena added a comment. Given more clarification about memory model of atomic operations. Repository: rC Clang https://reviews.llvm.org/D46386 Files: docs/LanguageExtensions.rst include/clang/Basic/Builtins.def include/clang/Basic/Diagnostic

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

2018-05-08 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: docs/LanguageExtensions.rst:1998 +``__ATOMIC_CONSUME``, ``__ATOMIC_ACQUIRE``, ``__ATOMIC_RELEASE``, +``__ATOMIC_ACQ_REL``, or ``__ATOMIC_SEQ_CST`` following C++11 memory model semantics. + Thank you for adding this doc

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

2018-05-08 Thread Elena Demikhovsky via Phabricator via cfe-commits
delena updated this revision to Diff 145646. delena added a comment. Removed the unsigned version of atomics. Enhanced semantics check. Added more tests. Added documentation. Repository: rC Clang https://reviews.llvm.org/D46386 Files: docs/LanguageExtensions.rst include/clang/Basic/Built

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

2018-05-04 Thread Elena Demikhovsky via Phabricator via cfe-commits
delena added a comment. In https://reviews.llvm.org/D46386#1087533, @Anastasia wrote: > Is this some sort of a vendor extension then? OpenCL 1.2 atomic builtins > don't have ordering parameter. OpenCL 1.2 atomic builtins have relaxed semantics. Always, it is not parameter, it is defined behav

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

2018-05-04 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment. Is this some sort of a vendor extension then? OpenCL 1.2 atomic builtins don't have ordering parameter. Repository: rC Clang https://reviews.llvm.org/D46386 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

2018-05-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Given that these are overloaded builtins, why are there separate builtins for `min` and `umin`? If this is a Clang extension, it needs to be documented in our extensions documentation. The documentation should describe the exact ordering semantics, to the extent we w

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

2018-05-03 Thread Elena Demikhovsky via Phabricator via cfe-commits
delena added inline comments. Comment at: lib/Sema/SemaChecking.cpp:3098 + case AtomicExpr::AO__atomic_fetch_umax: +IsMinMax = true; +Form = Arithmetic; jfb wrote: > Should `__sync_fetch_and_min` and others also set `IsMinMax`? __sync_fetch_and_min is no

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

2018-05-03 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: lib/Sema/SemaChecking.cpp:3098 + case AtomicExpr::AO__atomic_fetch_umax: +IsMinMax = true; +Form = Arithmetic; Should `__sync_fetch_and_min` and others also set `IsMinMax`? Repository: rC Clang https://reviews.

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

2018-05-03 Thread Elena Demikhovsky via Phabricator via cfe-commits
delena created this revision. delena added reviewers: igorb, t.p.northover, ABataev, jfb, rjmccall. Herald added subscribers: cfe-commits, Anastasia. Added __atomic_fetch_min, max, umin, umax intrinsics to clang. These intrinsics work exactly as all other __atomic_fetch_* intrinsics and allow to