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

Status: RESOLVED FIXED

(Core :: XSLT, defect)

Description

[Oleg Romashin (:romaxa)]

Attachment: 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

Comment 1

[Oleg Romashin (:romaxa)]

Any progress?

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Have you tried the testcase in bug 53518 to see if it still passes on all affected platforms with this fix?

Comment 3

[Oleg Romashin (:romaxa)]

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 4

[Wan-Teh Chang]

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

[timeless]

[e-mail], am I right in guessing that bug 383607 is a more correct fix for this general problem?

Comment 6

[Oleg Romashin (:romaxa)]

Yes, for JS Part...

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

Comment 7

[timeless]

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

[Gavin Reaney]

(In reply to comment #5)
> [e-mail], 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

[Wan-Teh Chang]

(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

[Gavin Reaney]

(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.

Comment 11

[Oleg Romashin (:romaxa)]

Attachment: FPU_IS_ARM_FPA define for XSLT content txDouble.h

JS and NSS already have this fix, there only xslt...

Comment 12

[Reed Loden [:reed]]

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