JS must init the floating point unit on every new-thread, or even more often

RESOLVED FIXED in M18

Status

()

Core
JavaScript Engine
P3
normal
RESOLVED FIXED
18 years ago
16 years ago

People

(Reporter: brendan, Assigned: brendan)

Tracking

Trunk
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

18 years ago
I changed the JS engine to initialize the FPU to sane numeric modes once for the
first thread that runs JS.  That was wrong: if an OS is insane, it'll reset the
FPU modes for other threads, so JS must re-reset them on every new-thread (if it
can hook into new-thread).  For a long time, JS has redundantly initialized the
FPU on every JS_NewContext, which is at least 1:1 with thread, and N:1 for big N
in Mozilla-the-browser (each window in the DOM/UI/Layout thread has its own cx).

See bug 39237, which tried to morph into this bug, but should not (once we fix
this bug, 39237 will still be a valid, Futured bug about keyboard event probs).

Mike Kaply of IBM found strange similar problems on OS2.  I'm going to restore
the old JS FPU initialization timing.

/be
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → M18

Comment 1

18 years ago
We are able to work around this by putting the _control87 call directly before 
the call to JS_DefineConstDoubles.

Rather than backing all the changes out, it might be easier to either make this 
a configure option like "broken floating point" or just put it in an XP_OS2.
(Assignee)

Comment 2

18 years ago
Created attachment 13453 [details] [diff] [review]
proposed fix
(Assignee)

Comment 3

18 years ago
Mike, anyone: please test this patch and give me an r= if it's good.  Thanks,

/be

Comment 4

18 years ago
We did some research and found that we don't need special _control87 flags for 
OS/2. We can 
 take the M_IX86 path and have no OS/2 specific code here.

Comment 5

18 years ago
Created attachment 13460 [details] [diff] [review]
New diff without OS/2 specific stuff

Comment 6

18 years ago
Based on just eyeballing the code, r=mkaply

I have it building but I won't know for some time whether it is OK.

If possible though, I would like it checked in anyway to get a nightly build 
tonight.

Thanks for the help on this.
(Assignee)

Comment 7

18 years ago
Thanks, it's in (sorry to nitpick your patch, but it has CRLF rather than LF as
line terminator, and at least one four-space initial indentation was expanded
wrongly to a hard tab; hey, I've attached CRLF-terminated patches myself, but
they don't work for Unix users, whereas LF-termination works all around, IIRC). 
Thanks again, let me know if it doesn't work for OS2.

/be

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Updated

18 years ago
Blocks: 39237

Comment 8

18 years ago
I just found out that GNU C does not define _M_IX86

Our other main OS/2 contributor would prefer an explicit mention of XP_OS2 on 
this line, so how about:

#if !defined __MWERKS__ && defined (XP_PC) && (defined (_M_IX86) || 
defined(XP_OS2))
You need to log in before you can comment on or make changes to this bug.