Closed
Bug 750620
Opened 12 years ago
Closed 12 years ago
Make double-conversion portable to exotic architectures
Categories
(Core :: MFBT, defect)
Core
MFBT
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: gaston, Assigned: glandium)
References
Details
Attachments
(1 file, 3 obsolete files)
3.27 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
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•12 years ago
|
Reporter | ||
Comment 1•12 years ago
|
||
This adds ppc/alpha/sparc* but there probably should be some more...
Assignee: nobody → landry
Attachment #619854 -
Flags: review?(nfroyd)
Assignee | ||
Comment 2•12 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•12 years ago
|
||
Mike, any news ?
Assignee | ||
Comment 4•12 years ago
|
||
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•12 years ago
|
Attachment #619854 -
Attachment is obsolete: true
Comment 5•12 years ago
|
||
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•12 years ago
|
||
Do they use codereview for this or just issues on google code?
Comment 7•12 years ago
|
||
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•12 years ago
|
||
Filed http://code.google.com/p/double-conversion/issues/detail?id=28
Comment 9•12 years ago
|
||
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•12 years ago
|
||
Attachment #625069 -
Flags: review?(nfroyd)
Assignee | ||
Updated•12 years ago
|
Attachment #622305 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Assignee: landry → mh+mozilla
Assignee | ||
Updated•12 years ago
|
OS: OpenBSD → All
Hardware: PowerPC → All
Assignee | ||
Updated•12 years ago
|
Blocks: android-mips
Assignee | ||
Updated•12 years ago
|
No longer blocks: android-mips
Assignee | ||
Updated•12 years ago
|
Blocks: android-mips
Comment 11•12 years ago
|
||
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•12 years ago
|
||
Attachment #625628 -
Flags: review?(nfroyd)
Assignee | ||
Updated•12 years ago
|
Attachment #625069 -
Attachment is obsolete: true
Updated•12 years ago
|
Attachment #625628 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 13•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/72e4dea9d2b2
Target Milestone: --- → mozilla15
Comment 14•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/72e4dea9d2b2
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•