[libcxx-commits] [PATCH] D114109: [libc++] Enable <atomic> when threads are disabled
Eli Friedman via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Jan 26 17:25:21 PST 2022
efriedma added inline comments.
================
Comment at: libcxx/include/atomic:2697
typedef atomic<__libcpp_signed_lock_free> atomic_signed_lock_free;
typedef atomic<__libcpp_unsigned_lock_free> atomic_unsigned_lock_free;
----------------
androm3da wrote:
> efriedma wrote:
> > androm3da wrote:
> > > After this commit landed downstream we encountered a failure building for an arm baremetal target that defines the __CLANG_ATOMIC_*_LOCK_FREE vals to '1' ("sometimes lock free").
> > >
> > > This results in the `atomic_{,un}signed_lock_free` typedefs here referencing an undefined `_libcpp_{,un}signed_lock_free`.
> > >
> > > I suspect that before this commit `_LIBCPP_HAS_NO_ATOMIC_HEADER` was false.
> > >
> > > I think maybe we could consider an `#error:` in the else case above that might make it clearer?
> > >
> > > I could use some advice as to how to handle this -- does it make sense to have the atomic header in the case where these types are only designated 'sometimes lock free'? If not I think the fix would be to change the conditional for _LIBCPP_HAS_NO_ATOMIC_HEADER to consider these? Otherwise, if it does make sense to have an atomic header, can I just suppress these typedefs? That does seem to address the build failure so far.
> > The standard suggests we should just suppress the typedefs, not the entire header.
> >
> > From the current C++ standard, [compliance]p3:
> >
> > > The supplied version of the header <atomic> shall meet the same requirements as for a hosted implementation except that support for always lock-free integral atomic types ([atomics.lockfree]) is implementation-defined, and whether or not the type aliases atomic_signed_lock_free and atomic_unsigned_lock_free are defined ([atomics.alias]) is implementation-defined. The other headers listed in this table shall meet the same requirements as for a hosted implementation.
> > The standard suggests we should just suppress the typedefs, not the entire header.
> >
> > From the current C++ standard, [compliance]p3:
> >
> > > The supplied version of the header <atomic> shall meet the same requirements as for a hosted implementation except that support for always lock-free integral atomic types ([atomics.lockfree]) is implementation-defined, and whether or not the type aliases atomic_signed_lock_free and atomic_unsigned_lock_free are defined ([atomics.alias]) is implementation-defined. The other headers listed in this table shall meet the same requirements as for a hosted implementation.
>
> Ok, thanks, that clears it up.
>
> @efriedma @ldionne should I post a change like this?
>
>
> ```
> --- a/libcxx/include/atomic
> +++ b/libcxx/include/atomic
> @@ -2691,10 +2691,13 @@ typedef conditional<_LIBCPP_CONTENTION_LOCK_FREE, __cxx_contention_t, char>::typ
> typedef conditional<_LIBCPP_CONTENTION_LOCK_FREE, __cxx_contention_t, unsigned char>::type __libcpp_unsigned_lock_free;
> #else
> // No signed/unsigned lock-free types
> +#define _LIBCPP_NO_LOCK_FREE_TYPES
> #endif
>
> +#if !defined(_LIBCPP_NO_LOCK_FREE_TYPES)
> typedef atomic<__libcpp_signed_lock_free> atomic_signed_lock_free;
> typedef atomic<__libcpp_unsigned_lock_free> atomic_unsigned_lock_free;
> +#endif
>
> #define ATOMIC_FLAG_INIT {false}
> #define ATOMIC_VAR_INIT(__v) {__v}
>
> ```
Looks reasonable to me.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114109/new/
https://reviews.llvm.org/D114109
More information about the libcxx-commits
mailing list