Skip to content
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.

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

Check for asm support in UI tests that require it #84099

Merged
merged 1 commit into from
Apr 13, 2021

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Apr 11, 2021

Add needs-asm-support compiletest directive, and use it in asm tests
that require asm support without relying on any architecture specific
features.

Closes #84038.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 11, 2021
@Mark-Simulacrum
Copy link
Member

Hm, I wonder if we should avoid limiting to just x86_64 - regardless, I would like to see the comment annotated with why we're requiring that target/platform.

r? @Amanieu

@tmiasko
Copy link
Contributor Author

tmiasko commented Apr 11, 2021

We could add needs-asm-support directive in compiletest.

@Amanieu
Copy link
Member

Amanieu commented Apr 11, 2021

All the other asm tests do this. The main reason is that, although the specific functionality that is being tested is not arch-specific, we still need to pick an architecture that supports asm!.

For simplicity we only run the tests on x86_64 to ensure they are tested by CI. The arch-specific parts of asm! are tested in src/test/assembly/asm which uses #![no_core] to ensure the tests are always run no matter the host arch.

@Mark-Simulacrum
Copy link
Member

Makes sense, OK. I think comments or using something like needs-asm-support would be good, but not necessarily blockers.

@tmiasko tmiasko force-pushed the asm-only-x86_64 branch 2 times, most recently from 6fd4c7d to 5e87f97 Compare April 11, 2021 17:13
@Amanieu
Copy link
Member

Amanieu commented Apr 12, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Apr 12, 2021

📌 Commit 5e87f97 has been approved by Amanieu

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 12, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 12, 2021
Check for asm support in UI tests that require it

Add `needs-asm-support` compiletest directive, and use it in asm tests
that require asm support without relying on any architecture specific
features.

Closes rust-lang#84038.
@Dylan-DPC-zz
Copy link

failed in rollup

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 12, 2021
Add `needs-asm-support` compiletest directive, and use it in asm tests
that require asm support without relying on any architecture specific
features.
@Amanieu
Copy link
Member

Amanieu commented Apr 13, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Apr 13, 2021

📌 Commit da40e69 has been approved by Amanieu

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 13, 2021
@bors
Copy link
Contributor

bors commented Apr 13, 2021

⌛ Testing commit da40e69 with merge 2e7eb85...

@bors
Copy link
Contributor

bors commented Apr 13, 2021

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing 2e7eb85 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 13, 2021
@bors bors merged commit 2e7eb85 into rust-lang:master Apr 13, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 13, 2021
@tmiasko tmiasko deleted the asm-only-x86_64 branch April 13, 2021 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1.50 added asm tests without architecture gates for those that don't support asm
7 participants