-
Notifications
You must be signed in to change notification settings - Fork 76
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
Fix test for float32_cvt_i32 #3178
Fix test for float32_cvt_i32 #3178
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix looks fine, but need to figure out what's changed in the macOS CI
(Given the timing, I wonder whether it could |
61e4123
to
a34e7e4
Compare
a34e7e4
to
6916882
Compare
The problem was with operations on NaNs. The problem was exposed by turning optimizations on: including
Looks like |
I see, thanks. I would prefer to continue testing that the nan behavior is consistent with the C stubs, since our OCaml intrinsics don't depend on compiler flags - do you know what C operation(s) have a different result with -O2? We don't enable fast-math so the change is a little surprising. |
There were a few differences. At least |
This reverts commit 6916882.
I think we determined that the difference comes from |
One of the C stubs in
flambda-backend/tests/simd/
uses incompatible types. There is a macro that calls_mm_extract_ps
which expects__m128
argument, but one of the uses of the macro is with the result of_mm_cvtps_epi32
which is of type__m128i
. This issue was originally reported by @bclement-ocp.This PR attempts to fix it:
_mm_extract_epi32
instead of_mm_extract_ps
for_mm_cvtps_epi32
.standard
toc_flags
in this test directory, to get the warning about "incompatible type" in the CI.