This is an archive of the discontinued LLVM Phabricator instance.

[libc] Add implementations of fdim[f|l].
ClosedPublic

Authored by lntue on Nov 5 2020, 11:29 PM.

Details

Summary

Implementing fdim, fdimf, and fdiml for llvm-libc.

Diff Detail

Event Timeline

lntue created this revision.Nov 5 2020, 11:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 5 2020, 11:29 PM
lntue requested review of this revision.Nov 5 2020, 11:29 PM
sivachandra added inline comments.Nov 6 2020, 3:45 PM
libc/test/src/math/FDimTest.h
16

To be consistent with other places, do not put the test classes in the any namespace.

25

Can you move this declaration out of the method? This way, other methods can also use them without redeclarations?

libc/test/src/math/fdim_test.cpp
18

We don't need these. I will clean it up in the ilogb tests.

libc/utils/FPUtil/BasicOperations.h
78

The operation x - y can lead to overflow or underflow. Shouldn't we detect that and return the appropriate value as specified by the standard? Or, should we assume that the machine instruction does the right thing?

lntue updated this revision to Diff 304043.Nov 9 2020, 9:47 PM
lntue marked an inline comment as done.

[libc] Add implementations of fdim[f|l].

lntue updated this revision to Diff 304044.Nov 9 2020, 9:48 PM

[libc] Add implementations of fdim[f|l].

lntue updated this revision to Diff 304054.Nov 9 2020, 10:14 PM

Update constants for FDimTest.

lntue marked an inline comment as done.Nov 9 2020, 10:20 PM
lntue added inline comments.
libc/test/src/math/FDimTest.h
25

FPBits class cannot be constexpr yet, so we cannot have these constants as static members of the class yet. I moved them out as const members instead.

libc/utils/FPUtil/BasicOperations.h
78

We are assuming CPU's supporting IEEE 754 arithmetic, so the machine instruction will do the right thing. For example the spec specified to return HUGE_VAL in case of overflow, which is resolved to inf for IEEE 754 standard.

sivachandra accepted this revision.Nov 9 2020, 11:01 PM
sivachandra added inline comments.
libc/test/src/math/FDimTest.h
25

LGTM.

libc/utils/FPUtil/BasicOperations.h
78

SGTM. So, in general, out position is:

If we use a language operator for a floating point operation, then we assume that the machine instruction (or instructions) implementing the operator conform to the IEEE-754 standard.

Does that sound reasonable?

This revision is now accepted and ready to land.Nov 9 2020, 11:01 PM
lntue added inline comments.Nov 10 2020, 6:03 AM
libc/utils/FPUtil/BasicOperations.h
78

SGTM

This revision was landed with ongoing or failed builds.Nov 10 2020, 3:49 PM
This revision was automatically updated to reflect the committed changes.