search

rust-lang / libm

A port of MUSL's libm to Rust.
Other
540 stars 98 forks source link

Add a small conformance test to sqrt/sqrtf #274

Closed ZagButNoZig closed 1 year ago

ZagButNoZig commented 1 year ago

What this PR does

Add a test that guarantees that the result of sqrt/sqrtf is bit for bit identical on all platforms for a given array of test values.

Why this PR

This is a proposed fix for issue #255. Which asks all sqrt implementations to be IEEE 754 2008 compliant. While this does not prove compliance with the spec it at least proves conformance between platforms.

Issues with this PR

Since the values and their results are hard coded, they would need to be updated on every implementation change. The values are also somewhat arbitrary and don't cover every case inside the sqrt implementation.

Any feedback is very welcome.

Amanieu commented 1 year ago

Tests are failing for sqrt(-1) because the sign of the NaN is different from what was expected. I think this is an acceptable divergence.

ZagButNoZig commented 1 year ago

I removed the tests with negative numbers. I'm really confused as to why the divergence happened in the first place. I have run the software sqrtf implementation on x86 (the non sse path) and it also seems to return 0xff800000, but as far as I can tell the ARM doesn't have a hardware intrinsic, so it must also be using the software implementation of sqrtf but returns a different result (0x7f800000). Which means both platforms produce a different result on this line if I didn't miss anything.

Amanieu commented 1 year ago

The sign of a NaN result is implementation-defined and not specified by IEEE-754. So it makes sense that different architectures produce different results.