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

[Weiming Zhao via Phabricator via llvm-commits]

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)


Repository:
  rL LLVM

https://reviews.llvm.org/D34918