This is an archive of the discontinued LLVM Phabricator instance.

enable <atomic> header on systems without thread-support
AbandonedPublic

Authored by ldionne on Jul 26 2019, 1:46 PM.

Details

Reviewers
burnpanck
Group Reviewers
Restricted Project
Summary

This patch enables the <atomic> header on systems without thread support, such as for example ARMv7em baremetal targets.
It consists essentially of three changes:

  1. Do not blindly refuse <atomic> when _LIBCPP_HAS_NO_THREADS is defined. I removed corresponding #error at the top of <atomic>. Furthermore, I changed the logic in <__config> where _LIBCPP_HAS_NO_ATOMIC_HEADER is determined to ignore _LIBCPP_HAS_NO_THREADS.
  2. In <__config>, define _LIBCPP_ATOMIC_ONLY_USE_BUILTINS when _LIBCPP_HAS_NO_THREADS is defined (instead of just when _LIBCPP_FREESTANDING is defined), since there is no other way to implement <atomic> without thread support. Arguably, when building without thread support, one is effectively doing a freestanding compilation, so one could require the user to supply “-ffreestanding”. However, it seems that this is not usually done on the systems I work with. Furthermore, it is conceivable that one wants to build libc++ for a hosted platform that does not supports threads but still would want to have support for atomics.
  3. In memory.cpp, change #ifdef _LIBCPP_HAS_NO_ATOMIC_HEADER to #ifdef _LIBCPP_HAS_NO_THREADS. Since this ifdef protects against the use of threading primitives, the latter seems more correct, while the former leads to compilation errors with the new logic for _LIBCPP_HAS_NO_ATOMIC_HEADER. It seems to indicate that the implementation of <memory> uses threading primitives instead of atomics. Therefore, in standalone builds, where concurrency comes in the form of interrupts rather than threads, one has to be careful with the use of <memory>. If at some point libc++ on baremetal becomes officially supported, that issue should probably be documented somewhere.

Of course, so far my patch fails to provide tests for the issue, which I’m not sure how to proceed with. I have never run any of the LLVM tests myself, as this seems to be extra challenging for cross-builds for baremetal. I did manually test the built libc++ in a simple (“blinky”) project that uses hardware timer interrupts to trigger state changes, relying on <atomic> for synchronisation. It appears to work, contrary to the non-atomic version of it.

Diff Detail

Repository
rCXX libc++

Event Timeline

burnpanck created this revision.Jul 26 2019, 1:46 PM
jfb added a subscriber: __simt__.Jul 26 2019, 1:56 PM

I think we want people to use -ffreestanding more for things like this, so the part where this is circumvented isn't clearly the right choice. The rest seems fine, and is exactly the reason why I created _LIBCPP_ATOMIC_ONLY_USE_BUILTINS.

My reasoning is the following: If _LIBCPP_ATOMIC_ONLY_USE_BUILTINS were only implied by _LIBCPP_FREESTANDING but not by _LIBCPP_HAS_NO_THREADS, the following paradoxical situation would appear for platforms without threads:

  • libc++ *with* -ffreestanding would be a conforming freestanding implementation, but not a conforming hosted implementation (no threads)
  • libc++ *without* -ffreestanding would still not be a conforming hosted implementation, but now it wouldn't even be a conforming freestanding implementation (because atomics are now missing).

Thus, the flag -ffreestanding, whose purpose is to *lower* the conformance level actually leads to a *higher* conformance level than without! The standard requires atomics in all conforming cases, whereas it would allow to disable threads under -ffreestanding (but it doesn't require it).

To me, -ffreestanding currently is ill-specified and has no good use. As far as I understand, the requirements for freestanding implementations are strictly smaller than for hosted implementations, but allowing implementation-defined extension. A conforming freestanding implementation is allowed to be a conforming hosted implementation, but it is not required to be. Thus, it is not clear what -ffreestanding even should do. It could be understood that *lack of -ffreestanding* requests a fully conforming hosted implementation; in that case, libc++ should choke on any platform that does not support threads unless -ffreestanding is given.

This change makes sense to me as-is. I agree with @__simt__ that we want people to use -ffreestanding in the future, however I'm also with @burnpanck on the fact that Freestanding is currently ill-defined (both in the Standard and in existing practice -- nobody seems to have agreement on what exactly is in there).

Since we currently allow disabling the use of threads without using -ffreestanding, I think it makes sense to _not_ require -ffreestanding to enable the builtin-only atomics. Once we figure out what exactly is in freestanding, we can change that and guard all those features by #if defined(_LIBCPP_FREESTANDING) && <existing feature> (or something along those lines), with the goal of requiring the use of -ffreestanding to enable those features. @__simt__ Would you still rather requiring the use of -ffreestanding to enable builtin-only atomics?

Friendly ping. Is there anything I can help with? In the meantime, I have been able to get some code to run, utilising this on a Cortex M4 in a bare-metal environment. To get it to work I have to patch LLVM 9 clang and libc++ in a few other places anyway, so there is no immediate need to get this upstreamed from my side - but I'd definitely would love to see LLVM as a viable alternative to GCC for such tasks at some point.

ldionne requested changes to this revision.Mar 3 2020, 12:50 PM

@burnpanck I'm willing to look at this again if you're willing to rebase on top of master, FWIW. There might be changes required since we landed the C++20 Synchronization Library.

This revision now requires changes to proceed.Mar 3 2020, 12:50 PM
ldionne commandeered this revision.Sep 5 2023, 2:05 PM
ldionne edited reviewers, added: burnpanck; removed: ldionne.

[Github PRs transition cleanup]

Abandoning since this needs a heavy rebase.

Herald added a project: Restricted Project. · View Herald TranscriptSep 5 2023, 2:05 PM
ldionne abandoned this revision.Sep 5 2023, 2:05 PM