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
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) {
> +
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
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 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
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
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
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
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),
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
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
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
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
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
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
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
> 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
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
22 matches
Mail list logo