Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable zstd for debug compression. #125642

Merged
merged 4 commits into from
Aug 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions compiler/rustc_llvm/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,15 +259,32 @@ fn main() {
cmd.args(&components);

for lib in output(&mut cmd).split_whitespace() {
let mut is_static = false;
let name = if let Some(stripped) = lib.strip_prefix("-l") {
stripped
} else if let Some(stripped) = lib.strip_prefix('-') {
stripped
} else if Path::new(lib).exists() {
// On MSVC llvm-config will print the full name to libraries, but
// we're only interested in the name part
let name = Path::new(lib).file_name().unwrap().to_str().unwrap();
name.trim_end_matches(".lib")
// On Unix when we get a static library llvm-config will print the
// full name and we *are* interested in the path, but we need to
// handle it separately. For example, when statically linking to
// libzstd llvm-config will output something like
// -lrt -ldl -lm -lz /usr/local/lib/libzstd.a -lxml2
// and we transform the zstd part into
// cargo:rustc-link-search-native=/usr/local/lib
// cargo:rustc-link-lib=static=zstd
let path = Path::new(lib);
if lib.ends_with(".a") {
is_static = true;
println!("cargo:rustc-link-search=native={}", path.parent().unwrap().display());
let name = path.file_stem().unwrap().to_str().unwrap();
name.trim_start_matches("lib")
} else {
let name = path.file_name().unwrap().to_str().unwrap();
name.trim_end_matches(".lib")
}
} else if lib.ends_with(".lib") {
// Some MSVC libraries just come up with `.lib` tacked on, so chop
// that off
Expand All @@ -285,7 +302,13 @@ fn main() {
continue;
}

let kind = if name.starts_with("LLVM") { llvm_kind } else { "dylib" };
let kind = if name.starts_with("LLVM") {
llvm_kind
} else if is_static {
"static"
} else {
"dylib"
};
println!("cargo:rustc-link-lib={kind}={name}");
}

Expand Down
3 changes: 3 additions & 0 deletions config.example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@
# library provided by LLVM.
#static-libstdcpp = false

# Enable LLVM to use zstd for compression.
#libzstd = false

# Whether to use Ninja to build LLVM. This runs much faster than make.
#ninja = true

Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/download-ci-llvm-stamp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Change this file to make users of the `download-ci-llvm` configuration download
a new version of LLVM from CI, even if the LLVM submodule hasn’t changed.

Last change is for: https://github.com/rust-lang/rust/pull/126298
Last change is for: https://github.com/rust-lang/rust/pull/125642
12 changes: 9 additions & 3 deletions src/bootstrap/src/core/build_steps/llvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,9 +368,7 @@ impl Step for Llvm {
cfg.define("LLVM_PROFDATA_FILE", path);
}

// Disable zstd to avoid a dependency on libzstd.so.
cfg.define("LLVM_ENABLE_ZSTD", "OFF");

// Libraries for ELF section compression.
if !target.is_windows() {
cfg.define("LLVM_ENABLE_ZLIB", "ON");
} else {
Expand Down Expand Up @@ -824,6 +822,14 @@ fn configure_llvm(builder: &Builder<'_>, target: TargetSelection, cfg: &mut cmak
}
}

// Libraries for ELF section compression.
if builder.config.llvm_libzstd {
cfg.define("LLVM_ENABLE_ZSTD", "FORCE_ON");
cfg.define("LLVM_USE_STATIC_ZSTD", "TRUE");
} else {
cfg.define("LLVM_ENABLE_ZSTD", "OFF");
}
khuey marked this conversation as resolved.
Show resolved Hide resolved

if let Some(ref linker) = builder.config.llvm_use_linker {
cfg.define("LLVM_USE_LINKER", linker);
}
Expand Down
6 changes: 6 additions & 0 deletions src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ pub struct Config {
pub llvm_thin_lto: bool,
pub llvm_release_debuginfo: bool,
pub llvm_static_stdcpp: bool,
pub llvm_libzstd: bool,
/// `None` if `llvm_from_ci` is true and we haven't yet downloaded llvm.
#[cfg(not(test))]
llvm_link_shared: Cell<Option<bool>>,
Expand Down Expand Up @@ -878,6 +879,7 @@ define_config! {
plugins: Option<bool> = "plugins",
ccache: Option<StringOrBool> = "ccache",
static_libstdcpp: Option<bool> = "static-libstdcpp",
libzstd: Option<bool> = "libzstd",
ninja: Option<bool> = "ninja",
targets: Option<String> = "targets",
experimental_targets: Option<String> = "experimental-targets",
Expand Down Expand Up @@ -1153,6 +1155,7 @@ impl Config {
llvm_optimize: true,
ninja_in_file: true,
llvm_static_stdcpp: false,
llvm_libzstd: false,
backtrace: true,
rust_optimize: RustOptimize::Bool(true),
rust_optimize_tests: true,
Expand Down Expand Up @@ -1787,6 +1790,7 @@ impl Config {
plugins,
ccache,
static_libstdcpp,
libzstd,
ninja,
targets,
experimental_targets,
Expand Down Expand Up @@ -1821,6 +1825,7 @@ impl Config {
set(&mut config.llvm_thin_lto, thin_lto);
set(&mut config.llvm_release_debuginfo, release_debuginfo);
set(&mut config.llvm_static_stdcpp, static_libstdcpp);
set(&mut config.llvm_libzstd, libzstd);
if let Some(v) = link_shared {
config.llvm_link_shared.set(Some(v));
}
Expand Down Expand Up @@ -1871,6 +1876,7 @@ impl Config {
check_ci_llvm!(optimize_toml);
check_ci_llvm!(thin_lto);
check_ci_llvm!(release_debuginfo);
check_ci_llvm!(libzstd);
check_ci_llvm!(targets);
check_ci_llvm!(experimental_targets);
check_ci_llvm!(clang_cl);
Expand Down
5 changes: 5 additions & 0 deletions src/bootstrap/src/utils/change_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,4 +220,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[
severity: ChangeSeverity::Warning,
summary: "For tarball sources, default value for `rust.channel` will be taken from `src/ci/channel` file.",
},
ChangeInfo {
change_id: 125642,
severity: ChangeSeverity::Info,
summary: "New option `llvm.libzstd` to control whether llvm is built with zstd support.",
},
];
5 changes: 5 additions & 0 deletions src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ COPY host-x86_64/dist-x86_64-linux/build-clang.sh /tmp/
RUN ./build-clang.sh
ENV CC=clang CXX=clang++

# rustc's LLVM needs zstd.
COPY scripts/zstd.sh /tmp/
RUN ./zstd.sh

COPY scripts/sccache.sh /scripts/
RUN sh /scripts/sccache.sh

Expand All @@ -79,6 +83,7 @@ ENV RUST_CONFIGURE_ARGS \
--set target.x86_64-unknown-linux-gnu.ranlib=/rustroot/bin/llvm-ranlib \
--set llvm.thin-lto=true \
--set llvm.ninja=false \
--set llvm.libzstd=true \
--set rust.jemalloc \
--set rust.use-lld=true \
--set rust.lto=thin \
Expand Down
2 changes: 2 additions & 0 deletions src/ci/docker/host-x86_64/x86_64-gnu/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
pkg-config \
xz-utils \
mingw-w64 \
zlib1g-dev \
libzstd-dev \
&& rm -rf /var/lib/apt/lists/*

COPY scripts/sccache.sh /scripts/
Expand Down
29 changes: 29 additions & 0 deletions src/ci/docker/scripts/zstd.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#!/bin/bash
set -ex

hide_output() {
set +x
on_err="
echo ERROR: An error was encountered with the build.
cat /tmp/zstd_build.log
exit 1
"
trap "$on_err" ERR
bash -c "while true; do sleep 30; echo \$(date) - building ...; done" &
PING_LOOP_PID=$!
"$@" &> /tmp/zstd_build.log
trap - ERR
kill $PING_LOOP_PID
rm /tmp/zstd_build.log
set -x
}

ZSTD=1.5.6
curl -L https://github.com/facebook/zstd/releases/download/v$ZSTD/zstd-$ZSTD.tar.gz | tar xzf -

cd zstd-$ZSTD
CFLAGS=-fPIC hide_output make -j$(nproc) VERBOSE=1
hide_output make install

cd ..
rm -rf zstd-$ZSTD
1 change: 1 addition & 0 deletions tests/run-make/rust-lld-compress-debug-sections/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main() {}
39 changes: 39 additions & 0 deletions tests/run-make/rust-lld-compress-debug-sections/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Checks the `compress-debug-sections` option on rust-lld.

//@ needs-rust-lld
//@ only-linux
//@ ignore-cross-compile

// FIXME: This test isn't comprehensive and isn't covering all possible combinations.

use run_make_support::{assert_contains, cmd, llvm_readobj, run_in_tmpdir, rustc};
Comment on lines +7 to +9
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@khuey To elaborate a bit why it might be desirable to extend compiletest a bit here: Currently, this test doesn't guarantee we actually test that zstd and zlib compression works, as noted in this FIXME.

We could at least test both positive cases if the necessary case recognition was added to compiletest's headers, so that we can implement what you currently do using a runtime if by instead using compiletest's revisions.

Copy link
Member

@jieyouxu jieyouxu Jul 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so that we can implement what you currently do using a runtime if by instead using compiletest's revisions.

Unfortunately, revisions are only supported by a subset of test suites (ui, codegen, and maybe a couple more), it is not supported in run-make tests. However, we could try to pass info from compiletest about whether zstd/zlib is available in the environment (or I'm not sure if it's possible, prevent a sub test case from finding zstd/zlib in the environment).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, I keep forgetting the arbitrary limitations compiletest has, my mistake.


fn check_compression(compression: &str, to_find: &str) {
run_in_tmpdir(|| {
let out = rustc()
.arg("-Zlinker-features=+lld")
.arg("-Clink-self-contained=+linker")
.arg("-Zunstable-options")
.arg("-Cdebuginfo=full")
.link_arg(&format!("-Wl,--compress-debug-sections={compression}"))
.input("main.rs")
.run_unchecked();
let stderr = out.stderr_utf8();
if stderr.is_empty() {
llvm_readobj().arg("-t").arg("main").run().assert_stdout_contains(to_find);
jieyouxu marked this conversation as resolved.
Show resolved Hide resolved
} else {
assert_contains(
stderr,
format!(
"LLVM was not built with LLVM_ENABLE_{to_find} \
or did not find {compression} at build time"
),
);
}
});
}

fn main() {
check_compression("zlib", "ZLIB");
check_compression("zstd", "ZSTD");
}
Loading