Skip to content

Do not run ui test if options specific to LLVM are used when another codegen backend is used #146618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 17, 2025
Merged

Do not run ui test if options specific to LLVM are used when another codegen backend is used #146618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/tools/compiletest/src/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,10 @@ impl CodegenBackend {
Self::Llvm => "llvm",
}
}

pub fn is_llvm(self) -> bool {
matches!(self, Self::Llvm)
}
}

/// Configuration for `compiletest` *per invocation*.
Expand Down
13 changes: 8 additions & 5 deletions src/tools/compiletest/src/directives/needs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ pub(super) fn handle_needs(
},
Need {
name: "needs-enzyme",
condition: config.has_enzyme,
ignore_reason: "ignored when LLVM Enzyme is disabled",
condition: config.has_enzyme && config.default_codegen_backend.is_llvm(),
ignore_reason: "ignored when LLVM Enzyme is disabled or LLVM is not the default codegen backend",
},
Need {
name: "needs-run-enabled",
Expand Down Expand Up @@ -161,8 +161,8 @@ pub(super) fn handle_needs(
},
Need {
name: "needs-llvm-zstd",
condition: cache.llvm_zstd,
ignore_reason: "ignored if LLVM wasn't build with zstd for ELF section compression",
condition: cache.llvm_zstd && config.default_codegen_backend.is_llvm(),
ignore_reason: "ignored if LLVM wasn't build with zstd for ELF section compression or LLVM is not the default codegen backend",
},
Need {
name: "needs-rustc-debug-assertions",
Expand Down Expand Up @@ -279,7 +279,10 @@ pub(super) fn handle_needs(

// Handled elsewhere.
if name == "needs-llvm-components" {
return IgnoreDecision::Continue;
if config.default_codegen_backend.is_llvm() {
return IgnoreDecision::Continue;
}
return IgnoreDecision::Ignore { reason: "LLVM specific test".into() };
Comment on lines +282 to +285
Copy link
Member

@workingjubilee workingjubilee Oct 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not correct. needs-llvm-components is only informational so that the test is elided if the target cannot be built for using the available LLVM components. Many of these are x86 targets that we should be able to test using cg_gcc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In particular, needs-llvm-components is commonly used for gnarly target-specific tests that we set up with no_core so they can run on all targets; we then need to tell compiletest about that so that it knows whether building for this target is possible at all. None of these are backend-specific.

Those are never "run" tests since obviously we can't run random binaries for a different target, so most of codegen is not relevant. However, tests like tests/ui/target-feature/abi-incompatible-target-feature-flag-enable.rs and its brethren are still crucial to run for all codegen backends since a bunch of the target feature logic lives inside the backends.

}

let mut found_valid = false;
Expand Down
Loading