[webkit-reviews] review denied: [Bug 97008] [BlackBerry] Allow denormal floats in ARM VFP : [Attachment 165301] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Sep 23 20:23:09 PDT 2012


Filip Pizlo <fpizlo at apple.com> has denied Cosmin Truta <ctruta at gmail.com>'s
request for review:
Bug 97008: [BlackBerry] Allow denormal floats in ARM VFP
https://bugs.webkit.org/show_bug.cgi?id=97008

Attachment 165301: Patch
https://bugs.webkit.org/attachment.cgi?id=165301&action=review

------- Additional Comments from Filip Pizlo <fpizlo at apple.com>
I don't like the way you've done this.

1) If you need to do things to your CPU to make it conform, then wouldn't it be
better to do those things in the code that uses JavaScriptCore rather than in
JavaScriptCore itself?	The way you've implemented it, a program that uses
JavaScriptCore will start out having one kind of FP semantics, until the first
JSGlobalData is instantiated, at which point it will henceforth have a
different kind of FP semantics.  That's weird.	Wouldn't it be better to have
the client program set up the FP state that it wants at the beginning
(equivalent of main())?

2) If you really must put this into JavaScriptCore, say because you don't want
the client program to care about what kinds of FP settings JavaScriptCore
wants, then wouldn't it be infinitely better to put it someplace like
initializeThreading()?	Changing the FP state should have nothing to do with
creating a new JSC instance, especially since this is by definition a one-way
state change - JSGlobalData::~JSGlobalData does not reset the FP state, and you
don't flip it in any way on entry into this particular JSGlobalData instance.

Please consider doing either (1) or (2) instead of this current approach.


More information about the webkit-reviews mailing list