Auto merge of #131532 - Zalathar:rollup-688kw5t, r=Zalathar

Rollup of 3 pull requests

Successful merges:

 - #131305 (make `llvm::is_ci_llvm_modified` logic more precise)
 - #131524 (compiletest: Remove the magic hacks for finding output with `lto=thin`)
 - #131525 (compiletest: Simplify the choice of `--emit` mode for assembly tests)

r? `@ghost`
`@rustbot` modify labels: rollup

src/bootstrap/src/core/build_steps/llvm.rs

@@ -20,10 +20,9 @@ use build_helper::git::get_closest_merge_commit;
 
 use crate::core::builder::{Builder, RunConfig, ShouldRun, Step};
 use crate::core::config::{Config, TargetSelection};
-use crate::utils::channel;
 use crate::utils::exec::command;
 use crate::utils::helpers::{
-    self, HashStamp, exe, get_clang_cl_resource_dir, output, t, unhashed_basename, up_to_date,
+    self, HashStamp, exe, get_clang_cl_resource_dir, t, unhashed_basename, up_to_date,
 };
 use crate::{CLang, GitRepo, Kind, generate_smart_stamp_hash};
 
@@ -166,7 +165,7 @@ pub(crate) fn detect_llvm_sha(config: &Config, is_git: bool) -> String {
             config.src.join("src/version"),
         ])
         .unwrap()
-    } else if let Some(info) = channel::read_commit_info_file(&config.src) {
+    } else if let Some(info) = crate::utils::channel::read_commit_info_file(&config.src) {
         info.sha.trim().to_owned()
     } else {
         "".to_owned()
@@ -242,15 +241,29 @@ pub(crate) fn is_ci_llvm_available(config: &Config, asserts: bool) -> bool {
 
 /// Returns true if we're running in CI with modified LLVM (and thus can't download it)
 pub(crate) fn is_ci_llvm_modified(config: &Config) -> bool {
-    CiEnv::is_rust_lang_managed_ci_job() && config.rust_info.is_managed_git_subrepository() && {
-        // We assume we have access to git, so it's okay to unconditionally pass
-        // `true` here.
-        let llvm_sha = detect_llvm_sha(config, true);
-        let head_sha =
-            output(helpers::git(Some(&config.src)).arg("rev-parse").arg("HEAD").as_command_mut());
-        let head_sha = head_sha.trim();
-        llvm_sha == head_sha
+    // If not running in a CI environment, return false.
+    if !CiEnv::is_ci() {
+        return false;
+    }
+
+    // In rust-lang/rust managed CI, assert the existence of the LLVM submodule.
+    if CiEnv::is_rust_lang_managed_ci_job() {
+        assert!(
+            config.in_tree_llvm_info.is_managed_git_subrepository(),
+            "LLVM submodule must be fetched in rust-lang/rust managed CI builders."
+        );
     }
+    // If LLVM submodule isn't present, skip the change check as it won't work.
+    else if !config.in_tree_llvm_info.is_managed_git_subrepository() {
+        return false;
+    }
+
+    let llvm_sha = detect_llvm_sha(config, true);
+    let head_sha = crate::output(
+        helpers::git(Some(&config.src)).arg("rev-parse").arg("HEAD").as_command_mut(),
+    );
+    let head_sha = head_sha.trim();
+    llvm_sha == head_sha
 }
 
 #[derive(Debug, Clone, Hash, PartialEq, Eq)]

src/bootstrap/src/core/config/tests.rs

@@ -21,29 +21,21 @@ pub(crate) fn parse(config: &str) -> Config {
 
 #[test]
 fn download_ci_llvm() {
-    if crate::core::build_steps::llvm::is_ci_llvm_modified(&parse("")) {
-        eprintln!("Detected LLVM as non-available: running in CI and modified LLVM in this change");
-        return;
+    assert!(parse("").llvm_from_ci);
+    assert!(parse("llvm.download-ci-llvm = true").llvm_from_ci);
+    assert!(!parse("llvm.download-ci-llvm = false").llvm_from_ci);
+
+    let if_unchanged_config = parse("llvm.download-ci-llvm = \"if-unchanged\"");
+    if if_unchanged_config.llvm_from_ci {
+        let has_changes = if_unchanged_config
+            .last_modified_commit(&["src/llvm-project"], "download-ci-llvm", true)
+            .is_none();
+
+        assert!(
+            !has_changes,
+            "CI LLVM can't be enabled with 'if-unchanged' while there are changes in LLVM submodule."
+        );
     }
-
-    let parse_llvm = |s| parse(s).llvm_from_ci;
-    let if_unchanged = parse_llvm("llvm.download-ci-llvm = \"if-unchanged\"");
-
-    assert!(parse_llvm("llvm.download-ci-llvm = true"));
-    assert!(!parse_llvm("llvm.download-ci-llvm = false"));
-    assert_eq!(parse_llvm(""), if_unchanged);
-    assert_eq!(parse_llvm("rust.channel = \"dev\""), if_unchanged);
-    assert!(parse_llvm("rust.channel = \"stable\""));
-    assert_eq!(parse_llvm("build.build = \"x86_64-unknown-linux-gnu\""), if_unchanged);
-    assert_eq!(
-        parse_llvm(
-            "llvm.assertions = true \r\n build.build = \"x86_64-unknown-linux-gnu\" \r\n llvm.download-ci-llvm = \"if-unchanged\""
-        ),
-        if_unchanged
-    );
-    assert!(!parse_llvm(
-        "llvm.assertions = true \r\n build.build = \"aarch64-apple-darwin\" \r\n llvm.download-ci-llvm = \"if-unchanged\""
-    ));
 }
 
 // FIXME(onur-ozkan): extend scope of the test

src/tools/compiletest/src/runtest.rs

@@ -1780,102 +1780,49 @@ impl<'test> TestCx<'test> {
         proc_res.fatal(None, || on_failure(*self));
     }
 
-    fn get_output_file(&self, extension: &str) -> TargetLocation {
-        let thin_lto = self.props.compile_flags.iter().any(|s| s.ends_with("lto=thin"));
-        if thin_lto {
-            TargetLocation::ThisDirectory(self.output_base_dir())
-        } else {
-            // This works with both `--emit asm` (as default output name for the assembly)
-            // and `ptx-linker` because the latter can write output at requested location.
-            let output_path = self.output_base_name().with_extension(extension);
-
-            TargetLocation::ThisFile(output_path.clone())
-        }
-    }
-
-    fn get_filecheck_file(&self, extension: &str) -> PathBuf {
-        let thin_lto = self.props.compile_flags.iter().any(|s| s.ends_with("lto=thin"));
-        if thin_lto {
-            let name = self.testpaths.file.file_stem().unwrap().to_str().unwrap();
-            let canonical_name = name.replace('-', "_");
-            let mut output_file = None;
-            for entry in self.output_base_dir().read_dir().unwrap() {
-                if let Ok(entry) = entry {
-                    let entry_path = entry.path();
-                    let entry_file = entry_path.file_name().unwrap().to_str().unwrap();
-                    if entry_file.starts_with(&format!("{}.{}", name, canonical_name))
-                        && entry_file.ends_with(extension)
-                    {
-                        assert!(
-                            output_file.is_none(),
-                            "thinlto doesn't support multiple cgu tests"
-                        );
-                        output_file = Some(entry_file.to_string());
-                    }
-                }
-            }
-            if let Some(output_file) = output_file {
-                self.output_base_dir().join(output_file)
-            } else {
-                self.output_base_name().with_extension(extension)
-            }
-        } else {
-            self.output_base_name().with_extension(extension)
-        }
-    }
-
     // codegen tests (using FileCheck)
 
     fn compile_test_and_save_ir(&self) -> (ProcRes, PathBuf) {
-        let output_file = self.get_output_file("ll");
+        let output_path = self.output_base_name().with_extension("ll");
         let input_file = &self.testpaths.file;
         let rustc = self.make_compile_args(
             input_file,
-            output_file,
+            TargetLocation::ThisFile(output_path.clone()),
             Emit::LlvmIr,
             AllowUnused::No,
             LinkToAux::Yes,
             Vec::new(),
         );
 
         let proc_res = self.compose_and_run_compiler(rustc, None, self.testpaths);
-        let output_path = self.get_filecheck_file("ll");
         (proc_res, output_path)
     }
 
     fn compile_test_and_save_assembly(&self) -> (ProcRes, PathBuf) {
-        let output_file = self.get_output_file("s");
+        // This works with both `--emit asm` (as default output name for the assembly)
+        // and `ptx-linker` because the latter can write output at requested location.
+        let output_path = self.output_base_name().with_extension("s");
         let input_file = &self.testpaths.file;
 
-        let mut emit = Emit::None;
-        match self.props.assembly_output.as_ref().map(AsRef::as_ref) {
-            Some("emit-asm") => {
-                emit = Emit::Asm;
-            }
-
-            Some("bpf-linker") => {
-                emit = Emit::LinkArgsAsm;
-            }
-
-            Some("ptx-linker") => {
-                // No extra flags needed.
-            }
-
-            Some(header) => self.fatal(&format!("unknown 'assembly-output' header: {header}")),
-            None => self.fatal("missing 'assembly-output' header"),
-        }
+        // Use the `//@ assembly-output:` directive to determine how to emit assembly.
+        let emit = match self.props.assembly_output.as_deref() {
+            Some("emit-asm") => Emit::Asm,
+            Some("bpf-linker") => Emit::LinkArgsAsm,
+            Some("ptx-linker") => Emit::None, // No extra flags needed.
+            Some(other) => self.fatal(&format!("unknown 'assembly-output' directive: {other}")),
+            None => self.fatal("missing 'assembly-output' directive"),
+        };
 
         let rustc = self.make_compile_args(
             input_file,
-            output_file,
+            TargetLocation::ThisFile(output_path.clone()),
             emit,
             AllowUnused::No,
             LinkToAux::Yes,
             Vec::new(),
         );
 
         let proc_res = self.compose_and_run_compiler(rustc, None, self.testpaths);
-        let output_path = self.get_filecheck_file("s");
         (proc_res, output_path)
     }