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.

12 files changed, 182 insertions, 29 deletions

diff --git a/src/bootstrap/native.rs b/src/bootstrap/native.rs
index da2e03a1a08..bf3601cb312 100644
--- a/src/bootstrap/native.rs
+++ b/src/bootstrap/native.rs
@@ -203,8 +203,16 @@ impl Step for Llvm {
             cfg.define("LLVM_BUILD_32_BITS", "ON");
         }
 
+        let mut enabled_llvm_projects = Vec::new();
+
+        if util::forcing_clang_based_tests() {
+            enabled_llvm_projects.push("clang");
+            enabled_llvm_projects.push("compiler-rt");
+        }
+
         if want_lldb {
-            cfg.define("LLVM_ENABLE_PROJECTS", "clang;lldb");
+            enabled_llvm_projects.push("clang");
+            enabled_llvm_projects.push("lldb");
             // For the time being, disable code signing.
             cfg.define("LLDB_CODESIGN_IDENTITY", "");
             cfg.define("LLDB_NO_DEBUGSERVER", "ON");
@@ -214,6 +222,12 @@ impl Step for Llvm {
             cfg.define("LLVM_ENABLE_LIBXML2", "OFF");
         }
 
+        if enabled_llvm_projects.len() > 0 {
+            enabled_llvm_projects.sort();
+            enabled_llvm_projects.dedup();
+            cfg.define("LLVM_ENABLE_PROJECTS", enabled_llvm_projects.join(";"));
+        }
+
         if let Some(num_linkers) = builder.config.llvm_link_jobs {
             if num_linkers > 0 {
                 cfg.define("LLVM_PARALLEL_LINK_JOBS", num_linkers.to_string());
diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs
index 05b3ce6bc89..9d0aa09f15c 100644
--- a/src/bootstrap/test.rs
+++ b/src/bootstrap/test.rs
@@ -1143,24 +1143,9 @@ impl Step for Compiletest {
             }
         }
 
-        if let Some(var) = env::var_os("RUSTBUILD_FORCE_CLANG_BASED_TESTS") {
-            match &var.to_string_lossy().to_lowercase()[..] {
-                "1" | "yes" | "on" => {
-                    assert!(builder.config.lldb_enabled,
-                        "RUSTBUILD_FORCE_CLANG_BASED_TESTS needs Clang/LLDB to \
-                         be built.");
-                    let clang_exe = builder.llvm_out(target).join("bin").join("clang");
-                    cmd.arg("--run-clang-based-tests-with").arg(clang_exe);
-                }
-                "0" | "no" | "off" => {
-                    // Nothing to do.
-                }
-                other => {
-                    // Let's make sure typos don't get unnoticed
-                    panic!("Unrecognized option '{}' set in \
-                            RUSTBUILD_FORCE_CLANG_BASED_TESTS", other);
-                }
-            }
+        if util::forcing_clang_based_tests() {
+            let clang_exe = builder.llvm_out(target).join("bin").join("clang");
+            cmd.arg("--run-clang-based-tests-with").arg(clang_exe);
         }
 
         // Get paths from cmd args
diff --git a/src/bootstrap/util.rs b/src/bootstrap/util.rs
index f22f0559265..9f684678bb0 100644
--- a/src/bootstrap/util.rs
+++ b/src/bootstrap/util.rs
@@ -356,3 +356,19 @@ impl CiEnv {
         }
     }
 }
+
+pub fn forcing_clang_based_tests() -> bool {
+    if let Some(var) = env::var_os("RUSTBUILD_FORCE_CLANG_BASED_TESTS") {
+        match &var.to_string_lossy().to_lowercase()[..] {
+            "1" | "yes" | "on" => true,
+            "0" | "no" | "off" => false,
+            other => {
+                // Let's make sure typos don't go unnoticed
+                panic!("Unrecognized option '{}' set in \
+                        RUSTBUILD_FORCE_CLANG_BASED_TESTS", other)
+            }
+        }
+    } else {
+        false
+    }
+}
diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs
index 49cd3eff21a..44b6e036557 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",
@@ -3211,7 +3211,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();
diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs
index 4d47491661e..3d8092f6e00 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
diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs
index 66ba95810a6..1eee9ab8c0b 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 1c793996c83..74c41969268 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,
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 00000000000..59a7d61892f
--- /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 00000000000..90f33f24dc4
--- /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 00000000000..e3f24828be3
--- /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 00000000000..8129dcb85d9
--- /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 00000000000..8a74d74a420
--- /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
+}