555805 - Support architectures that don't implement subnormal floats

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect, P3)

Description

[Stan Switzer]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.8) Gecko/20100202 Firefox/3.5.8
Build Identifier: Tamarin Redux, March '10

A number of architectures don't implement subnormal floating point representations in hardware, but instead trap to a software emulator. Some of these (in particular, ARM) support a "run-fast" mode that simply flushes subnormal values to zero. While the run-fast mode is under control of the user program, the trap handler appears to be the province of the operating system, so clearing the flush-to-zero bit is not necessarily a solution but can instead result in an unhandled trap.

The upshot is that the smallest non-zero number (Number.MIN_VALUE) is larger (2.2250738585072014e-308) than the currently hard-coded IEEE value (4.94e-324).

After much discussion of whether this should be fixed using conditional compilation or with dynamic code, I've decided that only a dynamic solution is completely satisfactory (we COULD conditionally-compile whether it is dynamic, but to that end?).  On systems that have this issue, subnormal support is something that could change with any operating system upgrade or, say, between different processors in the same product family. So the value of MIN_VALUE might be different for different invocations or installations of the same program.

What would we like to be able to say about MIN_VALUE?  I think we want the following two things to be true:

  Min value is not zero:  Number.MIN_VALUE > 0

and

  All smaller values (for instance MIN_VALUE/2) are zero: Number.MIN_VALUE/2 == 0



Reproducible: Always

Steps to Reproduce:
Currently, the value of Number.MIN_VALUE is equal to zero on the iPhone.

Comment 1

[Stan Switzer]

Attachment: Proposed patch to compute Number.MIN_VALUE correctly on machines without subnormal doubles

Comment 2

[Rick Reitmaier]

FYI, the ARM arch manual states the following as being "rounded to +0" in flush-to-zero mode; 
(1) single precision results which lie in the range -2e-126 < x < +2e-126 
(2) double precision results which lie in the range -2e-1022 < x < +2e-1022

Comment 3

[Stan Switzer]

To clarify: -2^(-1022) < x < 2^(1022) is the round-to-zero range.

2^(-1022) ~= 2.2250738585072014e-308

Comment 4

[Rick Reitmaier]

Good point, I meant base 2 not base 10.

Comment 5

[Stan Switzer]

Perforce CL 643276; still on a sideline branch.
Watson Bugs 2552754, 2471354

Comment 6

[Lars T Hansen]

Documentation update request filed: bug #557277.

Comment 7

[Lars T Hansen]

A couple of issues with the patch:

- I don't know why this code should be #ifdef ARM, particularly.

- The code in the DEBUG block is probably wrong, it assumes a particular
  floating-point layout but floating point layouts can be strange, for
  example with high/low words laid out in one endianness and the bytes within
  the words laid out in the other.  I have to dig out some references.

Other than that the patch seems sound.

Comment 8

[Lars T Hansen]

Specifically, look at AVMSYSTEM_DOUBLE_MSW_FIRST in core/avmfeatures.h.  It looks as though we're not handling this case well throughout the code, however - the only code that appears to handle it systematically is the UNIX case in MathUtils.cpp.  Other code #errors out or ignores the problem.  There are no obvious open bugs to track this problem though.

Comment 9

[Steven Johnson]

(In reply to comment #8)
> Specifically, look at AVMSYSTEM_DOUBLE_MSW_FIRST in core/avmfeatures.h.  It
> looks as though we're not handling this case well throughout the code, however

AFAIK, the only mainstream architecture with this property is MIPS (and even then maybe only some configs) -- as our MIPS support is still poorly tested, I'm not surprised if it's dodgy.

Comment 10

[Lars T Hansen]

Mess around floating point layouts filed as bug #557286, won't record it as a blocker though, and won't require us to deal with it for the purposes of closing the present bug.

Comment 11

[Lars T Hansen]

Acceptance test cases will require updating too.

Comment 12

[Lars T Hansen]

Attachment: Restructured patch

Slightly cleaned up patch; cleaned up test cases.

Comment 13

[Edwin Smith]

Comment on attachment 437113 [details] [diff] [review]
Restructured patch

I assume the plan is to start using double_overlay all over the code where necessary, and thats why its not just defined in NumberClass.cpp.

Comment 14

[Lars T Hansen]

(In reply to comment #13)
> (From update of attachment 437113 [details] [diff] [review])
> I assume the plan is to start using double_overlay all over the code where
> necessary, and thats why its not just defined in NumberClass.cpp.

yep.  every day in every way things are going better and better.

Comment 15

[Stan Switzer]

(In reply to comment #9)
> AFAIK, the only mainstream architecture with this property is MIPS (and even
> then maybe only some configs)

It's surprisingly common, actually, but most OS/RTL implementations hide it by handling the trap. This paper has a lot of interesting information on that:

   http://doc.cat-v.org/plan_9/4th_edition/papers/port

Yeah, a lot of that is only interesting to us old-timers.

Comment 16

[Lars T Hansen]

tamarin-redux-argo changeset:   3934:57a1c695b847

Comment 17

[Brent Baker]

Testcase was added with proper changeset: (changeset in comment #16 is incorrect)
tamarin-redux-argo changeset: 3935:57a1c695b847

http://asteam.macromedia.com/hg/tamarin-redux-argo/diff/57a1c695b847/test/acceptance/ecma3/Number/e15_7_1.as