Closed Bug 369722 Opened 13 years ago Closed 12 years ago
.c jsnum .h tx Double .h not required IEEE _ARM define on Codesourcery EABI gcc3 .4 .4 compiler
4.54 KB, patch
|Details | Diff | Splinter Review|
1.75 KB, patch
|Details | Diff | Splinter Review|
Codesourcery 3.4.4 compiler use in Maemo platform. Gecko have special defines in nspr, js, xslt for ARM compiler. ......... #if defined(__arm) || defined(__arm32__) || defined(__arm26__) || defined(__arm__) #define CPU_IS_ARM ........ See Bug 106864, Bug 209814, Bug 278981. But for Codesourcery gcc3.4.4 ARM and gcc4.x.x that defines should be disabled, because they have EABI... See patch in attachments Also there are fix for wrong define name CPU_IS_ARM -> IEEE_ARM
Attachment #254407 - Flags: review?(darin.moz)
Summary: prdtoa.c jsnum.h txDouble.h not required CPU_IS_ARM define on Codesourcery EABI gcc3.4.4 compiler → prdtoa.c jsnum.h txDouble.h not required IEEE_ARM define on Codesourcery EABI gcc3.4.4 compiler
Attachment #254407 - Flags: review?(shaver) → review+
Have you tried the testcase in bug 53518 to see if it still passes on all affected platforms with this fix?
This my patch will affected only for ARM gnueabi platform. I have checked last testcase from bug 53518 https://bugzilla.mozilla.org/attachment.cgi?id=137614 and it pass all cases.
Comment on attachment 254407 [details] [diff] [review] Probably this is the fix. I checked in the NSPR (prdtoa.c) change in this patch on the NSPR trunk (NSPR 4.7) and the NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla trunk, Gecko 1.9 pre-release).
email@example.com, am I right in guessing that bug 383607 is a more correct fix for this general problem?
Yes, for JS Part... There are still XSLT part which not fixed yet. txDouble.h
sure, but I'd note that if their fix is more correct, then a patch that uses it would be better than me landing a less correct patch.
(In reply to comment #5) > firstname.lastname@example.org, am I right in guessing that bug 383607 is a more correct fix > for this general problem? > I think so. The approach in bug 383607 allows the floating point model to be detected independently of the other aspects of the ABI (some older compilers certainly allow the FP model to be set to either FPA or VFP). The only slight snag in bug 383607 is that different toolchains tend to use different macro names to indicate the floating point model, so it may be necessary to add additional checks for a particular toolchain (__VFP_FP__ works for gcc-arm and maybe others). If __ARM_EABI__ always implies VFP (i.e. using FPA is impossible) then it would be OK to add an __ARM_EABI__ check to the existing #ifdef __VFP_FP__ block in jsnum.h. [I haven't had a chance to check this yet.] That approach should be fine to use throughout the other header files too. Since ARM supports two floating point models (VFP and FPA), the macro name IEEE_ARM is slightly inaccurate so I wouldn't complain if it was changed.
(In reply to comment #8) > > Since ARM supports two floating point models (VFP and FPA), the macro name > IEEE_ARM is slightly inaccurate so I wouldn't complain if it was changed. Gavin, could you suggest a better name for the IEEE_ARM macro? Are we using the VFP or FPA model when IEEE_ARM is defined?
(In reply to comment #9) > (In reply to comment #8) > > > > Since ARM supports two floating point models (VFP and FPA), the macro name > > IEEE_ARM is slightly inaccurate so I wouldn't complain if it was changed. > > Gavin, could you suggest a better name for the IEEE_ARM macro? > Are we using the VFP or FPA model when IEEE_ARM is defined? > For the JS engine the name 'FPU_IS_ARM_FPA' is used. IEEE_ARM corresponds to the FPA model.
JS and NSS already have this fix, there only xslt...
Assignee: wtc → nobody
Component: NSPR → XSLT
Product: NSPR → Core
QA Contact: nspr → xslt
Version: other → Trunk
Checking in content/xslt/public/txDouble.h; /cvsroot/mozilla/content/xslt/public/txDouble.h,v <-- txDouble.h new revision: 1.3; previous revision: 1.2 done
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M11
You need to log in before you can comment on or make changes to this bug.