Last Comment Bug 746273 - Double-check mfbt/FloatingPoint.h's bitwise floating point operations for correctness
: Double-check mfbt/FloatingPoint.h's bitwise floating point operations for cor...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: MFBT (show other bugs)
: Trunk
: All Windows 2000
: -- normal (vote)
: ---
Assigned To: Jim Blandy :jimb
:
Mentors:
Depends on:
Blocks: 714260
  Show dependency treegraph
 
Reported: 2012-04-17 12:16 PDT by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2012-06-14 08:48 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Dummy patch on which to flag jimb -- see mfbt/FloatingPoint.h for the code to review (1 bytes, patch)
2012-04-17 12:26 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
nfroyd: review+
Details | Diff | Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2012-04-17 12:16:16 PDT
Bug 714260 moved all the low-level floating point bit-level algorithms into mfbt/FloatingPoint.h.  At that time I thought I'd gotten enough reviews of the patch to land it.  A couple few weeks later, tho, and paranoia's starting to set in.  I'd like someone to do a last comparison of the algorithms defined there to what IEEE-754 says should be the case, specifically checking for consistency.

I *do not* believe or expect there's a difference between what's implemented and what should be implemented.  But given the JS engine's reliance on these low-level details for value representation, it's worth another look.

Given that I don't think there's a problem here, I don't think it's critical this happen before the upcoming merge.  But it should definitely happen comfortably before the merge after that, so that *if* something gets found, we can fix it in the then-aurora branch without having to mess with scheduling.  jimb says he can do this after the 30th, early in the next cycle -- unless someone complains loudly, I think that's adequate timing given I doubt anything's actually wrong.
Comment 1 Jeff Walden [:Waldo] (remove +bmo to email) 2012-04-17 12:26:07 PDT
Created attachment 615820 [details] [diff] [review]
Dummy patch on which to flag jimb -- see mfbt/FloatingPoint.h for the code to review
Comment 2 Lukas Blakk [:lsblakk] use ?needinfo 2012-05-02 15:46:54 PDT
Assigning to Jeff for now since he's the only one who's touching it but if another viable bug owner comes alone, who's name isn't 'nobody@mozilla.com', please reassign as needed.
Comment 3 Lukas Blakk [:lsblakk] use ?needinfo 2012-05-02 15:49:45 PDT
Sorry, moving this over to jimb who's review is all that is needed here.
Comment 4 Lukas Blakk [:lsblakk] use ?needinfo 2012-06-03 10:58:01 PDT
[Triage Comment]

Merging of 14 to the beta channel is happening tomorrow. What's the current game plan for this issue?  I'll remove tracking unless someone makes a case that this will be worked on in time to land in the first couple of beta builds.
Comment 5 Nathan Froyd [:froydnj] 2012-06-08 12:45:08 PDT
Apparently Splinter is confused if there's no diff.

As far as consistency goes, everything looks like it does what it says on the tin.
Comment 6 Jeff Walden [:Waldo] (remove +bmo to email) 2012-06-08 13:47:41 PDT
Good, good, that's what I expected.  We're all set here then (including on all branches, since there's nothing to change).

Note You need to log in before you can comment on or make changes to this bug.