Skip to content

Commit

Permalink
Rollup merge of rust-lang#61036 - michaelwoerister:pgo-xlto-test, r=a…
Browse files Browse the repository at this point in the history
…lexcrichton

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.
  • Loading branch information
Centril committed May 25, 2019
2 parents 4761a82 + b3c5cdd commit 994fc4d
Show file tree
Hide file tree
Showing 10 changed files with 149 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/bootstrap/native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ impl Step for Llvm {
}

if want_lldb {
cfg.define("LLVM_ENABLE_PROJECTS", "clang;lldb");
cfg.define("LLVM_ENABLE_PROJECTS", "clang;lldb;compiler-rt");
// For the time being, disable code signing.
cfg.define("LLDB_CODESIGN_IDENTITY", "");
cfg.define("LLDB_NO_DEBUGSERVER", "ON");
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1381,7 +1381,7 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
"insert profiling code"),
pgo_gen: PgoGenerate = (PgoGenerate::Disabled, parse_pgo_generate, [TRACKED],
"Generate PGO profile data, to a given file, or to the default location if it's empty."),
pgo_use: String = (String::new(), parse_string, [TRACKED],
pgo_use: Option<PathBuf> = (None, parse_opt_pathbuf, [TRACKED],
"Use PGO profile data from the given profile file."),
disable_instrumentation_preinliner: bool = (false, parse_bool, [TRACKED],
"Disable the instrumentation pre-inliner, useful for profiling / PGO."),
Expand Down Expand Up @@ -2021,7 +2021,7 @@ pub fn build_session_options_and_crate_config(
}
}

if debugging_opts.pgo_gen.enabled() && !debugging_opts.pgo_use.is_empty() {
if debugging_opts.pgo_gen.enabled() && debugging_opts.pgo_use.is_some() {
early_error(
error_format,
"options `-Z pgo-gen` and `-Z pgo-use` are exclusive",
Expand Down Expand Up @@ -3212,7 +3212,7 @@ mod tests {
assert_ne!(reference.dep_tracking_hash(), opts.dep_tracking_hash());

opts = reference.clone();
opts.debugging_opts.pgo_use = String::from("abc");
opts.debugging_opts.pgo_use = Some(PathBuf::from("abc"));
assert_ne!(reference.dep_tracking_hash(), opts.dep_tracking_hash());

opts = reference.clone();
Expand Down
9 changes: 9 additions & 0 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1272,6 +1272,15 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
sess.err("Linker plugin based LTO is not supported together with \
`-C prefer-dynamic` when targeting MSVC");
}

// Make sure that any given profiling data actually exists so LLVM can't
// decide to silently skip PGO.
if let Some(ref path) = sess.opts.debugging_opts.pgo_use {
if !path.exists() {
sess.err(&format!("File `{}` passed to `-Zpgo-use` does not exist.",
path.display()));
}
}
}

/// Hash value constructed out of all the `-C metadata` arguments passed to the
Expand Down
8 changes: 3 additions & 5 deletions src/librustc_codegen_llvm/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -721,11 +721,9 @@ pub unsafe fn with_llvm_pmb(llmod: &llvm::Module,
}
};

let pgo_use_path = if config.pgo_use.is_empty() {
None
} else {
Some(CString::new(config.pgo_use.as_bytes()).unwrap())
};
let pgo_use_path = config.pgo_use.as_ref().map(|path_buf| {
CString::new(path_buf.to_string_lossy().as_bytes()).unwrap()
});

llvm::LLVMRustConfigurePassManagerBuilder(
builder,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_codegen_ssa/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub struct ModuleConfig {
pub opt_size: Option<config::OptLevel>,

pub pgo_gen: PgoGenerate,
pub pgo_use: String,
pub pgo_use: Option<PathBuf>,

// Flags indicating which outputs to produce.
pub emit_pre_lto_bc: bool,
Expand Down Expand Up @@ -95,7 +95,7 @@ impl ModuleConfig {
opt_size: None,

pgo_gen: PgoGenerate::Disabled,
pgo_use: String::new(),
pgo_use: None,

emit_no_opt_bc: false,
emit_pre_lto_bc: false,
Expand Down
87 changes: 87 additions & 0 deletions src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# needs-matching-clang

# This test makes sure that cross-language inlining can be used in conjunction
# with profile-guided optimization. The test only tests that the whole workflow
# can be executed without anything crashing. It does not test whether PGO or
# xLTO have any specific effect on the generated code.

-include ../tools.mk

COMMON_FLAGS=-Copt-level=3 -Ccodegen-units=1

# LLVM doesn't support instrumenting binaries that use SEH:
# https://bugs.llvm.org/show_bug.cgi?id=41279
#
# Things work fine with -Cpanic=abort though.
ifdef IS_MSVC
COMMON_FLAGS+= -Cpanic=abort
endif

all: cpp-executable rust-executable

cpp-executable:
$(RUSTC) -Clinker-plugin-lto=on \
-Zpgo-gen="$(TMPDIR)"/cpp-profdata \
-o "$(TMPDIR)"/librustlib-xlto.a \
$(COMMON_FLAGS) \
./rustlib.rs
$(CLANG) -flto=thin \
-fprofile-generate="$(TMPDIR)"/cpp-profdata \
-fuse-ld=lld \
-L "$(TMPDIR)" \
-lrustlib-xlto \
-o "$(TMPDIR)"/cmain \
-O3 \
./cmain.c
$(TMPDIR)/cmain
# Postprocess the profiling data so it can be used by the compiler
"$(LLVM_BIN_DIR)"/llvm-profdata merge \
-o "$(TMPDIR)"/cpp-profdata/merged.profdata \
"$(TMPDIR)"/cpp-profdata/default_*.profraw
$(RUSTC) -Clinker-plugin-lto=on \
-Zpgo-use="$(TMPDIR)"/cpp-profdata/merged.profdata \
-o "$(TMPDIR)"/librustlib-xlto.a \
$(COMMON_FLAGS) \
./rustlib.rs
$(CLANG) -flto=thin \
-fprofile-use="$(TMPDIR)"/cpp-profdata/merged.profdata \
-fuse-ld=lld \
-L "$(TMPDIR)" \
-lrustlib-xlto \
-o "$(TMPDIR)"/cmain \
-O3 \
./cmain.c

rust-executable:
exit
$(CLANG) ./clib.c -fprofile-generate="$(TMPDIR)"/rs-profdata -flto=thin -c -o $(TMPDIR)/clib.o -O3
(cd $(TMPDIR); $(AR) crus ./libxyz.a ./clib.o)
$(RUSTC) -Clinker-plugin-lto=on \
-Zpgo-gen="$(TMPDIR)"/rs-profdata \
-L$(TMPDIR) \
$(COMMON_FLAGS) \
-Clinker=$(CLANG) \
-Clink-arg=-fuse-ld=lld \
-o $(TMPDIR)/rsmain \
./main.rs
$(TMPDIR)/rsmain
# Postprocess the profiling data so it can be used by the compiler
"$(LLVM_BIN_DIR)"/llvm-profdata merge \
-o "$(TMPDIR)"/rs-profdata/merged.profdata \
"$(TMPDIR)"/rs-profdata/default_*.profraw
$(CLANG) ./clib.c \
-fprofile-use="$(TMPDIR)"/rs-profdata/merged.profdata \
-flto=thin \
-c \
-o $(TMPDIR)/clib.o \
-O3
rm "$(TMPDIR)"/libxyz.a
(cd $(TMPDIR); $(AR) crus ./libxyz.a ./clib.o)
$(RUSTC) -Clinker-plugin-lto=on \
-Zpgo-use="$(TMPDIR)"/rs-profdata/merged.profdata \
-L$(TMPDIR) \
$(COMMON_FLAGS) \
-Clinker=$(CLANG) \
-Clink-arg=-fuse-ld=lld \
-o $(TMPDIR)/rsmain \
./main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#include <stdint.h>

uint32_t c_always_inlined() {
return 1234;
}

__attribute__((noinline)) uint32_t c_never_inlined() {
return 12345;
}
12 changes: 12 additions & 0 deletions src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/cmain.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#include <stdint.h>

// A trivial function defined in Rust, returning a constant value. This should
// always be inlined.
uint32_t rust_always_inlined();


uint32_t rust_never_inlined();

int main(int argc, char** argv) {
return (rust_never_inlined() + rust_always_inlined()) * 0;
}
11 changes: 11 additions & 0 deletions src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#[link(name = "xyz")]
extern "C" {
fn c_always_inlined() -> u32;
fn c_never_inlined() -> u32;
}

fn main() {
unsafe {
println!("blub: {}", c_always_inlined() + c_never_inlined());
}
}
12 changes: 12 additions & 0 deletions src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/rustlib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#![crate_type="staticlib"]

#[no_mangle]
pub extern "C" fn rust_always_inlined() -> u32 {
42
}

#[no_mangle]
#[inline(never)]
pub extern "C" fn rust_never_inlined() -> u32 {
421
}

0 comments on commit 994fc4d

Please sign in to comment.