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.

Author: bors

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());

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

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
+    }
+}

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();

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

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,

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,

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

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;
+}

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;
+}

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());
+    }
+}

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
+}