Make double-conversion portable to exotic architectures

RESOLVED FIXED in mozilla15

Status

()

Core
MFBT
RESOLVED FIXED
5 years ago
5 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)

(Reporter)

Description

5 years ago
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...
(Reporter)

Updated

5 years ago
Depends on: 614888
(Reporter)

Updated

5 years ago
Depends on: 614188
No longer depends on: 614888
(Reporter)

Comment 1

5 years ago
Created attachment 619854 [details] [diff] [review]
Add some archs

This adds ppc/alpha/sparc* but there probably should be some more...
Assignee: nobody → landry
Attachment #619854 - Flags: review?(nfroyd)
(Assignee)

Comment 2

5 years ago
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)
(Reporter)

Comment 3

5 years ago
Mike, any news ?
(Assignee)

Comment 4

5 years ago
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
Attachment #622305 - Flags: review?(nfroyd)
(Assignee)

Updated

5 years ago
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/
(Assignee)

Comment 6

5 years ago
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.
(Assignee)

Comment 8

5 years ago
Filed
http://code.google.com/p/double-conversion/issues/detail?id=28
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-
(Assignee)

Comment 10

5 years ago
Created attachment 625069 [details] [diff] [review]
Declare double conversion correctness for more architectures
Attachment #625069 - Flags: review?(nfroyd)
(Assignee)

Updated

5 years ago
Attachment #622305 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Assignee: landry → mh+mozilla
(Assignee)

Updated

5 years ago
OS: OpenBSD → All
Hardware: PowerPC → All
(Assignee)

Updated

5 years ago
Blocks: 734309
(Assignee)

Updated

5 years ago
No longer blocks: 734309
(Assignee)

Updated

5 years ago
Blocks: 734309
(Assignee)

Updated

5 years ago
No longer depends on: 614188
(Assignee)

Updated

5 years ago
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-
(Assignee)

Comment 12

5 years ago
Created attachment 625628 [details] [diff] [review]
Declare double conversion correctness for more architectures
Attachment #625628 - Flags: review?(nfroyd)
(Assignee)

Updated

5 years ago
Attachment #625069 - Attachment is obsolete: true
Attachment #625628 - Flags: review?(nfroyd) → review+
(Assignee)

Comment 13

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/72e4dea9d2b2
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/72e4dea9d2b2
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.