Last Comment Bug 750620 - Make double-conversion portable to exotic architectures
: Make double-conversion portable to exotic architectures
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: MFBT (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla15
Assigned To: Mike Hommey [:glandium]
:
:
Mentors:
Depends on: 614188
Blocks: android-mips
  Show dependency treegraph
 
Reported: 2012-05-01 00:39 PDT by Landry Breuil (:gaston)
Modified: 2012-05-23 08:08 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add some archs (1.23 KB, patch)
2012-05-01 00:45 PDT, Landry Breuil (:gaston)
no flags Details | Diff | Splinter Review
Declare double conversion correctness for more architectures (1.47 KB, patch)
2012-05-09 00:29 PDT, Mike Hommey [:glandium]
nfroyd: review-
Details | Diff | Splinter Review
Declare double conversion correctness for more architectures (1.83 KB, patch)
2012-05-18 05:24 PDT, Mike Hommey [:glandium]
nfroyd: review-
Details | Diff | Splinter Review
Declare double conversion correctness for more architectures (3.27 KB, patch)
2012-05-21 06:42 PDT, Mike Hommey [:glandium]
nfroyd: review+
Details | Diff | Splinter Review

Description Landry Breuil (:gaston) 2012-05-01 00:39:05 PDT
mfbt/double-conversion/utils.h only cares about tier1 archs, what about the others ?

According to http://trac.webkit.org/browser/trunk/Source/WTF/wtf/dtoa/utils.h#L52 way more architectures should be handled...
Comment 1 Landry Breuil (:gaston) 2012-05-01 00:45:41 PDT
Created attachment 619854 [details] [diff] [review]
Add some archs

This adds ppc/alpha/sparc* but there probably should be some more...
Comment 2 Mike Hommey [:glandium] 2012-05-01 02:06:37 PDT
Comment on attachment 619854 [details] [diff] [review]
Add some archs

Let's do this in one step. I'll check the architectures I have access to too and will add them. That includes avr32, hppa, ia64, m68k, s390/s390x and sh4.
Comment 3 Landry Breuil (:gaston) 2012-05-08 00:28:20 PDT
Mike, any news ?
Comment 4 Mike Hommey [:glandium] 2012-05-09 00:29:09 PDT
Created attachment 622305 [details] [diff] [review]
Declare double conversion correctness for more architectures

A few notes:
- _MIPS_ARCH_MIPS32R2 is not defined on Debian mips, so added __mips__
- __sparc__ is defined sparc64
- __s390__ is defined on s390x
Comment 5 Jeff Walden [:Waldo] (remove +bmo to email) 2012-05-09 05:51:48 PDT
Given the fairly quick turnaround on the last patch we submitted there, it probably makes sense to do this upstream first, then pull in a newer revision with these changes:

http://code.google.com/p/double-conversion/
Comment 6 Mike Hommey [:glandium] 2012-05-09 06:00:05 PDT
Do they use codereview for this or just issues on google code?
Comment 7 Jeff Walden [:Waldo] (remove +bmo to email) 2012-05-09 06:12:47 PDT
decoder just filed an issue with them:

http://code.google.com/p/double-conversion/issues/detail?id=26

They appear to be doing the actual review here:

http://code.google.com/p/double-conversion/issues/detail?id=27

So probably filing an issue and having a very small patch to point to should be adequate.
Comment 8 Mike Hommey [:glandium] 2012-05-09 06:45:02 PDT
Filed
http://code.google.com/p/double-conversion/issues/detail?id=28
Comment 9 Nathan Froyd [:froydnj] 2012-05-09 07:21:46 PDT
Comment on attachment 622305 [details] [diff] [review]
Declare double conversion correctness for more architectures

Review of attachment 622305 [details] [diff] [review]:
-----------------------------------------------------------------

What, no defined(__vax__)? :)

The changes themselves are OK, but this needs to be done as a patch file that update.sh applies.
Comment 10 Mike Hommey [:glandium] 2012-05-18 05:24:42 PDT
Created attachment 625069 [details] [diff] [review]
Declare double conversion correctness for more architectures
Comment 11 Nathan Froyd [:froydnj] 2012-05-21 06:38:06 PDT
Comment on attachment 625069 [details] [diff] [review]
Declare double conversion correctness for more architectures

Review of attachment 625069 [details] [diff] [review]:
-----------------------------------------------------------------

Needs more-architectures.patch to be included.
Comment 12 Mike Hommey [:glandium] 2012-05-21 06:42:47 PDT
Created attachment 625628 [details] [diff] [review]
Declare double conversion correctness for more architectures
Comment 14 Ed Morley [:emorley] 2012-05-23 08:08:34 PDT
https://hg.mozilla.org/mozilla-central/rev/72e4dea9d2b2

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