[CI] Use LLVM_ENABLE_RUNTIMES for runtimes builds on Linux

[boomanaiden154]

This patch switches us to using LLVM_ENABLE_RUNTIMES rather than using
separate runtimes builds for some reductions in CMake configuration time
and some simplification of the monolithic-linux.sh script.

[Endilll]

Something feels wrong with this PR:

1. There's a "full changes here" button on the Files Changed tab, which shows me a bunch of changes to libc++ tests adding #include <span>
2. In the new version, I don't see LLVM_ENABLE_RUTIMES passed to cmake. Not even CMAKE_BUILD_TYPE and CMAKE_CXX_COMPILER.

What's going on?

[boomanaiden154]

There's a "full changes here" button on the Files Changed tab, which shows me a bunch of changes to libc++ tests adding #include

I'm not sure how you're seeing this. Those changes are in #142693.

In the new version, I don't see LLVM_ENABLE_RUTIMES passed to cmake. Not even CMAKE_BUILD_TYPE and CMAKE_CXX_COMPILER.

We're just modifying the existing CMake invocation. That one has LLVM_ENABLE_RUNTIMES set to libcxx;libcxxabi;libunwind. #142695 and #142696 modify that though so we can start building libc and compiler-rt through the runtimes build given building them through LLVM_ENABLE_PROJECTS is going to be deprecated soon.

[DavidSpickett]

CMake once with the runtimes enabled. Modify the test parameters each time.

Looks good to me.

[Endilll]

I'm not sure how you're seeing this.

[Endilll]

I just opened 9e3490b in my local git, and yes, all libc++ changes are there alongside changes to monolithic-linux.sh that we're interested in

[Endilll]

We're just modifying the existing CMake invocation. That one has LLVM_ENABLE_RUNTIMES set to libcxx;libcxxabi;libunwind. #142695 and #142696 modify that though so we can start building libc and compiler-rt through the runtimes build given building them through LLVM_ENABLE_PROJECTS is going to be deprecated soon.

What I see is that you're reusing the same initial CMake config for everything, even though in order to test Clang against libc++ and its tests, we need to reconfigure CMake to use just-built Clang, by supplying -D CMAKE_C_COMPILER="${INSTALL_DIR}/bin/clang" and -D CMAKE_CXX_COMPILER="${INSTALL_DIR}/bin/clang++". Otherwise we're simply losing this additional coverage for Clang.

I don't see this addressed in two PRs you linked.

[cor3ntin]

This is a merge commit, it might be worth rebasing / cherry picking on a clean branch

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Aiden Grossman (boomanaiden154)

Changes

This patch switches us to using LLVM_ENABLE_RUNTIMES rather than using
separate runtimes builds for some reductions in CMake configuration time
and some simplification of the monolithic-linux.sh script.

---

Full diff: https://github.com/llvm/llvm-project/pull/142694.diff

1 Files Affected:

• (modified) .ci/monolithic-linux.sh (+10-36)

diff --git a/.ci/monolithic-linux.sh b/.ci/monolithic-linux.sh
index f5a31fa45a641..52a80958b4025 100755
--- a/.ci/monolithic-linux.sh
+++ b/.ci/monolithic-linux.sh
@@ -102,51 +102,25 @@ if [[ "${runtimes}" != "" ]]; then
     exit 1
   fi
 
-  echo "--- ninja install-clang"
-
-  ninja -C ${BUILD_DIR} install-clang install-clang-resource-headers
-
-  RUNTIMES_BUILD_DIR="${MONOREPO_ROOT}/build-runtimes"
-  INSTALL_DIR="${BUILD_DIR}/install"
-  mkdir -p ${RUNTIMES_BUILD_DIR}
-
   echo "--- cmake runtimes C++26"
 
-  rm -rf "${RUNTIMES_BUILD_DIR}"
-  cmake -S "${MONOREPO_ROOT}/runtimes" -B "${RUNTIMES_BUILD_DIR}" -GNinja \
-      -D CMAKE_C_COMPILER="${INSTALL_DIR}/bin/clang" \
-      -D CMAKE_CXX_COMPILER="${INSTALL_DIR}/bin/clang++" \
-      -D LLVM_ENABLE_RUNTIMES="${runtimes}" \
-      -D LIBCXX_CXX_ABI=libcxxabi \
-      -D CMAKE_BUILD_TYPE=RelWithDebInfo \
-      -D CMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
-      -D LIBCXX_TEST_PARAMS="std=c++26" \
-      -D LIBCXXABI_TEST_PARAMS="std=c++26" \
-      -D LLVM_LIT_ARGS="${lit_args}"
+  cmake \
+    -D LIBCXX_TEST_PARAMS="std=c++26" \
+    -D LIBCXXABI_TEST_PARAMS="std=c++26" \
+    "${BUILD_DIR}"
 
   echo "--- ninja runtimes C++26"
 
-  ninja -vC "${RUNTIMES_BUILD_DIR}" ${runtime_targets}
+  ninja -C "${BUILD_DIR}" ${runtime_targets}
 
   echo "--- cmake runtimes clang modules"
 
-  # We don't need to do a clean build of runtimes, because LIBCXX_TEST_PARAMS
-  # and LIBCXXABI_TEST_PARAMS only affect lit configuration, which successfully
-  # propagates without a clean build. Other that those two variables, builds
-  # are supposed to be the same.
-
-  cmake -S "${MONOREPO_ROOT}/runtimes" -B "${RUNTIMES_BUILD_DIR}" -GNinja \
-      -D CMAKE_C_COMPILER="${INSTALL_DIR}/bin/clang" \
-      -D CMAKE_CXX_COMPILER="${INSTALL_DIR}/bin/clang++" \
-      -D LLVM_ENABLE_RUNTIMES="${runtimes}" \
-      -D LIBCXX_CXX_ABI=libcxxabi \
-      -D CMAKE_BUILD_TYPE=RelWithDebInfo \
-      -D CMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
-      -D LIBCXX_TEST_PARAMS="enable_modules=clang" \
-      -D LIBCXXABI_TEST_PARAMS="enable_modules=clang" \
-      -D LLVM_LIT_ARGS="${lit_args}"
+  cmake \
+    -D LIBCXX_TEST_PARAMS="enable_modules=clang" \
+    -D LIBCXXABI_TEST_PARAMS="enable_modules=clang" \
+    "${BUILD_DIR}"
 
   echo "--- ninja runtimes clang modules"
 
-  ninja -vC "${RUNTIMES_BUILD_DIR}" ${runtime_targets}
+  ninja -C "${BUILD_DIR}" ${runtime_targets}
 fi

[boomanaiden154]

What I see is that you're reusing the same initial CMake config for everything, even though in order to test Clang against libc++ and its tests, we need to reconfigure CMake to use just-built Clang, by supplying -D CMAKE_C_COMPILER="${INSTALL_DIR}/bin/clang" and -D CMAKE_CXX_COMPILER="${INSTALL_DIR}/bin/clang++". Otherwise we're simply losing this additional coverage for Clang.

We're using LLVM_ENABLE_RUNTIMES. It uses the just built clang to build the runtimes specified. We're just doing this in a single CMake invocation (which will spawn sub-CMake invocations) rather than in multiple CMake invocations.

[Endilll]

We're using LLVM_ENABLE_RUNTIMES. It uses the just built clang to build the runtimes specified.

That explains it, thank you.
There's still an outstanding question of unrelated changes to libc++ tests that are included in this PR.

[boomanaiden154]

There's still an outstanding question of unrelated changes to libc++ tests that are included in this PR.

I'm still not sure how they're ending up in here. I haven't seen this before with spr. This will definitely be fixed before I end up landing the patch and I'm guessing will be resolved when I change the branch target to main.

[boomanaiden154]

Branch seems to be cleaned up now.

[ldionne]

I think I don't quite understand what this change does. You're basically just re-generating the CMake cache and re-running ninja every time?

[boomanaiden154]

Yes. Rerunning CMake with the existing cache so we don't have to rerun all the checks just to change the test configuration. Then we run ninja which goes through the runtimes build. This is mainly to make it simpler to move compiler-rt and libc over to the runtimes build.