OpenSmalltalk / opensmalltalk-vm

Cross-platform virtual machine for Squeak, Pharo, Cuis, and Newspeak.
http://opensmalltalk.org/
Other
562 stars 110 forks source link

primitives 54 and 554 for timesTwoPower: answer incorrect result on 64bits macOS VM #383

Open nicolas-cellier-aka-nice opened 5 years ago

nicolas-cellier-aka-nice commented 5 years ago

This is due to a macOS bug in 64bits libm! (Hard to believe, isn't it?)

I've opened a ticket https://bugreport.apple.com/web/?problemID=48021471

Area: Something not on this list

Summary: ldexp incorrectly rounds gradual underflow for some specific values.

Steps to Reproduce:

include

include

int main() { int exp=-54; double y=ldexp(11.0,exp); / y is binary 1.0112^-51 / double u=ldexp(1.0,-1074); / u is the minimal denormalized IEEE754 double / double v=ldexp(y,-1023); / v is binary 1.0112^-1074 and should round to u / printf("u=%g v=%g\n",u,v); return 0; }

Expected Results: v should be rounded to 1.0 * 2^-1074 (round to nearest, tie to even default rule) Thus we should have u == v.

Actual Results: v is rounded upward to binary 10.0 2^-1074 = 1.0 2^-1073 output is u=4.94066e-324 v=9.88131e-324

Note 1: this fails for 4 different values of exp -54,-53,+968,+969 (replace exponent -1023 by -1074-3-exp, that is -1023,-1024,-2045,-2046) Note 2: this did not happen previously with 32bits libm version. Note 3: this sounds a bit like this bug https://stackoverflow.com/questions/32150888/should-ldexp-round-correctly

Version/Build: macOS HighSierra 10.13.16

Configuration: compiled with clang --version Apple LLVM version 10.0.0 (clang-1000.11.45.5) Target: x86_64-apple-darwin17.7.0 Thread model: posix InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin Xcode Version 10.1 (10B61)

krono commented 4 years ago

What can we use instead? Platform-specific asm?

nicolas-cellier-aka-nice commented 4 years ago

Eliot was working for providing native bit-identical floating point functions for Terf. So we would use the patched fdlibm once finished.

eliotmiranda commented 4 years ago

Hi Nicolas, Tobias,

On Thu, May 14, 2020 at 12:47 PM Nicolas Cellier notifications@github.com wrote:

Eliot was working for providing native bit-identical floating point functions for Terf. So we would use the patched fdlibm once finished.

It seems to be ready to go. All we do is define BIT_IDENTICAL_FLOATING_POINT in the makefile. In fact, it might be good to invert things, and have this be the default unless one defines, e.g. PLATFORM_SPECIFIC_FLOATING_POINT. What do y'all think?

build.macos64x64$ diff squeak.cog.spur/Makefile croquet.cog.spur/Makefile 5a6,7

BIT_IDENTICAL_FLOATING_POINT=BIT_IDENTICAL_FLOATING_POINT

8a11,17

Produce Croquet.app, CroquetAssert.app & CroquetDebug.app

APPNAME:=Croquet APPNAMEDEF:=$(APPNAME) APPIDENTIFIER:=org.Croquet.$(APPNAME)

CFLAGS:=-DCROQUET=1

11c20 < include ../common/Makefile.app.squeak

include ../common/Makefile.app

,,,^..^,,, best, Eliot

marceltaeumel commented 4 years ago

+1 for PLATFORM_SPECIFIC_FLOATING_POINT

At a glance, I couldn't read the meaning "cross-platform" in BIT_IDENTICAL_FLOATING_POINT :-)

Best, Marcel

cstes commented 4 years ago

For what it's worth, I think that if those primitives 54 and 554 fail sometimes, then they can perhaps be added in a Test framework (perhaps those Tests were already written) - the "Run Tests" in Squeak. For example I can see that for some other "float" bug there is such a test : testZeroRaisedToNegativePower "this is a test related to http://bugs.squeak.org/view.php?id=6781" (there is also a method testTimesTwoPowerGradualUnderflow in KernelTests-Numbers -> FloatTest -> test - arithmetic.

Ideally I think the Squeak kernel should have some configuration option to be able to use the system provided libmath (or not) so that it's possible to choose which one , when compiling/building the VM.

cstes commented 4 years ago

I checked the Float-tests (Kernel-Numbers) in Squeak6.0alpha-19793-64bit.image.

I don't immediately see in the Test framework whether there is a Squeak test, equivalent to the tests for this issue.

Is it perhaps already in the Test framework , maybe I just don't see it ?

David Stes