[llvm] Remove libcxx, libcxxabi and libunwind from supported LLVM_ENABLE_PROJECTS

[ldionne]

Summary

This is a breaking change. If you were passing one of those three runtimes
in LLVM_ENABLE_PROJECTS, you need to start passing them in LLVM_ENABLE_RUNTIMES
instead. The runtimes in LLVM_ENABLE_RUNTIMES will start being built using
the "bootstrapping build" instead, which means that they will be built
using the just-built Clang. This is usually what you wanted anyway.

If you were using LLVM_ENABLE_PROJECTS=all with the explicit goal of
building these three runtimes, you can now use LLVM_ENABLE_RUNTIMES=all
and these runtimes will be built using the bootstrapping build.

[phosek]

LGTM

[bcain]

@ldionne do you have an idea when you plan to land this change? I'm pretty sure it won't break our usage but I'd like to test it out to be sure.

I don't see any LLVM_ENABLE_PROJECTS (beyond clang+lld) - only LLVM_ENABLE_RUNTIMES so I think we're ok.

https://github.com/quic/toolchain_for_hexagon/blob/29a926ac283a4afb9dc4d5ae4a0f2c18b6649243/build-toolchain.sh

[bcain]

In D132480#3760209, @bcain wrote:

@ldionne do you have an idea when you plan to land this change? I'm pretty sure it won't break our usage but I'd like to test it out to be sure.

I don't see any LLVM_ENABLE_PROJECTS (beyond clang+lld) - only LLVM_ENABLE_RUNTIMES so I think we're ok.

https://github.com/quic/toolchain_for_hexagon/blob/29a926ac283a4afb9dc4d5ae4a0f2c18b6649243/build-toolchain.sh

Oh - I see now, LLVM_ENABLE_PROJECTS=libcxx;libcxxabi is used by our downstream toolchain and not this upstream one. If you could see fit to give us ~one week before you land this, that would be appreciated. And if not, well -- it's not as if we weren't warned.

[ldionne]

Rebase onto main.

[ldionne]

I'm in no hurry to ship this, but 1 week would seem reasonable. I've been trying to make changes in this stack very slowly to give time for folks to react before we've landed a bunch of changes that would make reverting more difficult.

Pinging libc++ vendors for awareness. I'll aim to land this BREAKING CHANGE in roughly 1 week.

[Ericson2314]

In D132324 I also removed the hack in clang/lib/Driver/ToolChains/Linux.cpp, I think you want to do that too?

[ldionne]

In D132480#3783088, @Ericson2314 wrote:

In D132324 I also removed the hack in clang/lib/Driver/ToolChains/Linux.cpp, I think you want to do that too?

I think it makes sense to do that separately as a (in theory) NFC patch.

[Ericson2314]

OK. As long as we don't forget about it :).

[aaron.ballman]

It looks like this change took down our Sphinx publication bot: https://lab.llvm.org/buildbot/#/builders/89/builds/33379 -- @tonic are you able to help resolve this?

[tonic]

Adding @andreil99 as he can help with the sphinx docs.

[ldionne]

Do you want me to revert? I engineered this so as to make it easy to revert if needed.

[andreil99]

I can fix the buildbot if you confirm that both Sphinx and Doxygen documentation gets built with LLVM_ENABLE_RUNTIMES.

[aaron.ballman]

Btw, it looks like this hit the lldb doc building bot as well: https://lab.llvm.org/buildbot/#/builders/31/builds/28249

In D132480#3803630, @andreil99 wrote:

I can fix the buildbot if you confirm that both Sphinx and Doxygen documentation gets built with LLVM_ENABLE_RUNTIMES.

This is what I'm seeing on the lldb bot output, which suggests we don't enable LLVM_ENABLE_RUNTIMES currently:

argv: ['cmake', '../src/llvm', '-DLLVM_ENABLE_SPHINX=ON', '-DSPHINX_OUTPUT_HTML=ON', '-DSPHINX_OUTPUT_MAN=ON', '-DLLDB_INCLUDE_TESTS=OFF', '-DLLVM_TEMPORARILY_ALLOW_OLD_TOOLCHAIN=ON', '-DLLVM_ENABLE_ASSERTIONS=OFF', '-DLLVM_ENABLE_PROJECTS=llvm;libunwind', '-DCMAKE_BUILD_TYPE=Release', '-DLLVM_LIT_ARGS=-v -vv', '-GNinja']

and the publish bot uses:

cmake ../llvm-project/llvm -DLLVM_ENABLE_SPHINX=ON -DSPHINX_OUTPUT_HTML=ON -DSPHINX_OUTPUT_MAN=OFF -DSPHINX_WARNINGS_AS_ERRORS=OFF -DLLVM_ENABLE_ASSERTIONS=OFF -DCMAKE_BUILD_TYPE=Release '-DLLVM_ENABLE_PROJECTS=libc;clang-tools-extra;libcxx;lld;libunwind;openmp;clang;llvm;polly;lldb;libcxxabi;flang' '-DLLVM_LIT_ARGS=-v -vv' -GNinja
``

[aaron.ballman]

In D132480#3803617, @ldionne wrote:

Do you want me to revert? I engineered this so as to make it easy to revert if needed.

I don't have strong feelings; I think we can live for a day with one-day-out-of-date docs up on the web. But if @andreil99 would like the extra time to look into it, I'm also totally fine with reverting.

[andreil99]

This hits all the doc building bots on the exact same way.

The fix is trivial, assuming documentation builds properly with LLVM_ENABLE_RUNTIMES. It didn't support that before.
I cannot check that quickly, unfortunately. So, if somebody can check and confirm, that would help a lot.

[ldionne]

In D132480#3803725, @andreil99 wrote:

This hits all the doc building bots on the exact same way.

The fix is trivial, assuming documentation builds properly with LLVM_ENABLE_RUNTIMES. It didn't support that before.
I cannot check that quickly, unfortunately. So, if somebody can check and confirm, that would help a lot.

It should, give me a second to confirm that on a Linux docker image.

[Ericson2314]

If we try to build docs with LLVM_ENABLE_RUNTIMES might it try to bootstrap them, wasting effort?

That was my concern when I hit this before. TBH I feel putting them under LLVM_ENABLE_PROJECTS while silly, did express well that everything can be built in parallel (modulo "regular" header dependencies like Clang -> LLVM) without bootstrapping being needed.

[ldionne]

In D132480#3804015, @Ericson2314 wrote:

If we try to build docs with LLVM_ENABLE_RUNTIMES might it try to bootstrap them, wasting effort?

No, it won't.

I'm still working on this, I'll update this thread tomorrow. If I don't manage to get this working tomorrow morning, I'll revert this.

[ldionne]

In D132480#3804400, @ldionne wrote:

In D132480#3804015, @Ericson2314 wrote:

If we try to build docs with LLVM_ENABLE_RUNTIMES might it try to bootstrap them, wasting effort?

No, it won't.

I'm still working on this, I'll update this thread tomorrow. If I don't manage to get this working tomorrow morning, I'll revert this.

I'm going deeper and deeper into the rabbit hole and as I gain more understanding of how the bootstrapping build works, this is not looking positive. Basically, my understanding is that we *have* to build Clang (and compiler-rt) before we even configure the other runtimes, because that's required for their CMake configuration to work properly. When we configure the runtimes like libcxx, CMake performs various compiler checks (like is -fsomethingsomething available), and that obviously requires having the compiler built already. When I think about it, it makes sense: the documentation could even technically include different content based on the compiler we're building libc++ for (although we don't do that and I think it'd be a bad idea).

This is obviously wasted effort in the context of generating only our documentation. Currently, the only way forward I can think of is to change the documentation jobs to perform two CMake invocations: first build the documentation with an invocation rooted at llvm and using LLVM_ENABLE_PROJECTS=clang;lld;etc, and then build the documentation for the runtimes with an invocation rooted at <root>/runtimes using LLVM_ENABLE_RUNTIMES=libcxx;libcxxabi;etc. I'm going to revert this change temporarily as I look at how to make these changes.

That was my concern when I hit this before. TBH I feel putting them under LLVM_ENABLE_PROJECTS while silly, did express well that everything can be built in parallel (modulo "regular" header dependencies like Clang -> LLVM) without bootstrapping being needed.

Yeah, I get you. That being said, there were a lot of problems with supporting that, for example the fact that llvm/CMakeLists.txt creates a lot of variables and selects a lot of defaults that don't make sense for the runtimes. I had documented that in some of the posts I made when we agreed to move the runtimes out of LLVM_ENABLE_PROJECTS a few years back.

[aaron.ballman]

In D132480#3805546, @ldionne wrote:

I'm going deeper and deeper into the rabbit hole and as I gain more understanding of how the bootstrapping build works, this is not looking positive.

Thank you for doing the research here, and thank you for the revert while continuing to investigate!

[ldionne]

Rebase onto main.

[ldionne]

@andreil99 Are the sphinx documentation publishing changes live (so I can commit this again)?

[ldionne]

Rebase again.

[andreil99]

In D132480#3833227, @ldionne wrote:

@andreil99 Are the sphinx documentation publishing changes live (so I can commit this again)?

Correct.

[aaron.ballman]

It looks like the libunwind builder is broken by these changes: https://lab.llvm.org/buildbot/#/builders/31/builds/28817

[Ericson2314]

@ldionne Yeah that sort of thing is what I suspected. I am thinking maybe it isn't so bad that these projects can be built this way. IMO it should be permissible to combine any two CMake projects together if they are just to be built in parallel without any dependencies between them.

[gkistanova]

In D132480#3833369, @aaron.ballman wrote:

It looks like the libunwind builder is broken by these changes: https://lab.llvm.org/buildbot/#/builders/31/builds/28817

This is on my plate. I’ll commit the fix shortly. It will pick up at the next buildbot restart.

libcxx/docs/ReleaseNotes.rst

- In freestanding mode, ``atomic<small enum class>`` does not contain a lock byte anymore if the platform
   the platform supporting lockfree atomics for that size. Starting in LLVM 15.0.1, the ABI for these types has been
   restored to what it used to be (no lock byte), which is the most efficient implementation.
 
   This ABI break only affects users that compile with ``-ffreestanding``, and only for ``atomic<T>`` where ``T``
   is a non-builtin type that could be lockfree on the platform. See https://llvm.org/D133377 for more details.
 
 Build System Changes
 --------------------
+- Support for ``libcxx``, ``libcxxabi`` and ``libunwind`` in ``LLVM_ENABLE_PROJECTS`` has officially
+  been removed. Instead, please build according to :ref:`these instructions <build instructions>`.

libcxx/utils/ci/buildkite-pipeline.yml

agents:
       queue: "libcxx-builders"
       os: "linux"
     retry:
       automatic:
         - exit_status: -1  # Agent was lost
           limit: 2
     timeout_in_minutes: 120
 
-  - group: "Legacy"
-    steps:
-    - label: "Legacy LLVM_ENABLE_PROJECTS build"
-      command: "libcxx/utils/ci/run-buildbot legacy-project-build"
-      artifact_paths:
-        - "**/test-results.xml"
-        - "**/*.abilist"
-      env:
-          CC: "clang-${LLVM_HEAD_VERSION}"
-          CXX: "clang++-${LLVM_HEAD_VERSION}"
-      agents:
-        queue: "libcxx-builders"
-        os: "linux"
-      retry:
-        automatic:
-          - exit_status: -1  # Agent was lost
-            limit: 2
-      timeout_in_minutes: 120
-
   # Tests with various build configurations.
   - label: "Static libraries"
     command: "libcxx/utils/ci/run-buildbot generic-static"
     artifact_paths:
       - "**/test-results.xml"
       - "**/*.abilist"
     env:
         CC: "clang-${LLVM_HEAD_VERSION}"

libcxx/utils/ci/run-buildbot

${CMAKE} \
           -DLLVM_ENABLE_ASSERTIONS=ON
 
     echo "+++ Running the libc++ and libc++abi tests"
     ${NINJA} -C "${BUILD_DIR}" check-runtimes
 
     echo "--- Installing libc++ and libc++abi to a fake location"
     ${NINJA} -C "${BUILD_DIR}" install-runtimes
 ;;
-legacy-project-build)
-    clean
-
-    echo "--- Generating CMake"
-    ${CMAKE} \
-          -S "${MONOREPO_ROOT}/llvm" \
-          -B "${BUILD_DIR}" \
-          -DLLVM_ENABLE_PROJECTS="libcxx;libunwind;libcxxabi" \
-          -GNinja -DCMAKE_MAKE_PROGRAM="${NINJA}" \
-          -DCMAKE_BUILD_TYPE=RelWithDebInfo \
-          -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
-          -DLLVM_LIT_ARGS="-sv --show-unsupported --xunit-xml-output test-results.xml --timeout=1500" \
-          -DLIBCXX_CXX_ABI=libcxxabi
-    check-runtimes
-;;
 aarch64)
     clean
     generate-cmake -C "${MONOREPO_ROOT}/libcxx/cmake/caches/AArch64.cmake"
     check-runtimes
 ;;
 aarch64-noexceptions)
     clean
     generate-cmake -C "${MONOREPO_ROOT}/libcxx/cmake/caches/AArch64.cmake" \

llvm/CMakeLists.txt

elseif(CMAKE_BUILD_TYPE)
     set(CMAKE_TRY_COMPILE_CONFIGURATION ${CMAKE_BUILD_TYPE})
   endif()
 endif()
 
 # Side-by-side subprojects layout: automatically set the
 # LLVM_EXTERNAL_${project}_SOURCE_DIR using LLVM_ALL_PROJECTS
 # This allows an easy way of setting up a build directory for llvm and another
 # one for llvm+clang+... using the same sources.
-set(LLVM_ALL_PROJECTS "bolt;clang;clang-tools-extra;compiler-rt;cross-project-tests;libc;libclc;libcxx;libcxxabi;libunwind;lld;lldb;mlir;openmp;polly;pstl")
+set(LLVM_ALL_PROJECTS "bolt;clang;clang-tools-extra;compiler-rt;cross-project-tests;libc;libclc;lld;lldb;mlir;openmp;polly;pstl")
 # The flang project is not yet part of "all" projects (see C++ requirements)
 set(LLVM_EXTRA_PROJECTS "flang")
 # List of all known projects in the mono repo
 set(LLVM_KNOWN_PROJECTS "${LLVM_ALL_PROJECTS};${LLVM_EXTRA_PROJECTS}")
 set(LLVM_ENABLE_PROJECTS "" CACHE STRING
     "Semicolon-separated list of projects to build (${LLVM_KNOWN_PROJECTS}), or \"all\".")
 # Make sure expansion happens first to not handle "all" in rest of the checks.
 if( LLVM_ENABLE_PROJECTS STREQUAL "all" )
   set( LLVM_ENABLE_PROJECTS ${LLVM_ALL_PROJECTS})
 endif()
 foreach(proj ${LLVM_ENABLE_PROJECTS})
   if (NOT proj STREQUAL "llvm" AND NOT "${proj}" IN_LIST LLVM_KNOWN_PROJECTS)
-     MESSAGE(FATAL_ERROR "${proj} isn't a known project: ${LLVM_KNOWN_PROJECTS}")
-  endif()
-endforeach()
-foreach(proj "libcxx" "libcxxabi" "libunwind")
-  if (${proj} IN_LIST LLVM_ENABLE_PROJECTS)
-    message(WARNING "Using LLVM_ENABLE_PROJECTS=${proj} is deprecated now, please use -DLLVM_ENABLE_RUNTIMES=${proj} or "
-                    "see the instructions at https://libcxx.llvm.org/BuildingLibcxx.html for building the runtimes.")
+     MESSAGE(FATAL_ERROR "${proj} isn't a known project: ${LLVM_KNOWN_PROJECTS}. Did you mean to enable it as a runtime in LLVM_ENABLE_RUNTIMES?")
   endif()
 endforeach()
 
 if ("flang" IN_LIST LLVM_ENABLE_PROJECTS)
   if (NOT "mlir" IN_LIST LLVM_ENABLE_PROJECTS)
     message(STATUS "Enabling MLIR as a dependency to flang")
     list(APPEND LLVM_ENABLE_PROJECTS "mlir")
   endif()