Skip to content

s390x: add f32 min/max tests#2059

Closed
RalfJung wants to merge 1 commit intorust-lang:mainfrom
RalfJung:s390x-minmax-test
Closed

s390x: add f32 min/max tests#2059
RalfJung wants to merge 1 commit intorust-lang:mainfrom
RalfJung:s390x-minmax-test

Conversation

@RalfJung
Copy link
Member

Currently no test actually covers instruction selection for vec_min / vec_max without the vector-enhancements-1 target feature.

r? @folkertdev

test_vec_2! { test_vec_max, vec_max, f32x4, f32x4 -> f32x4,
[1.0, f32::NAN, f32::INFINITY, 2.0],
[-10.0, -10.0, 5.0, f32::NAN],
[1.0, -10.0, f32::INFINITY, 2.0]
Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I expect this to fail as the fallback does not handle NaNs correctly.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, strange. Tests pass with and without the enhancement target feature. Is the fallback actually more accurate than the docs say?

Copy link
Member Author

@RalfJung RalfJung Mar 11, 2026

Choose a reason for hiding this comment

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

Ah no, the test probably gets optimized away. (EDIT: Or maybe not, LLVM might just do a proper fallback that matches what it documents rather than the IBM docs.)

Still useful to have the test as it would have shown the issue with #2058.

@RalfJung RalfJung changed the title add f32 min/max tests s390x: add f32 min/max tests Mar 11, 2026
@RalfJung
Copy link
Member Author

We can just land this with #2058

@RalfJung RalfJung closed this Mar 11, 2026
@RalfJung RalfJung deleted the s390x-minmax-test branch March 11, 2026 07:54
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.

2 participants