Fix fabs() for MIPS when used on -0.0
ClosedPublic
Actions

Authored by arichardson on Nov 17 2017, 5:09 PM.

Details

Reviewers

andrew
jhb
brooks
imp
emaste

Group Reviewers

MIPS

Commits

rS326342: Fix fabs() for MIPS when used on -0.0

Summary

It would previously return negative zero for -0.0 since -0.0 does not
compare less than 0. The issue was discovered when running the libc++
test suite on softfloat MIPS64.

I have verified that both clang and GCC generate sensible code for the
builtin. For soft float they clear the sign bit using integer operations
and in hard float mode they use abs.d.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

arichardson created this revision.Nov 17 2017, 5:09 PM

Herald added a subscriber: imp. · View Herald TranscriptNov 17 2017, 5:09 PM

My only question is if we should do the same for the arm C version.

In D13135#274569, @jhb wrote:

My only question is if we should do the same for the arm C version.

https://reviews.freebsd.org/D13131

jhb accepted this revision.Nov 21 2017, 8:05 PM

This revision is now accepted and ready to land.Nov 21 2017, 8:05 PM

FreeBSD's style is to put ()'s around return values, so it should probably be 'return (__builtin_fabs(x));'

LGTM

This is honestly fine as it is. Two minor nits, but they are uber nits and can be ignored perhaps...

lib/libc/mips/gen/fabs.c
43 ↗	(On Diff #35379)	The nit-pikcer's parade: (1) should have a blank line after the { since there's no local variables. (2) () around the return value. These are documented in style(9), but customs are often observed in their breech rather than their adherence.