This is an archive of the discontinued LLVM Phabricator instance.

[libc] Implement log1pf correctly rounded to all rounding modes.
ClosedPublic

Authored by lntue on Feb 3 2022, 6:31 PM.

Details

Summary

Implement log1pf correctly rounded to all rounding modes relying on logf implementation for exponent > 2^(-8).

Diff Detail

Event Timeline

lntue created this revision.Feb 3 2022, 6:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 3 2022, 6:31 PM
lntue requested review of this revision.Feb 3 2022, 6:31 PM

I get several warnings when I compile this version:

In file included from /localdisk/zimmerma/llvm-project/libc/src/stdlib/strtold.cpp:11:
In file included from /localdisk/zimmerma/llvm-project/libc/src/__support/str_to_float.h:16:
/localdisk/zimmerma/llvm-project/libc/src/__support/high_precision_decimal.h:115:42: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'uint32_t' (aka 'unsigned int') [-Wsign-compare]
    if (roundToDigit < 0 || roundToDigit >= this->num_digits) {
                            ~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~
/localdisk/zimmerma/llvm-project/libc/src/__support/high_precision_decimal.h:121:26: warning: comparison of integers of different signs: 'int' and 'uint32_t' (aka 'unsigned int') [-Wsign-compare]
        roundToDigit + 1 == this->num_digits) {
        ~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~
zimmermann6 accepted this revision.Feb 4 2022, 1:05 AM

apart the compiler warnings, all exhaustive tests comparing to MPFR do pass, for all four rounding modes. Good work!

This revision is now accepted and ready to land.Feb 4 2022, 1:05 AM
lntue added a comment.Feb 4 2022, 6:48 AM

I get several warnings when I compile this version:

In file included from /localdisk/zimmerma/llvm-project/libc/src/stdlib/strtold.cpp:11:
In file included from /localdisk/zimmerma/llvm-project/libc/src/__support/str_to_float.h:16:
/localdisk/zimmerma/llvm-project/libc/src/__support/high_precision_decimal.h:115:42: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'uint32_t' (aka 'unsigned int') [-Wsign-compare]
    if (roundToDigit < 0 || roundToDigit >= this->num_digits) {
                            ~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~
/localdisk/zimmerma/llvm-project/libc/src/__support/high_precision_decimal.h:121:26: warning: comparison of integers of different signs: 'int' and 'uint32_t' (aka 'unsigned int') [-Wsign-compare]
        roundToDigit + 1 == this->num_digits) {
        ~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~

We are in the process of cleaning up the project to build with gcc. Thess warnings will be fixed soon.

I get several warnings when I compile this version:

In file included from /localdisk/zimmerma/llvm-project/libc/src/stdlib/strtold.cpp:11:
In file included from /localdisk/zimmerma/llvm-project/libc/src/__support/str_to_float.h:16:
/localdisk/zimmerma/llvm-project/libc/src/__support/high_precision_decimal.h:115:42: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'uint32_t' (aka 'unsigned int') [-Wsign-compare]
    if (roundToDigit < 0 || roundToDigit >= this->num_digits) {
                            ~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~
/localdisk/zimmerma/llvm-project/libc/src/__support/high_precision_decimal.h:121:26: warning: comparison of integers of different signs: 'int' and 'uint32_t' (aka 'unsigned int') [-Wsign-compare]
        roundToDigit + 1 == this->num_digits) {
        ~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~

We are in the process of cleaning up the project to build with gcc. Thess warnings will be fixed soon.

Are these warnings really related to the GCC work?

lntue added a comment.Feb 7 2022, 10:30 AM

I get several warnings when I compile this version:

In file included from /localdisk/zimmerma/llvm-project/libc/src/stdlib/strtold.cpp:11:
In file included from /localdisk/zimmerma/llvm-project/libc/src/__support/str_to_float.h:16:
/localdisk/zimmerma/llvm-project/libc/src/__support/high_precision_decimal.h:115:42: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'uint32_t' (aka 'unsigned int') [-Wsign-compare]
    if (roundToDigit < 0 || roundToDigit >= this->num_digits) {
                            ~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~
/localdisk/zimmerma/llvm-project/libc/src/__support/high_precision_decimal.h:121:26: warning: comparison of integers of different signs: 'int' and 'uint32_t' (aka 'unsigned int') [-Wsign-compare]
        roundToDigit + 1 == this->num_digits) {
        ~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~

We are in the process of cleaning up the project to build with gcc. Thess warnings will be fixed soon.

Are these warnings really related to the GCC work?

These warnings are caused by https://reviews.llvm.org/D118791 and being fixed by https://reviews.llvm.org/D119156

overall LGTM with minor nit

libc/src/math/generic/log1pf.cpp
34

should this be in common_constants.cpp?

lntue updated this revision to Diff 406579.Feb 7 2022, 12:49 PM

Move LOG_F table to common_constants.

lntue marked an inline comment as done.Feb 7 2022, 12:50 PM
sivachandra accepted this revision.Feb 7 2022, 1:10 PM