This is an archive of the discontinued LLVM Phabricator instance.

[runtimes] Use the new "runtimes" build by default and deprecate other builds
ClosedPublic

Authored by ldionne on Oct 7 2021, 2:20 PM.

Details

Reviewers
phosek
ldionne
Group Reviewers
Restricted Project
Restricted Project
Commits
rG79175f336c1f: [runtimes] Use the new "runtimes" build by default and deprecate other builds
Summary

This commit makes the new "runtimes" build (with <monorepo>/runtimes as
the root of the CMake invocation) the default way of building libc++.
The other supported way of building libc++ is the "bootstrapping" build,
where <monorepo>/llvm is used as the root of the CMake invocation.

All other ways of building libc++ are deprecated effective immediately.
There should be no use-case for building libc++ that isn't supported by
one of these two builds, and the two new builds work on all environments
and are lightweight. They will also make it possible to greatly simplify
the build infrastructure of the runtimes, which is currently way too
convoluted.

Diff Detail

Event Timeline

ldionne created this revision.Oct 7 2021, 2:20 PM
ldionne requested review of this revision.Oct 7 2021, 2:20 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2021, 2:20 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

@phosek @mstorsjo Can you please have a look at this. This should be consistent with what we agreed upon on Discord the other day. Once we're all on the same page and we've confirmed that the new "runtimes" build works on all platforms (CI will tell), I'll add a bunch of relevant folks to this review for awareness. I'll also send an email to llvm-dev and cfe-dev.

The plan is:

  1. Deprecate all builds other than the "default" build (rooted at <monorepo>/runtimes) and the "bootstrapping" build (rooted at <monorepo>/llvm) immediately.
  2. Ask folks to move to the new builds immediately, since they already can.
  3. Support the other builds until LLVM 14 is released through the CI I added (part of which was already in place).
  4. After LLVM 14 is released, we can remove support for those other builds in main and start simplifying the heck out of our build infrastructure, knowing that everybody is going through <monorepo>/runtimes as their entry point.
mstorsjo added inline comments.Oct 7 2021, 2:39 PM
libcxx/docs/BuildingLibcxx.rst
63

FWIW, I'm not sure I entirely agree with the sentence implying that this is the only correct way to build the runtimes when putting together a toolchain: This approach doesn't work entirely when bootstrapping a toolchain entirely from scratch (when e.g. the cross C runtime isn't built yet).

In these cases, one still has to build the compiler first, then compiler-rt builtins, then externally build e.g. the cross C runtime, then proceed to build libc++ and the other runtimes. So when one has to do external steps inbetween, the standalone build (with e.g. <monorepo>/runtimes) decoupled from building the compiler, is the only practical way right now. See e.g. https://github.com/ClangBuiltLinux/llvm-distributors-conf-2021/issues/13#issuecomment-930911835 for more confirmation that this indeed isn't quite supported by the bootstrapping build process yet.

libcxx/docs/ReleaseNotes.rst
64

Typo: cater

70

So these will be marked as deprecated in the 14 release, and be unsupported by the time 15 is released? That sounds tolerable to me.

91

This example is lacking the LLVM_ENABLE_RUNTIMES=libcxx;libcxxabi bit.

Also technically, it's still possible to build them separately in two invocations/build dirs by pointing them at runtimes and specifying one at a time. That's of course something that one would rather avoid unless really needed, but I could see the need if building in a config where one wants to set specific CMAKE_CXX_FLAGS differently for each of the runtime libraries. (I used to need to do this before, but I've managed to upstream fixes so this no longer is needed.)

libcxx/utils/ci/run-buildbot
446

I'd kinda prefer to split out these CI changes to a separate commit, with separate explanations/arguments for why there, as this change isn't directly tied to the rest of the deprecation (it still tests essentially the same set of configurations, just slightly different labels on it).

ldionne updated this revision to Diff 378274.Oct 8 2021, 9:31 AM
ldionne marked 4 inline comments as done.

Address review comments and fix the CI (hopefully).

Herald added a project: Restricted Project. · View Herald TranscriptOct 8 2021, 9:31 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: mgorny. · View Herald Transcript
ldionne added inline comments.Oct 8 2021, 9:31 AM
libcxx/docs/BuildingLibcxx.rst
63

I will change to this is usually the correct way. I want to suggest that people look no further if they don't need more than that. If they need something more complicated, then sure, they can use the "default" build in some way to achieve their goals.

libcxx/docs/ReleaseNotes.rst
70

Yes. In other words, they are marked as deprecated immediately (so that means they are deprecated when we release LLVM 14). And as soon as we branch LLVM 14 out, we are free to remove support for those builds on main. It's somewhat aggressive, but at the same time we have a clear and easy migration path for everybody, and supporting all the ways to build right now is pretty taxing.

91

This example is lacking the LLVM_ENABLE_RUNTIMES=libcxx;libcxxabi bit.

Fixed, thanks.

Also technically, it's still possible to build them separately in two invocations/build dirs by pointing them at runtimes and specifying one at a time. That's of course something that one would rather avoid unless really needed, but I could see the need if building in a config where one wants to set specific CMAKE_CXX_FLAGS differently for each of the runtime libraries. (I used to need to do this before, but I've managed to upstream fixes so this no longer is needed.)

Nope, that's deprecated too after this patch. That requires using the various hacks to tell libc++abi where libc++ is and stuff like that (LIBCXXABI_LIBCXX_PATH). That's exactly the thing we're trying to move away from. If you need to pass different flags to the projects, we can add a better way to do that. Allowing the runtimes to be built separately goes against exactly the push I'm making, which is to ensure that we can assume we have a unified CMake invocation for all the runtimes.

libcxx/utils/ci/run-buildbot
446

IMO it is tied precisely to the deprecation, since I moved all the other jobs to the new runtimes build. This effectively promotes the new runtimes build to a first class citizen that we run CI on by default, and at the same time adds a new job that tests the old way to build, just to make sure we don't break it yet. Did you mis-read the change and now you agree, or do you still disagree this belongs here?

mstorsjo added inline comments.Oct 8 2021, 9:41 AM
libcxx/docs/BuildingLibcxx.rst
63

Ok, that sounds good.

libcxx/docs/ReleaseNotes.rst
70

Ok, that's probably workable for me, hopefully it is for all other downstreams/vendors packaging it too.

I might request to postpone the removal ever so slightly, until 14.0.0 actually is released, since I try to test things across both latest tagged stable release, up until current git main branch - so after 14.x is branched, in the 1-2 months until the release is tagged, I build versions ranging from 13.0.0 up to early 15.x git main with the same setup. But I think the <monorepo>/runtimes setup might have worked well enough in the 13.0.0 release already (building worked, testing didn't, I think) that I could switch my build scripts already while keeping building on 13.0.0.

91

Ok, right - yeah the interop between libcxxabi and libcxx is tricky and it makes sense to deprecate it.

libcxx/utils/ci/run-buildbot
446

It took a little while to wrap my head around what this CI restructuring did exactly, so my first reaction was to split it out - but on second thought I don't mind keeping it coupled with the actual deprecation/docs change.

ldionne marked 5 inline comments as done.Oct 8 2021, 10:34 AM
ldionne added inline comments.
libcxx/docs/ReleaseNotes.rst
70

Sure, we can keep supporting it a bit longer. The goal is not to frustrate people, it's just to have an aggressive deprecation while still giving enough time for folks to migrate over without too much hassle.

libcxx/utils/ci/run-buildbot
446

Awesome!

phosek accepted this revision.Oct 8 2021, 3:11 PM
phosek added inline comments.
libcxx/docs/BuildingLibcxx.rst
25

Do you think it'd make sense to move some of this documentation to <monorepo>/runtimes/docs? It should apply to all runtimes, including compiler-rt, libc, openmp, etc.

ldionne marked 3 inline comments as done.Oct 11 2021, 8:03 AM
ldionne added inline comments.
libcxx/docs/BuildingLibcxx.rst
25

Yes, I do, however that'll require some tweaking around the setup we have for generating documentation. I think we'll want to have a holistic vision before we do that. For example, should there be a runtimes.llvm.org, or should it still be libcxx.llvm.org? I suggest we shed that discussion for the time being.

ldionne marked an inline comment as done.Oct 11 2021, 8:20 AM
ldionne added subscribers: thakis, dim, broadwaylamb and 5 others.

Subscribing folks who might be interested / impacted by this.

Long story short, this commit standardizes on exactly one way of building libc++/libc++abi/libunwind using a system compiler in addition to the existing bootstrapping build that already existed (commonly called the "Runtimes" build). Said differently, it deprecates (but doesn't remove support for) the Standalone build and the LLVM Project build. The Standalone build had been on life support for a while, but the LLVM Project build was the previously documented way of building the runtimes with a system compiler. Fortunately, both should be easy to migrate to the new build - please see the diff for details (in particular the added documentation and the release note).

The current goal is to drop support for the Standalone and the Project builds somewhere in the LLVM 15 time frame.

The benefits of this deprecation/removal is that we'll be able to greatly reduce the complexity of our CMake and provide adequate support for the official ways to build, whereas right now we can't really achieve that due to the large number of ways to build and their complexity. Furthermore, the new way to build using a system compiler will work on embedded platforms and will eventually allow us to remove the requirement to have all the <root>/llvm tree available to build the runtimes.

I will also send an email notice to llvm-dev and the other lists.

bcain added a subscriber: bcain.Oct 11 2021, 10:47 AM

Would several separate runtime builds with each time one project in LLVM_ENABLE_RUNTIMES work, to replace standalone builds ?

For custom embedded toolchains, there is sometime the need to do things in a specific order for bootstrapping :

  • cmake on libcxx
  • install libcxx headers
  • cmake, build and install libunwind
  • cmake and build libcxxabi
  • build libcxx

IIRC, even with CMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY, there is still the need to install certain headers for the cmake compile tests to work.

Sometimes there are also :

  • custom CXXFLAGS/LDFLAGS for one project
  • custom install steps (symlinks, linker scripts)

They could be needed for example to generate a gcc-like runtime on a Linux target for use by both clang and gcc : importing __gcc_personality_v0 in libunwind to allow it to replace libgcc_s.so.1 for glibc, libgcc_eh.a and libgcc_s.so linker scripts, ...

Would several separate runtime builds with each time one project in LLVM_ENABLE_RUNTIMES work, to replace standalone builds ?

For custom embedded toolchains, there is sometime the need to do things in a specific order for bootstrapping :

  • cmake on libcxx
  • install libcxx headers
  • cmake, build and install libunwind
  • cmake and build libcxxabi
  • build libcxx

IIRC, even with CMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY, there is still the need to install certain headers for the cmake compile tests to work.

Sometimes there are also :

  • custom CXXFLAGS/LDFLAGS for one project
  • custom install steps (symlinks, linker scripts)

They could be needed for example to generate a gcc-like runtime on a Linux target for use by both clang and gcc : importing __gcc_personality_v0 in libunwind to allow it to replace libgcc_s.so.1 for glibc, libgcc_eh.a and libgcc_s.so linker scripts, ...

There's of course nothing stopping you from configuring them one at a time that way, but based on earlier comments from @ldionne, the tricky options for hooking up separate standalone builds of libcxxabi and libcxx to each other would be deprecated.

These days, libunwind can be built without C++ standard library headers, so that one can be built entirely untangled from libcxxabi/libcxx. (Ideally it should be buildable in the same invocation as all the other ones too, but when bootstrapping toolchains from scratch, where libunwind is necessary for the libcxx cmake tests to succeed, you might need to build it first before doing cmake for libcxx.)

Some amount of these interdependencies (e.g. libunwind requiring standard C++ library headers) have been fixed in the last few years, so it might be good to revisit the procedure and see how much trickery is needed now. I think the most vital bit is that you should be able to build libcxxabi and libcxx in the same cmake invocation without specifying options for them to find each other.

I haven't seen a lot of discussion on this, either here or on the lists where I posted my RFC. Does this sounds reasonable to you all?

Pinging @tstellar and @bcain in particular because I think they are using standalone builds at the moment, and I want to have their blessing that this is going to work for them. Actually folks, you could move to the new "default" build right away to try it out if you wanted.

bcain added a comment.Oct 14 2021, 2:25 PM

I haven't seen a lot of discussion on this, either here or on the lists where I posted my RFC. Does this sounds reasonable to you all?

Pinging @tstellar and @bcain in particular because I think they are using standalone builds at the moment, and I want to have their blessing that this is going to work for them. Actually folks, you could move to the new "default" build right away to try it out if you wanted.

Louis, thanks so much for sending the notice to llvm-dev, it will really help us plan for this change. We needed an incentive to switch and this is a great forcing function. I was uncomfortable with the fact that we were using odd (less popular) build configurations downstream.

Your question about whether "this is going to work" -- do you mean the deprecation, or the ultimate removal? I don't see any issue w/the deprecation (though I haven't tried this patch yet). The ultimate change to only support runtimes-style builds is planned for post LLVM-14? That works for us, we'll start changing downstream build configs pretty soon, in ~Nov/Dec 2021. I can't identify any objection, I am hoping/expecting there's no issues for us moving to runtimes (or formerly-known-as-runtime).

I haven't seen a lot of discussion on this, either here or on the lists where I posted my RFC. Does this sounds reasonable to you all?

Pinging @tstellar and @bcain in particular because I think they are using standalone builds at the moment, and I want to have their blessing that this is going to work for them. Actually folks, you could move to the new "default" build right away to try it out if you wanted.

This is fine with me.

ldionne accepted this revision.Oct 18 2021, 7:51 AM

I haven't seen a lot of discussion on this, either here or on the lists where I posted my RFC. Does this sounds reasonable to you all?

Pinging @tstellar and @bcain in particular because I think they are using standalone builds at the moment, and I want to have their blessing that this is going to work for them. Actually folks, you could move to the new "default" build right away to try it out if you wanted.

Louis, thanks so much for sending the notice to llvm-dev, it will really help us plan for this change. We needed an incentive to switch and this is a great forcing function. I was uncomfortable with the fact that we were using odd (less popular) build configurations downstream.

Awesome! That's what communication is for :-).

Your question about whether "this is going to work" -- do you mean the deprecation, or the ultimate removal?

What I meant here was "do you think the new default build is going to work as a replacement for your existing standalone build setup?". The situation I want to avoid is one where we deprecate (and eventually remove) the standalone build, but without actually providing you a proper replacement. This whole deprecation process is based on the assumption that the new "default" build solves all the problems that the standalone build used to solve, but better. If that turns out not to be true for some users, I want to know about it so we can address these issues and make sure we cater to your needs.

The ultimate change to only support runtimes-style builds is planned for post LLVM-14? That works for us, we'll start changing downstream build configs pretty soon, in ~Nov/Dec 2021. I can't identify any objection, I am hoping/expecting there's no issues for us moving to runtimes (or formerly-known-as-runtime).

Actually, to clarify, we will keep supporting two types of build. The first one will be the "default" build, where one builds the runtimes with the system compiler (similarly to what you did with the standalone builds). The second one is what we call the "Runtimes build" today (we're moving to the "Bootstrapping build" naming), where one builds Clang from source and then builds the runtimes using that Clang.

There is no plan to force people to build their own Clang before building the runtimes.

This is fine with me.

Awesome!

So what I'm going to do now is ship this since it's only a policy change anyway. We're still supporting all the same builds for now, but now at least we have a clear path forward and people can start moving to the new ways to build. If someone has an objection on this, please comment here (and perhaps ping me on Discord to make sure I don't miss it). Even though this will be closed, we can still discuss the details of how we're going to make this transition.

This revision is now accepted and ready to land.Oct 18 2021, 7:51 AM
ldionne updated this revision to Diff 380398.Oct 18 2021, 7:53 AM

Rebase onto main

Cross referencing the post on llvm-dev just in case someone gets to this review and wants to know more information about this change: https://lists.llvm.org/pipermail/llvm-dev/2021-October/153238.html

That email explains the transition in more detail and answers a few common questions about it.