Skip to content

Commit 87ed0b4

Browse files
borsbors
committed
Auto merge of #61036 - michaelwoerister:pgo-xlto-test, r=alexcrichton
PGO - Add a smoketest for combining PGO with cross-language LTO. This PR - Adds a test making sure that PGO can be combined with cross-language LTO. - Does a little cleanup on how the `pgo-use` flag is handled internally. - Makes the compiler error if the `pgo-use` file given to `rustc` doesn't actually exist. LLVM only gives a warning and then just doesn't do PGO. Clang, on the other hand, does give an error in this case. - Makes the build system also build `compiler-rt` when building LLDB. This way the Clang compiler that we get from building LLDB can perform PGO, which is something that the new test case wants to do. CI compile times shouldn't be affected too much.
2 parents b711179 + 577ea53 commit 87ed0b4

File tree

12 files changed

+182
-29
lines changed

12 files changed

+182
-29
lines changed

‎src/bootstrap/native.rs

+15-1
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,16 @@ impl Step for Llvm {
203203
cfg.define("LLVM_BUILD_32_BITS", "ON");
204204
}
205205

206+
let mut enabled_llvm_projects = Vec::new();
207+
208+
if util::forcing_clang_based_tests() {
209+
enabled_llvm_projects.push("clang");
210+
enabled_llvm_projects.push("compiler-rt");
211+
}
212+
206213
if want_lldb {
207-
cfg.define("LLVM_ENABLE_PROJECTS", "clang;lldb");
214+
enabled_llvm_projects.push("clang");
215+
enabled_llvm_projects.push("lldb");
208216
// For the time being, disable code signing.
209217
cfg.define("LLDB_CODESIGN_IDENTITY", "");
210218
cfg.define("LLDB_NO_DEBUGSERVER", "ON");
@@ -214,6 +222,12 @@ impl Step for Llvm {
214222
cfg.define("LLVM_ENABLE_LIBXML2", "OFF");
215223
}
216224

225+
if enabled_llvm_projects.len() > 0 {
226+
enabled_llvm_projects.sort();
227+
enabled_llvm_projects.dedup();
228+
cfg.define("LLVM_ENABLE_PROJECTS", enabled_llvm_projects.join(";"));
229+
}
230+
217231
if let Some(num_linkers) = builder.config.llvm_link_jobs {
218232
if num_linkers > 0 {
219233
cfg.define("LLVM_PARALLEL_LINK_JOBS", num_linkers.to_string());

‎src/bootstrap/test.rs

+3-18
Original file line numberDiff line numberDiff line change
@@ -1143,24 +1143,9 @@ impl Step for Compiletest {
11431143
}
11441144
}
11451145

1146-
if let Some(var) = env::var_os("RUSTBUILD_FORCE_CLANG_BASED_TESTS") {
1147-
match &var.to_string_lossy().to_lowercase()[..] {
1148-
"1" | "yes" | "on" => {
1149-
assert!(builder.config.lldb_enabled,
1150-
"RUSTBUILD_FORCE_CLANG_BASED_TESTS needs Clang/LLDB to \
1151-
be built.");
1152-
let clang_exe = builder.llvm_out(target).join("bin").join("clang");
1153-
cmd.arg("--run-clang-based-tests-with").arg(clang_exe);
1154-
}
1155-
"0" | "no" | "off" => {
1156-
// Nothing to do.
1157-
}
1158-
other => {
1159-
// Let's make sure typos don't get unnoticed
1160-
panic!("Unrecognized option '{}' set in \
1161-
RUSTBUILD_FORCE_CLANG_BASED_TESTS", other);
1162-
}
1163-
}
1146+
if util::forcing_clang_based_tests() {
1147+
let clang_exe = builder.llvm_out(target).join("bin").join("clang");
1148+
cmd.arg("--run-clang-based-tests-with").arg(clang_exe);
11641149
}
11651150

11661151
// Get paths from cmd args

‎src/bootstrap/util.rs

+16
Original file line numberDiff line numberDiff line change
@@ -356,3 +356,19 @@ impl CiEnv {
356356
}
357357
}
358358
}
359+
360+
pub fn forcing_clang_based_tests() -> bool {
361+
if let Some(var) = env::var_os("RUSTBUILD_FORCE_CLANG_BASED_TESTS") {
362+
match &var.to_string_lossy().to_lowercase()[..] {
363+
"1" | "yes" | "on" => true,
364+
"0" | "no" | "off" => false,
365+
other => {
366+
// Let's make sure typos don't go unnoticed
367+
panic!("Unrecognized option '{}' set in \
368+
RUSTBUILD_FORCE_CLANG_BASED_TESTS", other)
369+
}
370+
}
371+
} else {
372+
false
373+
}
374+
}

‎src/librustc/session/config.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1381,7 +1381,7 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
13811381
"insert profiling code"),
13821382
pgo_gen: PgoGenerate = (PgoGenerate::Disabled, parse_pgo_generate, [TRACKED],
13831383
"Generate PGO profile data, to a given file, or to the default location if it's empty."),
1384-
pgo_use: String = (String::new(), parse_string, [TRACKED],
1384+
pgo_use: Option<PathBuf> = (None, parse_opt_pathbuf, [TRACKED],
13851385
"Use PGO profile data from the given profile file."),
13861386
disable_instrumentation_preinliner: bool = (false, parse_bool, [TRACKED],
13871387
"Disable the instrumentation pre-inliner, useful for profiling / PGO."),
@@ -2021,7 +2021,7 @@ pub fn build_session_options_and_crate_config(
20212021
}
20222022
}
20232023

2024-
if debugging_opts.pgo_gen.enabled() && !debugging_opts.pgo_use.is_empty() {
2024+
if debugging_opts.pgo_gen.enabled() && debugging_opts.pgo_use.is_some() {
20252025
early_error(
20262026
error_format,
20272027
"options `-Z pgo-gen` and `-Z pgo-use` are exclusive",
@@ -3211,7 +3211,7 @@ mod tests {
32113211
assert_ne!(reference.dep_tracking_hash(), opts.dep_tracking_hash());
32123212

32133213
opts = reference.clone();
3214-
opts.debugging_opts.pgo_use = String::from("abc");
3214+
opts.debugging_opts.pgo_use = Some(PathBuf::from("abc"));
32153215
assert_ne!(reference.dep_tracking_hash(), opts.dep_tracking_hash());
32163216

32173217
opts = reference.clone();

‎src/librustc/session/mod.rs

+9
Original file line numberDiff line numberDiff line change
@@ -1272,6 +1272,15 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
12721272
sess.err("Linker plugin based LTO is not supported together with \
12731273
`-C prefer-dynamic` when targeting MSVC");
12741274
}
1275+
1276+
// Make sure that any given profiling data actually exists so LLVM can't
1277+
// decide to silently skip PGO.
1278+
if let Some(ref path) = sess.opts.debugging_opts.pgo_use {
1279+
if !path.exists() {
1280+
sess.err(&format!("File `{}` passed to `-Zpgo-use` does not exist.",
1281+
path.display()));
1282+
}
1283+
}
12751284
}
12761285

12771286
/// Hash value constructed out of all the `-C metadata` arguments passed to the

‎src/librustc_codegen_llvm/back/write.rs

+3-5
Original file line numberDiff line numberDiff line change
@@ -721,11 +721,9 @@ pub unsafe fn with_llvm_pmb(llmod: &llvm::Module,
721721
}
722722
};
723723

724-
let pgo_use_path = if config.pgo_use.is_empty() {
725-
None
726-
} else {
727-
Some(CString::new(config.pgo_use.as_bytes()).unwrap())
728-
};
724+
let pgo_use_path = config.pgo_use.as_ref().map(|path_buf| {
725+
CString::new(path_buf.to_string_lossy().as_bytes()).unwrap()
726+
});
729727

730728
llvm::LLVMRustConfigurePassManagerBuilder(
731729
builder,

‎src/librustc_codegen_ssa/back/write.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ pub struct ModuleConfig {
5757
pub opt_size: Option<config::OptLevel>,
5858

5959
pub pgo_gen: PgoGenerate,
60-
pub pgo_use: String,
60+
pub pgo_use: Option<PathBuf>,
6161

6262
// Flags indicating which outputs to produce.
6363
pub emit_pre_lto_bc: bool,
@@ -95,7 +95,7 @@ impl ModuleConfig {
9595
opt_size: None,
9696

9797
pgo_gen: PgoGenerate::Disabled,
98-
pgo_use: String::new(),
98+
pgo_use: None,
9999

100100
emit_no_opt_bc: false,
101101
emit_pre_lto_bc: false,

‎src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/Makefile

+87
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
# needs-matching-clang
2+
3+
# This test makes sure that cross-language inlining can be used in conjunction
4+
# with profile-guided optimization. The test only tests that the whole workflow
5+
# can be executed without anything crashing. It does not test whether PGO or
6+
# xLTO have any specific effect on the generated code.
7+
8+
-include ../tools.mk
9+
10+
COMMON_FLAGS=-Copt-level=3 -Ccodegen-units=1
11+
12+
# LLVM doesn't support instrumenting binaries that use SEH:
13+
# https://bugs.llvm.org/show_bug.cgi?id=41279
14+
#
15+
# Things work fine with -Cpanic=abort though.
16+
ifdef IS_MSVC
17+
COMMON_FLAGS+= -Cpanic=abort
18+
endif
19+
20+
all: cpp-executable rust-executable
21+
22+
cpp-executable:
23+
$(RUSTC) -Clinker-plugin-lto=on \
24+
-Zpgo-gen="$(TMPDIR)"/cpp-profdata \
25+
-o "$(TMPDIR)"/librustlib-xlto.a \
26+
$(COMMON_FLAGS) \
27+
./rustlib.rs
28+
$(CLANG) -flto=thin \
29+
-fprofile-generate="$(TMPDIR)"/cpp-profdata \
30+
-fuse-ld=lld \
31+
-L "$(TMPDIR)" \
32+
-lrustlib-xlto \
33+
-o "$(TMPDIR)"/cmain \
34+
-O3 \
35+
./cmain.c
36+
$(TMPDIR)/cmain
37+
# Postprocess the profiling data so it can be used by the compiler
38+
"$(LLVM_BIN_DIR)"/llvm-profdata merge \
39+
-o "$(TMPDIR)"/cpp-profdata/merged.profdata \
40+
"$(TMPDIR)"/cpp-profdata/default_*.profraw
41+
$(RUSTC) -Clinker-plugin-lto=on \
42+
-Zpgo-use="$(TMPDIR)"/cpp-profdata/merged.profdata \
43+
-o "$(TMPDIR)"/librustlib-xlto.a \
44+
$(COMMON_FLAGS) \
45+
./rustlib.rs
46+
$(CLANG) -flto=thin \
47+
-fprofile-use="$(TMPDIR)"/cpp-profdata/merged.profdata \
48+
-fuse-ld=lld \
49+
-L "$(TMPDIR)" \
50+
-lrustlib-xlto \
51+
-o "$(TMPDIR)"/cmain \
52+
-O3 \
53+
./cmain.c
54+
55+
rust-executable:
56+
exit
57+
$(CLANG) ./clib.c -fprofile-generate="$(TMPDIR)"/rs-profdata -flto=thin -c -o $(TMPDIR)/clib.o -O3
58+
(cd $(TMPDIR); $(AR) crus ./libxyz.a ./clib.o)
59+
$(RUSTC) -Clinker-plugin-lto=on \
60+
-Zpgo-gen="$(TMPDIR)"/rs-profdata \
61+
-L$(TMPDIR) \
62+
$(COMMON_FLAGS) \
63+
-Clinker=$(CLANG) \
64+
-Clink-arg=-fuse-ld=lld \
65+
-o $(TMPDIR)/rsmain \
66+
./main.rs
67+
$(TMPDIR)/rsmain
68+
# Postprocess the profiling data so it can be used by the compiler
69+
"$(LLVM_BIN_DIR)"/llvm-profdata merge \
70+
-o "$(TMPDIR)"/rs-profdata/merged.profdata \
71+
"$(TMPDIR)"/rs-profdata/default_*.profraw
72+
$(CLANG) ./clib.c \
73+
-fprofile-use="$(TMPDIR)"/rs-profdata/merged.profdata \
74+
-flto=thin \
75+
-c \
76+
-o $(TMPDIR)/clib.o \
77+
-O3
78+
rm "$(TMPDIR)"/libxyz.a
79+
(cd $(TMPDIR); $(AR) crus ./libxyz.a ./clib.o)
80+
$(RUSTC) -Clinker-plugin-lto=on \
81+
-Zpgo-use="$(TMPDIR)"/rs-profdata/merged.profdata \
82+
-L$(TMPDIR) \
83+
$(COMMON_FLAGS) \
84+
-Clinker=$(CLANG) \
85+
-Clink-arg=-fuse-ld=lld \
86+
-o $(TMPDIR)/rsmain \
87+
./main.rs

‎src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/clib.c

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#include <stdint.h>
2+
3+
uint32_t c_always_inlined() {
4+
return 1234;
5+
}
6+
7+
__attribute__((noinline)) uint32_t c_never_inlined() {
8+
return 12345;
9+
}

‎src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/cmain.c

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#include <stdint.h>
2+
3+
// A trivial function defined in Rust, returning a constant value. This should
4+
// always be inlined.
5+
uint32_t rust_always_inlined();
6+
7+
8+
uint32_t rust_never_inlined();
9+
10+
int main(int argc, char** argv) {
11+
return (rust_never_inlined() + rust_always_inlined()) * 0;
12+
}

‎src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/main.rs

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#[link(name = "xyz")]
2+
extern "C" {
3+
fn c_always_inlined() -> u32;
4+
fn c_never_inlined() -> u32;
5+
}
6+
7+
fn main() {
8+
unsafe {
9+
println!("blub: {}", c_always_inlined() + c_never_inlined());
10+
}
11+
}

‎src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/rustlib.rs

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#![crate_type="staticlib"]
2+
3+
#[no_mangle]
4+
pub extern "C" fn rust_always_inlined() -> u32 {
5+
42
6+
}
7+
8+
#[no_mangle]
9+
#[inline(never)]
10+
pub extern "C" fn rust_never_inlined() -> u32 {
11+
421
12+
}

0 commit comments

Comments
 (0)