From e349f8fe528a781842a138bb0fe7e380bcb0de1f Mon Sep 17 00:00:00 2001
From: Michael Woerister <michaelwoerister@posteo>
Date: Wed, 22 May 2019 12:58:56 +0200
Subject: [PATCH 1/5] Use a PathBuf instead of String for representing the
pgo-use path internally.
---
src/librustc/session/config.rs | 4 ++--
src/librustc_codegen_llvm/back/write.rs | 8 +++-----
src/librustc_codegen_ssa/back/write.rs | 4 ++--
3 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs
index f16137bd2c27a..b8fbc470ecaee 100644
--- a/src/librustc/session/config.rs
+++ b/src/librustc/session/config.rs
@@ -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."),
@@ -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",
diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs
index 66ba95810a625..1eee9ab8c0b67 100644
--- a/src/librustc_codegen_llvm/back/write.rs
+++ b/src/librustc_codegen_llvm/back/write.rs
@@ -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,
diff --git a/src/librustc_codegen_ssa/back/write.rs b/src/librustc_codegen_ssa/back/write.rs
index 1c793996c83db..74c41969268e9 100644
--- a/src/librustc_codegen_ssa/back/write.rs
+++ b/src/librustc_codegen_ssa/back/write.rs
@@ -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,
@@ -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,
From 01a59a350ce591b150c4e708434b5acb2d0f4da2 Mon Sep 17 00:00:00 2001
From: Michael Woerister <michaelwoerister@posteo>
Date: Wed, 22 May 2019 13:00:09 +0200
Subject: [PATCH 2/5] PGO: Check that pgo-use file actually exists. LLVM seems
to only emit an easy-to-overlook warning otherwise.
---
src/librustc/session/mod.rs | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs
index 4d47491661e86..3d8092f6e0070 100644
--- a/src/librustc/session/mod.rs
+++ b/src/librustc/session/mod.rs
@@ -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
From b1f27fae6b0c6ba0d6f9d19bd4c571109e93d6c6 Mon Sep 17 00:00:00 2001
From: Michael Woerister <michaelwoerister@posteo>
Date: Wed, 22 May 2019 13:01:12 +0200
Subject: [PATCH 3/5] rustbuild: Also build compiler-rt when building LLDB.
This allows clang-based run-make tests to use PGO.
---
src/bootstrap/native.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/bootstrap/native.rs b/src/bootstrap/native.rs
index da2e03a1a0848..b89cd4981a721 100644
--- a/src/bootstrap/native.rs
+++ b/src/bootstrap/native.rs
@@ -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");
From 51463d05283be3596bf8c8a29f83cb739bfd335a Mon Sep 17 00:00:00 2001
From: Michael Woerister <michaelwoerister@posteo>
Date: Mon, 20 May 2019 11:43:38 +0200
Subject: [PATCH 4/5] Add a smoketest for combining PGO with xLTO.
---
.../cross-lang-lto-pgo-smoketest/Makefile | 87 +++++++++++++++++++
.../cross-lang-lto-pgo-smoketest/clib.c | 9 ++
.../cross-lang-lto-pgo-smoketest/cmain.c | 12 +++
.../cross-lang-lto-pgo-smoketest/main.rs | 11 +++
.../cross-lang-lto-pgo-smoketest/rustlib.rs | 12 +++
5 files changed, 131 insertions(+)
create mode 100644 src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/Makefile
create mode 100644 src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/clib.c
create mode 100644 src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/cmain.c
create mode 100644 src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/main.rs
create mode 100644 src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/rustlib.rs
diff --git a/src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/Makefile b/src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/Makefile
new file mode 100644
index 0000000000000..59a7d61892ffb
--- /dev/null
+++ b/src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/Makefile
@@ -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
diff --git a/src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/clib.c b/src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/clib.c
new file mode 100644
index 0000000000000..90f33f24dc424
--- /dev/null
+++ b/src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/clib.c
@@ -0,0 +1,9 @@
+#include <stdint.h>
+
+uint32_t c_always_inlined() {
+ return 1234;
+}
+
+__attribute__((noinline)) uint32_t c_never_inlined() {
+ return 12345;
+}
diff --git a/src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/cmain.c b/src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/cmain.c
new file mode 100644
index 0000000000000..e3f24828be371
--- /dev/null
+++ b/src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/cmain.c
@@ -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;
+}
diff --git a/src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/main.rs b/src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/main.rs
new file mode 100644
index 0000000000000..8129dcb85d96a
--- /dev/null
+++ b/src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/main.rs
@@ -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());
+ }
+}
diff --git a/src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/rustlib.rs b/src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/rustlib.rs
new file mode 100644
index 0000000000000..8a74d74a420bd
--- /dev/null
+++ b/src/test/run-make-fulldeps/cross-lang-lto-pgo-smoketest/rustlib.rs
@@ -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
+}
From b3c5cdd3709169de4389fd1b6c5b85104c8b3b4e Mon Sep 17 00:00:00 2001
From: Michael Woerister <michaelwoerister@posteo>
Date: Wed, 22 May 2019 15:41:56 +0200
Subject: [PATCH 5/5] Fix unit test after pgo-use change.
---
src/librustc/session/config.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs
index b8fbc470ecaee..0ee6effcf5b59 100644
--- a/src/librustc/session/config.rs
+++ b/src/librustc/session/config.rs
@@ -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();