Make double-conversion portable to exotic architectures

RESOLVED FIXED in mozilla15

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: gaston, Assigned: glandium)

Tracking

(Blocks 1 bug)

Trunk
mozilla15
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

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...
Depends on: 614888
Depends on: 614188
No longer depends on: 614888
Posted patch Add some archs (obsolete) — Splinter Review
This adds ppc/alpha/sparc* but there probably should be some more...
Assignee: nobody → landry
Attachment #619854 - Flags: review?(nfroyd)
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.
Attachment #619854 - Flags: review?(nfroyd)
Mike, any news ?
A few notes:
- _MIPS_ARCH_MIPS32R2 is not defined on Debian mips, so added __mips__
- __sparc__ is defined sparc64
- __s390__ is defined on s390x
Attachment #622305 - Flags: review?(nfroyd)
Attachment #619854 - Attachment is obsolete: true
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/
Do they use codereview for this or just issues on google code?
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 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.
Attachment #622305 - Flags: review?(nfroyd) → review-
Attachment #622305 - Attachment is obsolete: true
Assignee: landry → mh+mozilla
OS: OpenBSD → All
Hardware: PowerPC → All
Blocks: android-mips
No longer blocks: android-mips
Blocks: android-mips
No longer depends on: 614188
Depends on: 614188
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.
Attachment #625069 - Flags: review?(nfroyd) → review-
Attachment #625069 - Attachment is obsolete: true
Attachment #625628 - Flags: review?(nfroyd) → review+
https://hg.mozilla.org/mozilla-central/rev/72e4dea9d2b2
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.