This is an archive of the discontinued LLVM Phabricator instance.

[libc][math] Set floating point exceptions for exp*f, sinhf, and coshf.
ClosedPublic

Authored by lntue on Feb 18 2023, 6:37 PM.

Details

Summary

Set FE_OVERFLOW and FE_UNDERFLOW for expf, exp2f, exp10f, expm1f, sinhf
and coshf.

Diff Detail

Event Timeline

lntue created this revision.Feb 18 2023, 6:37 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFeb 18 2023, 6:37 PM
lntue requested review of this revision.Feb 18 2023, 6:37 PM

Should the errno be set and exceptions raised based on math_errhandling?

lntue added a comment.Feb 22 2023, 2:29 PM

Should the errno be set and exceptions raised based on math_errhandling?

I skipped the check for math_errhandling because of the following line from https://en.cppreference.com/w/cpp/numeric/math/math_errhandling :

If the implementation supports IEEE floating-point arithmetic (IEC 60559), math_errhandling & MATH_ERREXCEPT is required to be non-zero.
renyichen accepted this revision.Feb 22 2023, 2:35 PM
renyichen added inline comments.
libc/test/src/math/exp10f_test.cpp
42

also test Underflow?

This revision is now accepted and ready to land.Feb 22 2023, 2:35 PM
sivachandra added a comment.EditedFeb 22 2023, 2:36 PM

Should the errno be set and exceptions raised based on math_errhandling?

I skipped the check for math_errhandling because of the following line from https://en.cppreference.com/w/cpp/numeric/math/math_errhandling :

If the implementation supports IEEE floating-point arithmetic (IEC 60559), math_errhandling & MATH_ERREXCEPT is required to be non-zero.

I think that is true when the implementation is conformant. But, under __FAST_MATH__, it is expected that math_errhandling evaluates to zero: https://github.com/llvm/llvm-project/blob/main/libc/config/linux/api.td#L57. There is __NO_MATH_ERRNO__ also.

lntue updated this revision to Diff 500227.Feb 24 2023, 8:46 AM

Only raise floating point exceptions for math functions if required.

lntue updated this revision to Diff 500230.Feb 24 2023, 8:52 AM

Add underflow tests for exp10f.

lntue marked an inline comment as done.Feb 24 2023, 8:52 AM
sivachandra accepted this revision.Feb 24 2023, 9:49 AM
sivachandra added inline comments.
libc/src/__support/FPUtil/FEnvImpl.h
52

The utils here are what I would view as "only if required" utils to be employed just before returning from a public function. So, I would prefer if they live in the src/math directory, much like https://github.com/llvm/llvm-project/blob/main/libc/src/math/generic/math_utils.h#L53. At some point they should be unified.

55

if constexpr (...) here and below?

lntue added inline comments.Feb 24 2023, 9:56 AM
libc/src/__support/FPUtil/FEnvImpl.h
55

If I remember it right, last time I tried on some platform (maybe Apple clang on mac M1 ?), math_errhandling is defined to be a non-constexpr builtin function call __builtin_math_errhandling, making if failed to compile if math_errhandling is put under if constexpr.

This revision was landed with ongoing or failed builds.Feb 24 2023, 9:56 AM
This revision was automatically updated to reflect the committed changes.