prdtoa.c jsnum.h txDouble.h not required IEEE_ARM define on Codesourcery EABI gcc3.4.4 compiler

RESOLVED FIXED in mozilla1.9beta3

Status

()

Core
XSLT
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: romaxa, Assigned: romaxa)

Tracking

Trunk
mozilla1.9beta3
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

11 years ago
Created attachment 254407 [details] [diff] [review]
Probably this is the fix.

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)
(Assignee)

Updated

11 years ago
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

Updated

11 years ago
Attachment #254407 - Flags: review?(darin.moz) → review+
(Assignee)

Updated

11 years ago
Attachment #254407 - Flags: review?(shaver)
(Assignee)

Updated

11 years ago
Attachment #254407 - Flags: review?(peterv)

Updated

11 years ago
Attachment #254407 - Flags: review?(shaver) → review+

Updated

11 years ago
Attachment #254407 - Flags: review?(jonas)
(Assignee)

Comment 1

11 years ago
Any progress?
Have you tried the testcase in bug 53518 to see if it still passes on all affected platforms with this fix?
(Assignee)

Comment 3

11 years ago
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.

Attachment #254407 - Flags: review?(peterv)
Attachment #254407 - Flags: review?(jonas)
Attachment #254407 - Flags: review+

Comment 4

11 years ago
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).

Comment 5

11 years ago
gavin@picsel.com, am I right in guessing that bug 383607 is a more correct fix for this general problem?
(Assignee)

Comment 6

11 years ago
Yes, for JS Part...

There are still XSLT part which not fixed yet.
txDouble.h

Comment 7

11 years ago
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.

Comment 8

11 years ago
(In reply to comment #5)
> gavin@picsel.com, 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.

Comment 9

11 years ago
(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?

Comment 10

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

Comment 11

10 years ago
Created attachment 286146 [details] [diff] [review]
FPU_IS_ARM_FPA define for XSLT content txDouble.h

JS and NSS already have this fix, there only xslt...
Attachment #286146 - Flags: review?(jonas)
Attachment #286146 - Flags: superreview+
Attachment #286146 - Flags: review?(jonas)
Attachment #286146 - Flags: review+
Assignee: wtc → nobody
Component: NSPR → XSLT
Product: NSPR → Core
QA Contact: nspr → xslt
Version: other → Trunk
Assignee: nobody → romaxa
Attachment #286146 - Flags: approval1.9?

Updated

10 years ago
Attachment #286146 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
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
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M11
You need to log in before you can comment on or make changes to this bug.