libm icon indicating copy to clipboard operation
libm copied to clipboard

`TotalEq` -0 in `fabs` test

Open Rudxain opened this issue 1 year ago • 6 comments

assert_eq! uses PartialEq, which doesn't discern between -0 and +0. total_cmp does distinguish them. So this patch makes the test spec-compliant

Rudxain avatar Apr 11 '24 21:04 Rudxain

I recommend reviewing any other code that uses == and/or assert_eq!, to check if it should be replaced by total_cmp. total_cmp also distinguishes between signed NANs, which may be useful somewhere else in the codebase

Rudxain avatar Apr 11 '24 21:04 Rudxain

It would be better to use .to_bits() to ensure an exact match.

Amanieu avatar Apr 11 '24 23:04 Amanieu

.to_bits()

I agree, but that would only apply to other cases (such as performance-sensitive code), not here. The spec imposes no requirements about fabs return bit pattern

Rudxain avatar Apr 11 '24 23:04 Rudxain

There is only a single bit pattern for 0 or -0.

Amanieu avatar Apr 12 '24 19:04 Amanieu

There is only a single bit pattern for 0 or -0.

Yes, but some platforms use "flush-to-zero" for subnormal numbers. I'm not entirely sure how that mechanism works 🤔

Rudxain avatar Apr 12 '24 19:04 Rudxain

That doesn't have any effect here. It's only done during arithmetic operations, not comparisons. In any case the Rust language doesn't allow flush-to-zero behavior, so such modes must be turned off in the CPU when running Rust code.

Amanieu avatar Apr 12 '24 23:04 Amanieu