750620 - Make double-conversion portable to exotic architectures

Status: RESOLVED FIXED

(Core :: MFBT, defect)

Description

[Landry Breuil (:gaston)]

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)]

Attachment: Add some archs

This adds ppc/alpha/sparc* but there probably should be some more...

Comment 2

[Mike Hommey [:glandium]]

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)]

Mike, any news ?

Comment 4

[Mike Hommey [:glandium]]

Attachment: 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]]

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]]

Do they use codereview for this or just issues on google code?

Comment 7

[Jeff Walden [:Waldo]]

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]]

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

Comment 9

[Nathan Froyd [:froydnj]]

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]]

Attachment: Declare double conversion correctness for more architectures

Comment 11

[Nathan Froyd [:froydnj]]

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]]

Attachment: Declare double conversion correctness for more architectures

Comment 13

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/72e4dea9d2b2

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/72e4dea9d2b2