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

[Asiri Rathnayake via llvm-commits]

On 5 Jul 2017 7:34 pm, "Weiming Zhao via Phabricator via llvm-commits" <
llvm-commits at lists.llvm.org> wrote:

weimingz added a comment.

In 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, @weimingz wrote:
>
> > In 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> or the `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.

Cheers,

/ Asiri



Repository:
  rL LLVM

https://reviews.llvm.org/D34918



_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170705/149b9cf9/attachment.html>