jfb abandoned this revision.
Comment at: lib/Frontend/InitPreprocessor.cpp:465
@@ +464,3 @@
+ if (LangOpts.CPlusPlus1z) {
+Builder.defineMacro("__cpp_lib_atomic_is_always_lock_free", "201603");
+ }
jfb wrote:
> rsmith wrote:
> > This should be defined by the
On Mon, Mar 21, 2016 at 4:46 PM, JF Bastien via cfe-commits
wrote:
> jfb added inline comments.
>
>
> Comment at: lib/Frontend/InitPreprocessor.cpp:465
> @@ +464,3 @@
> + if (LangOpts.CPlusPlus1z) {
> +Builder.defineMacro("__cpp_lib_atomic_is_always_lock_free", "201603");
> +
jfb added inline comments.
Comment at: lib/Frontend/InitPreprocessor.cpp:465
@@ +464,3 @@
+ if (LangOpts.CPlusPlus1z) {
+Builder.defineMacro("__cpp_lib_atomic_is_always_lock_free", "201603");
+ }
rsmith wrote:
> This should be defined by the relevant library
rsmith added inline comments.
Comment at: lib/Frontend/InitPreprocessor.cpp:465
@@ +464,3 @@
+ if (LangOpts.CPlusPlus1z) {
+Builder.defineMacro("__cpp_lib_atomic_is_always_lock_free", "201603");
+ }
This should be defined by the relevant library header, not
jyknight added a comment.
> Changed to what you suggested. Much nicer. I don't remember why I thought it
> was a bad idea.
Thanks, great! I don't have any opinion on what remains in this patch; someone
else should review now.
http://reviews.llvm.org/D17950
On 3/18/2016 11:54 AM, JF Bastien wrote:
Some architectures support byte granularity memory protection.
Accessing unsolicited bytes can cause a trap on those architectures.
I think it makes sense for atomic and Atomic to get
turned into the 4 byte __atomic intrinsics. Those 4
On Fri, Mar 18, 2016 at 12:11:22PM -0500, Craig, Ben via cfe-commits wrote:
> A lot of it is a frontend decision. What goes in the libcall feels an awful
> lot like the 386 vs 486 example that I hear a lot. If I want one binary
> that can run on both a 386 (very limited atomic support) and a 486
jyknight added inline comments.
Comment at: lib/Frontend/InitPreprocessor.cpp:305
@@ +304,3 @@
+if (TypeWidth == TypeAlign && (TypeWidth & (TypeWidth - 1)) == 0 &&
+TypeWidth <= InlineWidth)
+ return Always;
jfb wrote:
> bcraig wrote:
> > On some
On 3/18/2016 1:50 AM, Joerg Sonnenberger via cfe-commits wrote:
On Thu, Mar 17, 2016 at 05:56:17PM +, JF Bastien via cfe-commits wrote:
C++ atomics are explicitly designed to avoid problems with touching
adjacent bytes: if `atomic` where `sizeof(T) == 1` requires a 4-byte
`cmpxchg` then it's
jfb added a comment.
In http://reviews.llvm.org/D17950#376965, @jfb wrote:
> In http://reviews.llvm.org/D17950#376349, @jyknight wrote:
>
> > This conflicts with http://reviews.llvm.org/D17933. Most of this change
> > also seems unnecessary.
> >
> > - I think the `is_always_lock_free` function s
On Fri, Mar 18, 2016 at 7:30 AM, Craig, Ben via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> On 3/18/2016 1:50 AM, Joerg Sonnenberger via cfe-commits wrote:
>
>> On Thu, Mar 17, 2016 at 05:56:17PM +, JF Bastien via cfe-commits
>> wrote:
>>
>>> C++ atomics are explicitly designed to avoid
On Thu, Mar 17, 2016 at 05:56:17PM +, JF Bastien via cfe-commits wrote:
> C++ atomics are explicitly designed to avoid problems with touching
> adjacent bytes: if `atomic` where `sizeof(T) == 1` requires a 4-byte
> `cmpxchg` then it's up to the frontend to make sure `sizeof> >= 4`
> (or somethi
bcraig added a comment.
> C++ atomics are explicitly designed to avoid problems with touching adjacent
> bytes: if `atomic` where `sizeof(T) == 1` requires a 4-byte `cmpxchg` then
> it's up to the frontend to make sure `sizeof> >= 4` (or something
> equivalent such as making it non-lock-free).
bcraig added a subscriber: bcraig.
Comment at: lib/Frontend/InitPreprocessor.cpp:305
@@ +304,3 @@
+if (TypeWidth == TypeAlign && (TypeWidth & (TypeWidth - 1)) == 0 &&
+TypeWidth <= InlineWidth)
+ return Always;
On some targets (like Hexagon), 4-by
bcraig added inline comments.
Comment at: lib/Frontend/InitPreprocessor.cpp:305
@@ +304,3 @@
+if (TypeWidth == TypeAlign && (TypeWidth & (TypeWidth - 1)) == 0 &&
+TypeWidth <= InlineWidth)
+ return Always;
jfb wrote:
> bcraig wrote:
> > jyknight w
jfb added a comment.
In http://reviews.llvm.org/D17950#377386, @cfe-commits wrote:
> I know that MIPS does that, and an out-of-tree implementation of hexagon
> implements 1-byte cmpxchg in terms of the 4-byte version. The emulation
> code isn't particularly small, and it seems reasonable to m
I know that MIPS does that, and an out-of-tree implementation of hexagon
implements 1-byte cmpxchg in terms of the 4-byte version. The emulation
code isn't particularly small, and it seems reasonable to make it a
libcall. The emulation code seems sketchy from a correctness
perspective, as you
jyknight added a subscriber: jyknight.
jyknight added a comment.
This conflicts with http://reviews.llvm.org/D17933. Most of this change also
seems unnecessary.
- I think the `is_always_lock_free` function should be defined based on the
existing `__atomic_always_lock_free` builtin, not on defin
jfb updated this revision to Diff 51048.
jfb added a comment.
- Use __atomic_always_lock_free instead
http://reviews.llvm.org/D17950
Files:
lib/Frontend/InitPreprocessor.cpp
test/Lexer/cxx-features.cpp
Index: test/Lexer/cxx-features.cpp
=
jfb added inline comments.
Comment at: lib/Frontend/InitPreprocessor.cpp:305
@@ +304,3 @@
+if (TypeWidth == TypeAlign && (TypeWidth & (TypeWidth - 1)) == 0 &&
+TypeWidth <= InlineWidth)
+ return Always;
bcraig wrote:
> jyknight wrote:
> > jfb wrot
bcraig added inline comments.
Comment at: lib/Frontend/InitPreprocessor.cpp:305
@@ +304,3 @@
+if (TypeWidth == TypeAlign && (TypeWidth & (TypeWidth - 1)) == 0 &&
+TypeWidth <= InlineWidth)
+ return Always;
jyknight wrote:
> jfb wrote:
> > bcraig w
On Thu, Mar 17, 2016 at 12:55 PM, Craig, Ben
wrote:
> I know that MIPS does that, and an out-of-tree implementation of hexagon
> implements 1-byte cmpxchg in terms of the 4-byte version. The emulation
> code isn't particularly small, and it seems reasonable to make it a libcall.
That's fine. Ei
jfb added a comment.
In http://reviews.llvm.org/D17950#376349, @jyknight wrote:
> This conflicts with http://reviews.llvm.org/D17933. Most of this change also
> seems unnecessary.
>
> - I think the `is_always_lock_free` function should be defined based on the
> existing `__atomic_always_lock_fr
> A 4 byte cmpxchg could be lock free, while a 1 byte cmpxchg may not be
lock free.
That's not possible: if you have a 4-byte cmpxchg instruction, you can use
it to implement a 1-byte cmpxchg, too. Many targets do this already. (It
would be better if that was available generically so that code did
jfb added inline comments.
Comment at: lib/Frontend/InitPreprocessor.cpp:480
@@ +479,3 @@
+ if (LangOpts.CPlusPlus1z) {
+Builder.defineMacro("__cpp_lib_atomic_is_always_lock_free", "201603");
+ }
jfb wrote:
> How do you pick these numbers before the standard
jfb added inline comments.
Comment at: lib/Frontend/InitPreprocessor.cpp:480
@@ +479,3 @@
+ if (LangOpts.CPlusPlus1z) {
+Builder.defineMacro("__cpp_lib_atomic_is_always_lock_free", "201603");
+ }
How do you pick these numbers before the standard is actually
jfb added a comment.
I'll check with GCC folks whether they want to share macros the same way we're
already sharing `__GCC_ATOMIC_*_LOCK_FREE`. The new macros I propose are
`__LLVM__ATOMIC_*_BYTES_LOCK_FREE` and `__LLVM_LOCK_FREE_IS_SIZE_BASED`.
Let's do a first sanity-check round of code revie
jfb created this revision.
jfb added reviewers: mclow.lists, rsmith.
jfb added a subscriber: cfe-commits.
Herald added subscribers: dschuff, jfb.
This was voted into C++17 at last week's Jacksonville meeting. The final
P0152R1 paper will be in the upcoming post-Jacksonville mailing, and is also
28 matches
Mail list logo
