Skip to content

avx512_target_feature is now stable on nightly #1802

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

Merged
merged 3 commits into from
May 20, 2025

Conversation

folkertdev
Copy link
Contributor

and enabling it causes errors on CI

@rustbot
Copy link
Collaborator

rustbot commented May 19, 2025

r? @Amanieu

rustbot has assigned @Amanieu.
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

@Amanieu Amanieu enabled auto-merge May 19, 2025 18:53
auto-merge was automatically disabled May 19, 2025 19:13

Head branch was pushed to by a user without write access

@Amanieu Amanieu enabled auto-merge May 19, 2025 19:16
auto-merge was automatically disabled May 19, 2025 21:13

Head branch was pushed to by a user without write access

@folkertdev folkertdev force-pushed the remove-avx512_target_feature branch from 25d712d to 09592d4 Compare May 19, 2025 21:16
@folkertdev
Copy link
Contributor Author

Just writing up what we found here (yet more reasons to make this a submodule): rust-lang/rust#127013 changed the formatting of f16 values (to something much more helpful) in rust. In C, and f16 is apparently formatted as a hexadecimal value. So we see results like this in the intrinsics tests:

C: result -1: float16x4x2_t(float16x4_t(0x0000, 0x0000, 0x37ff, 0x37ff), float16x4_t(0x0400, 0x0400, 0x3800, 0x3800))
Rust: result -1: float16x4x2_t(float16x4_t(0.0, 0.0, 0.4998, 0.4998), float16x4_t(6.104e-5, 6.104e-5, 0.5, 0.5))

I think the quick fix is to make stdarch have the old formatting behavior, so I implemented some hacks to make that happen. Long-term, hopefully there is a better way to compare the C and rust output?

@folkertdev folkertdev force-pushed the remove-avx512_target_feature branch from 09592d4 to 3bc5019 Compare May 19, 2025 21:25
Comment on lines 142 to 146
// the `intrinsic-test` crate compares the output of C and Rust intrinsics. Currently, It uses
// a string representation of the output value to compare. In C, f16 values are currently printed
// as hexadecimal integers. Since https://github.com/rust-lang/rust/pull/127013, rust does print
// them as decimal floating point values. To keep the intrinsics tests working, for now, format
// vectors containing f16 values like C prints them.
Copy link
Member

Choose a reason for hiding this comment

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

f16 vector should use the same default formatting as f16. Could you change this to only apply to the Rust output of intrinsic-test instead of all users of the Debug impl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, it takes a bunch more lines but it's probably better this way.

@folkertdev folkertdev force-pushed the remove-avx512_target_feature branch from 2ff1273 to 4c72851 Compare May 20, 2025 11:07
@Amanieu Amanieu added this pull request to the merge queue May 20, 2025
Merged via the queue into rust-lang:master with commit ffa1235 May 20, 2025
62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants