Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-22 Thread JF Bastien via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-21 Thread Richard Smith via cfe-commits
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) { > +

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-21 Thread JF Bastien via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-21 Thread Richard Smith via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-21 Thread James Y Knight via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-20 Thread Joerg Sonnenberger via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-19 Thread Craig, Ben via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-19 Thread JF Bastien via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-19 Thread Joerg Sonnenberger via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-19 Thread Ben Craig via cfe-commits
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),

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-19 Thread Ben Craig via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-19 Thread JF Bastien via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-19 Thread Craig, Ben via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-19 Thread James Y Knight via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-19 Thread JF Bastien via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-19 Thread JF Bastien via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-19 Thread Ben Craig via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-18 Thread James Y Knight via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-18 Thread JF Bastien via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-18 Thread James Y Knight via cfe-commits
> 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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-07 Thread JF Bastien via cfe-commits
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

Re: [PATCH] D17950: Implement is_always_lock_free

2016-03-07 Thread JF Bastien via cfe-commits
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