Skip to content

Conversation

jchecahi
Copy link

@jchecahi jchecahi commented Oct 6, 2025

The needs-asm-support directive checks whether the host architecture supports inline assembly, not the target architecture. For tests that explicitly specify a target via --target in their compile-flags, this directive is incorrect and unnecessary.

These tests are cross-compiling to specific targets (like x86_64, arm, aarch64, riscv, etc.) that are already known to have stable asm support. The directive was causing these tests to be incorrectly skipped on hosts that don't support asm, even though the target does.

Tests with explicit targets should rely on needs-llvm-components to ensure the appropriate backend is available, rather than checking host asm support.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 6, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 6, 2025

r? @jackh726

rustbot has assigned @jackh726.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@jchecahi jchecahi force-pushed the remove-needs-asm-support-explicit branch from e8663b2 to d2aaa62 Compare October 6, 2025 16:51
The `needs-asm-support` directive checks whether the host architecture
supports inline assembly, not the target architecture. For tests that
explicitly specify a target via `--target` in their compile-flags, this
directive is incorrect and unnecessary.

These tests are cross-compiling to specific targets (like x86_64, arm,
aarch64, riscv, etc.) that are already known to have stable asm support.
The directive was causing these tests to be incorrectly skipped on hosts
that don't support asm, even though the target does.

Tests with explicit targets should rely on `needs-llvm-components` to
ensure the appropriate backend is available, rather than checking host
asm support.
@jchecahi jchecahi force-pushed the remove-needs-asm-support-explicit branch from d2aaa62 to 3217497 Compare October 6, 2025 16:56
@bjorn3
Copy link
Member

bjorn3 commented Oct 6, 2025

Maybe instead of //@ compile-flags: --target ... we could have a //@ target directive that //@ needs-asm-support then checks? There are other directives that check properties of the target architecture that would be incorrect with //@ compile-flags: --target .... Some of which are genuinely necessary for other codegen backends, like //@ needs-unwind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants