This is an archive of the discontinued LLVM Phabricator instance.

[builtins][test] Skip scalbn rounding tests on newlib
ClosedPublic

Authored by dcandler on May 10 2023, 11:01 AM.

Details

Summary

The picolib/newlib implementation of scalbn gives slightly different
results compared to glibc and compiler-rt's inlined
__compiler_rt_scalbn in certain rounding modes. Since these tests
are already disabled for msvc which doesn't respect the mode change,
this patch skips them for newlib as well.

Diff Detail

Event Timeline

dcandler created this revision.May 10 2023, 11:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 10 2023, 11:01 AM
Herald added a subscriber: Enna1. · View Herald Transcript
dcandler requested review of this revision.May 10 2023, 11:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 10 2023, 11:01 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript

This change is simple enough but I am not sure compiler-rt has committed to support picolib/newlib and ensure its tests passing.
How do you build and test this for newlib?

This was built and tested using the LLVM embedded toolchain for Arm which uses picolibc.

compiler-rt/test/builtins/Unit/compiler_rt_scalbnf_test.c
63

Small nit to myself, shouldn't drop the f from scalbnf.

MaskRay added a comment.EditedMay 11 2023, 2:39 PM

It'd be good to have a clearer message than "results in certain cases." What are the "certain cases"? Is it a precision problem in picolibc?

More specifically, the cases where results differ are the tests that use a large exponent and can trigger a underflow/overflow when the rounding mode is changed to FE_UPWARD or FE_DOWNWARD. For example:

FE_UPWARD:                           (compiler-rt/glibc)            (newlib)
scalbnf(-0x1p+0 [BF800000], -1000) = -0x0p+0 [80000000]          != -0x1p-149 [80000001]
scalbnf(-0x1p+0 [BF800000], +1000) = -0x1.fffffep+127 [FF7FFFFF] != -inf [FF800000]
FE_DOWNWARD:
scalbnf(-0x1p+0 [BF800000], -1000) = -0x1p-149 [80000001]        != -0x0p+0 [80000000]
scalbnf(-0x1p+0 [BF800000], +1000) = -inf [FF800000]             != -0x1.fffffep+127 [FF7FFFFF]

However, the results are only different for negative inputs. For positive values the results match:

FE_UPWARD:                          (compiler-rt/glibc)           (newlib)
scalbnf(0x1p+0 [3F800000], -1000) = 0x1p-149 [1]               == 0x1p-149 [1]
scalbnf(0x1p+0 [3F800000], +1000) = inf [7F800000]             == inf [7F800000]
FE_DOWNWARD:
scalbnf(0x1p+0 [3F800000], -1000) = 0x0p+0 [0]                 == 0x0p+0 [0]
scalbnf(0x1p+0 [3F800000], +1000) = 0x1.fffffep+127 [7F7FFFFF] == 0x1.fffffep+127 [7F7FFFFF]

Similarly there's no problem for the FE_TOWARDZERO or FE_TONEAREST modes. The compiler-rt and glibc values look correct, so it appears the newlib scalbn can be less accurate in upward and downward modes.

dcandler updated this revision to Diff 525723.May 25 2023, 11:41 AM

Updated comment.

MaskRay accepted this revision.May 25 2023, 1:13 PM
This revision is now accepted and ready to land.May 25 2023, 1:13 PM
dcandler updated this revision to Diff 526068.May 26 2023, 8:08 AM

Small formatting fix for the failed pre-merge check.

This revision was automatically updated to reflect the committed changes.