This is an archive of the discontinued LLVM Phabricator instance.

extend llvm variables for build flexibility
Needs RevisionPublic

Authored by ozkanonur on Mar 8 2023, 3:05 PM.

Details

Summary

Currently we are using llvm as a submodule in Rust language and building it actively during the development or in CI pipelines. This change allows us to only include the libs/tools/utils we need which reduces the build time a lot for developers and CI pipelines. I believe this will be good for any other individual developers as well.

Diff Detail

Event Timeline

ozkanonur created this revision.Mar 8 2023, 3:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 8 2023, 3:05 PM
ozkanonur requested review of this revision.Mar 8 2023, 3:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 8 2023, 3:05 PM
smeenai requested changes to this revision.Mar 8 2023, 7:14 PM
smeenai added reviewers: beanz, compnerd, phosek, tstellar.

Adding the usual CMake reviewers.

We've generally tried to move away from the direction of having CMake variables to include/exclude parts of the build, and instead encourage users to only build the specific targets they need instead of the all target. LLVM's CMake has support for distributions to make that more ergonomic. LLVM also exports its targets, so Rust could import them and add a dependency on them directly. Do either of those work for you?

You should upload patches with full context. See https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface if you're using the web interface or https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-command-line if you set up Arcanist.

This revision now requires changes to proceed.Mar 8 2023, 7:14 PM

One other alternative for Rust to consider would be to support building as part of an LLVM build using LLVM_EXTERNAL_PROJECTS and LLVM_EXTERNAL_<PROJECT>_SOURCE_DIR, so that Rust becomes part of the LLVM build graph and can easily access its targets. See https://github.com/apple/swift/blob/c95e11c011e5e6ee19f9c21332ff04df29dd49db/cmake/modules/SwiftSharedCMakeConfig.cmake#L269 for how Swift does it.

I agree with @smeenai, I think we should try to come with a more maintainable and less error prone approach.

LLVM libraries and tools have dependencies. If the user enables only a subset of libraries or tools, but not their dependencies, it'll result in broken output. Since the build graph evolves over time, the set of libraries and tools that needs to be selected is also going to change over time complicating the maintenance.

This is not just a theoretical concern; in Fuchsia we build a subset of LLVM libraries for use in our debugger (we use LLVM_DISTRIBUTION_COMPONENTS to specify which ones) and our build breaks regularly because of changing dependencies to the point where we started looking for a better solution.

ozkanonur added a comment.EditedMar 9 2023, 2:56 AM

We've generally tried to move away from the direction of having CMake variables to include/exclude parts of the build, and instead encourage users to only build the specific targets they need instead of the all target. LLVM's CMake has support for distributions to make that more ergonomic. LLVM also exports its targets, so Rust could import them and add a dependency on them directly. Do either of those work for you?

We are already specify only required targets to reduce compilation and it's helping a lot. But still, a lot of compilation happens for things that we absolutely don't need in Rust. And, I don't think any of those really helps.

Here is the part where we configure llvm build https://github.com/rust-lang/rust/blob/66a2d6221069e0d08ceacf2a3201600e2092d2e0/src/bootstrap/native.rs#L248-L520

On top of our existing llvm build flow, with the PR I am proposing here, we will be able to do the following:

diff
@@ -326,10 +319,18 @@ fn run(self, builder: &Builder<'_>) -> LlvmResult {
             .define("LLVM_ENABLE_LIBEDIT", "OFF")
             .define("LLVM_ENABLE_BINDINGS", "OFF")
             .define("LLVM_ENABLE_Z3_SOLVER", "OFF")
+            .define("LLVM_ENABLE_TOOLS", "llvm-bcanalyzer;llvm-cov;llvm-dis;llvm-dwarfdump;llvm-dwp;llvm-nm;llvm-objdump")
+            .define("LLVM_ENABLE_UTILS", "FileCheck")
+            .define("LLVM_ENABLE_LIBS", "only needed libraries will be here")
             .define("LLVM_PARALLEL_COMPILE_JOBS", builder.jobs().to_string())
             .define("LLVM_TARGET_ARCH", target_native.split('-').next().unwrap())
             .define("LLVM_DEFAULT_TARGET_TRIPLE", target_native);

Which will help us(not only us for sure, ref stackoverflow topic: https://stackoverflow.com/questions/71614937/llvm-build-only-selected-tools-or-targets) a lot by reducing compilation times.

I understand the design concerns here. But wouldn't be possible if we inform the developer in docs? Like "this might break builds/tests when incorrectly used". If I am not mistaken llvm already warns such things for some of the flags/variables.

Heh, "target" being an extremely overloaded term strikes again :)

You're using target in the sense of an LLVM backend target (LLVM_TARGETS_TO_BUILD). I meant it in the sense of a build system target, e.g. clang, lld, llvm-ar, libLLVMAnalysis.a, etc. Every tool, utility and library that you're trying to enable or disable at the CMake level has its own build system target, and you should only specify the build system targets you need, instead of trying to limit that at the configuration level and then building every single target. There's also umbrella targets like llvm-libraries and clang-libraries as conveniences for referring to all libraries for a particular project.