From e349f8fe528a781842a138bb0fe7e380bcb0de1f Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Wed, 22 May 2019 12:58:56 +0200 Subject: [PATCH 1/9] 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 = (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, pub pgo_gen: PgoGenerate, - pub pgo_use: String, + pub pgo_use: Option, // 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 Date: Wed, 22 May 2019 13:00:09 +0200 Subject: [PATCH 2/9] 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 Date: Wed, 22 May 2019 13:01:12 +0200 Subject: [PATCH 3/9] 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 Date: Mon, 20 May 2019 11:43:38 +0200 Subject: [PATCH 4/9] 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 + +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 + +// 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 Date: Wed, 22 May 2019 15:41:56 +0200 Subject: [PATCH 5/9] 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(); From 1a35a1c688511bb09a67c1430d55e022ac5f88eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Miku=C5=82a?= Date: Thu, 23 May 2019 13:32:30 +0200 Subject: [PATCH 6/9] Ship profiler with windows-gnu --- appveyor.yml | 4 ++-- src/libprofiler_builtins/build.rs | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index dffd79c56e48a..7c5c779584109 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -104,7 +104,7 @@ environment: DEPLOY: 1 - CI_JOB_NAME: dist-i686-mingw MSYS_BITS: 32 - RUST_CONFIGURE_ARGS: --build=i686-pc-windows-gnu --enable-full-tools + RUST_CONFIGURE_ARGS: --build=i686-pc-windows-gnu --enable-full-tools --enable-profiler SCRIPT: python x.py dist MINGW_URL: https://s3-us-west-1.amazonaws.com/rust-lang-ci2/rust-ci-mirror MINGW_ARCHIVE: i686-6.3.0-release-posix-dwarf-rt_v5-rev2.7z @@ -114,7 +114,7 @@ environment: - CI_JOB_NAME: dist-x86_64-mingw MSYS_BITS: 64 SCRIPT: python x.py dist - RUST_CONFIGURE_ARGS: --build=x86_64-pc-windows-gnu --enable-full-tools + RUST_CONFIGURE_ARGS: --build=x86_64-pc-windows-gnu --enable-full-tools --enable-profiler MINGW_URL: https://s3-us-west-1.amazonaws.com/rust-lang-ci2/rust-ci-mirror MINGW_ARCHIVE: x86_64-6.3.0-release-posix-seh-rt_v5-rev2.7z MINGW_DIR: mingw64 diff --git a/src/libprofiler_builtins/build.rs b/src/libprofiler_builtins/build.rs index 491986480deba..0b2bda577d75f 100644 --- a/src/libprofiler_builtins/build.rs +++ b/src/libprofiler_builtins/build.rs @@ -41,7 +41,11 @@ fn main() { cfg.flag("-fomit-frame-pointer"); cfg.flag("-ffreestanding"); cfg.define("VISIBILITY_HIDDEN", None); - cfg.define("COMPILER_RT_HAS_UNAME", Some("1")); + if !target.contains("windows") { + cfg.define("COMPILER_RT_HAS_UNAME", Some("1")); + } else { + profile_sources.push("WindowsMMap.c"); + } } // Assume that the Unixes we are building this for have fnctl() available From e396f992558746689e1d562c5a6bb0b92bf70b93 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 23 May 2019 17:47:53 +1000 Subject: [PATCH 7/9] Don't arena-allocate static symbols. It's just a waste of memory. This also gets rid of the special case for "". --- src/libsyntax_pos/symbol.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index b1e1a056db4ad..26422e891c536 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -866,20 +866,13 @@ pub struct Interner { } impl Interner { - fn prefill(init: &[&str]) -> Self { - let mut this = Interner::default(); - this.names.reserve(init.len()); - this.strings.reserve(init.len()); - - // We can't allocate empty strings in the arena, so handle this here. - assert!(kw::Invalid.as_u32() == 0 && init[0].is_empty()); - this.names.insert("", kw::Invalid); - this.strings.push(""); - - for string in &init[1..] { - this.intern(string); + fn prefill(init: &[&'static str]) -> Self { + let symbols = (0 .. init.len() as u32).map(Symbol::new); + Interner { + strings: init.to_vec(), + names: init.iter().copied().zip(symbols).collect(), + ..Default::default() } - this } pub fn intern(&mut self, string: &str) -> Symbol { From fd1998914dcd2822a11367d3761d8574cb4eb634 Mon Sep 17 00:00:00 2001 From: vishalsodani Date: Sat, 25 May 2019 02:48:01 +0000 Subject: [PATCH 8/9] Fix spelling in release notes --- RELEASES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASES.md b/RELEASES.md index 4185961187b39..91e3c5f721952 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -157,7 +157,7 @@ Libraries produce a warning if their returning type is unused. - [The methods `checked_pow`, `saturating_pow`, `wrapping_pow`, and `overflowing_pow` are now available for all numeric types.][57873] These are - equivalvent to methods such as `wrapping_add` for the `pow` operation. + equivalent to methods such as `wrapping_add` for the `pow` operation. Stabilized APIs From 72145ea2fe18cc2eb2e61c89f5adfe9c1c6c7cb7 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 25 May 2019 09:11:20 +0200 Subject: [PATCH 9/9] MaybeUninit doctest: remove unnecessary type ascription --- src/libcore/mem.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 56869f38a4f6b..ce4aee7ebc54f 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -982,7 +982,7 @@ impl DerefMut for ManuallyDrop { /// out.write(vec![1, 2, 3]); /// } /// -/// let mut v: MaybeUninit> = MaybeUninit::uninit(); +/// let mut v = MaybeUninit::uninit(); /// unsafe { make_vec(v.as_mut_ptr()); } /// // Now we know `v` is initialized! This also makes sure the vector gets /// // properly dropped. @@ -1071,7 +1071,7 @@ impl DerefMut for ManuallyDrop { /// optimizations, potentially resulting in a larger size: /// /// ```rust -/// # use std::mem::{MaybeUninit, size_of, align_of}; +/// # use std::mem::{MaybeUninit, size_of}; /// assert_eq!(size_of::>(), 1); /// assert_eq!(size_of::>>(), 2); /// ```