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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

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?

for now, keep formatting vectors containing f16 values the old way (that is the same as what C does)
@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?

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