[PATCH] D34918: [libc++] Avoid atomic built-ins for NO_THREADS build

[Jonathan Roelofs via llvm-commits]

On 7/5/17 2:42 PM, Asiri Rathnayake wrote:
>
>
> On 5 Jul 2017 7:34 pm, "Weiming Zhao via Phabricator via llvm-commits" 
> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>
>     weimingz added a comment.
>
>     In https://reviews.llvm.org/D34918#799141
>     <https://reviews.llvm.org/D34918#799141>, @EricWF wrote:
>
>     > > On architectures without atomic instructions, the atomic
>     built-ins cannot be lowered. If _LIBCPP_HAS_NO_THREADS is enabled,
>     we should just use regular code.
>     >
>     > Does "cannot be lowered" mean using them causes a compile error?
>     If so I'm curious as to why you're the first one to run into this
>     issue. If no compile error is caused then could you re-explain the
>     rational for this change.
>     >
>     > In https://reviews.llvm.org/D34918#798422
>     <https://reviews.llvm.org/D34918#798422>, @weimingz wrote:
>     >
>     > > In https://reviews.llvm.org/D34918#797661
>     <https://reviews.llvm.org/D34918#797661>, @joerg wrote:
>     > >
>     > > > Lock-free atomic operations are also signal safe. Your code
>     is not. While I don't know whether all this functions are not
>     required to be signal safe, the general assertion is certainly
>     questionable.
>     > >
>     > >
>     > > No, they are no signal safe.  Per [1], "call to any library
>     function, except the following signal-safe functions (note, in
>     particular, dynamic allocation is not signal-safe):".
>     > >
>     > > Locale.cpp should be fine with the change.
>     > >  {set,get}_new_handler, {set,get}_unexpected_handler are not
>     in the list.
>     > >  I'm not very sure about "__libcpp_refstring". Seems it is
>     only used by stdexcept. Although throw expression is explicitly
>     stated as not signal safe, the ref counting of stdexcept might be.
>     We can leave it unchanged.
>     >
>     >
>     > It would be really nice if we didn't cause the user unnecessary
>     pain. Just because these functions aren't *required* to be signal
>     safe doesn't necessarily mean they shouldn't be.
>     >
>     > If this patch is indeed needed I would like to see it done
>     similar to how `__libcpp_refcount_foo`
>     <https://github.com/llvm-mirror/libcxx/blob/master/include/memory#L3370
>     <https://github.com/llvm-mirror/libcxx/blob/master/include/memory#L3370>>
>     or the `src/include/atomic_support.h`
>     <https://github.com/llvm-mirror/libcxx/blob/master/src/include/atomic_support.h
>     <https://github.com/llvm-mirror/libcxx/blob/master/src/include/atomic_support.h>>
>     logic is implemented, where instead of having `#ifdef` branches at
>     each call site the logic has been abstracted away into a function.
>     >
>     > That being said I would like to better understand the rational
>     for this patch before proceeding.
>
>
>     When the builtin cannot be lowered, compiler just emits function
>     call, which eventually becomes a linker error as no libs implement
>     those functions (libgcc. libclang-rt, libc)
>
>
> I thought this is where compiler-rt comes in? You should be able to 
> build that for cortex-m0.
>
> I haven't seen this on bare-metal though, I think it works for Linux 
> platforms. Not 100% sure. Adding Renato for comments.

Yeah, it can be implemented via a syscall on linux, but on baremetal you 
have to disable/re-enable interrupts in order to have something that 
behaves like compare & swap.


Jon
>
> Cheers,
>
> / Asiri
>
>
>
>     Repository:
>       rL LLVM
>
>     https://reviews.llvm.org/D34918 <https://reviews.llvm.org/D34918>
>
>
>
>     _______________________________________________
>     llvm-commits mailing list
>     llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>     http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>     <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>
>

-- 
Jon Roelofs
jonathan at codesourcery.com
CodeSourcery / Mentor Embedded / Siemens

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170705/dc25ff28/attachment.html>