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.

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

Don't test issue-44056.rs on non-AVX hardware #55667

Closed
wants to merge 1 commit into from
Closed

Don't test issue-44056.rs on non-AVX hardware #55667

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
1 change: 1 addition & 0 deletions src/test/ui/issues/issue-44056.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

// compile-pass
Copy link
Contributor

@gnzlbg gnzlbg Nov 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This test is compile-pass, why is it being run ?

As in, we can cross-compile for x86_64 with avx from all x86_64 toolchains, and the tests checks that doing so works AFAICT, but this test shouldn't run.

Copy link
Contributor

Choose a reason for hiding this comment

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

From the logs:

---- [ui] ui/run-pass/issues/issue-44056.rs stdout ----

it does appear that this test is in fact being run.

Copy link
Contributor

Choose a reason for hiding this comment

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

See #55667 (comment). This test was recently moved to compile-pass (but should not have been, as the original issue requires execution).

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, looking at the issue, this test does need execution. For that we would need a way to filter tests by target features available at run-time. Shouldn't be to hard to add a flag to prevent this tests from executing (e.g. using is_x86_feature_detected!("avx") ).

// only-x86_64
// gate-test-avx512_target_feature
Copy link
Member

Choose a reason for hiding this comment

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

gate-test is AFAIK used to indicate that this is a "feature gate test", to check the policy that every feature gate comes with a test making sure stuff is indeed gated.

You want some kind of only- annotation here to run the test only when certain conditions are met.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is the syntax for "only"? I grep through the code and only see // only-$arch and it's not obvious where the docs for this stuff is...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't seem like avx is valid for that: https://rust-lang-nursery.github.io/rustc-guide/tests/adding.html#ignoring-tests :(

Copy link
Member

Choose a reason for hiding this comment

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

only and ignore accept the same flags, maybe you can grep for ignore?

The code for this is at

rust/src/tools/compiletest/src/header.rs

Line 726 in 0117b42

fn parse_cfg_name_directive(&self, line: &str, prefix: &str) -> ParsedNameDirective {
. Maybe you need to add a way to control test execution based on CPU flags.

// no-prefer-dynamic
// compile-flags: -Ctarget-feature=+avx -Clto

Expand Down