543401 - nanojit: merge SoftFloatFilter implementations

Status: RESOLVED FIXED

(Core Graveyard :: Nanojit, defect)

Description

[Nicholas Nethercote [inactive]]

TM and TR have their own versions of SoftFloatFilter, but the differences between them are superficial.  A single version should live in NJ.  This version could then easily be used by lirasm, which would allow SoftFloat to be tested from lirasm.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: NJ patch

This patch adds SoftFloatFilter into NJ.  The added version is a mixture of what I thought were the best features of the two SoftFloatFilters in TM and TR (but there weren't many differences).  I also changed lirasm to run the SoftFloatFilter if arm_vfp is false, which is necessary for it to be tested properly.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: TM patch

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: TR patch

Comment 4

[Andreas Gal :gal]

Steven, I think I rewrote the code to match what TR does a while back, but watch out for subtle differences when you review this.

Comment 5

[Steven Johnson]

Comment on attachment 424683 [details] [diff] [review]
NJ patch

Disclaimer: I've reviewed the code but not yet actually integrated & tested it on TR running on ARM hardware. That would be nice to do before landing, IMHO. That said, approved with a few quibbles:

Weren't we going to add something like NJ_SOFTFLOAT_SUPPORTED so that platforms that can't use softfloat don't compile in the relevant support code? No point in including can't-possibly-be-used code on (say) x86...

>    static double FASTCALL fneg(double a)           { return -a; }

AFAIK, FASTCALL only applies to integer arguments, so this is pointless, but harmless, I guess.

>             if ((op >= LIR_feq && op <= LIR_fge))

Time to add is_fcmp(), methinks, but that's scope creep...


> LIR.h:     extern const SoftFloatOps softFloatOps;
> LIR.cpp:   const SoftFloatOps softFloatOps;

any reason this shouldn't be static to LIR.cpp?

Comment 6

[Nicholas Nethercote [inactive]]

(In reply to comment #5)
> Weren't we going to add something like NJ_SOFTFLOAT_SUPPORTED so that platforms
> that can't use softfloat don't compile in the relevant support code? No point
> in including can't-possibly-be-used code on (say) x86...

That's bug 542326.  It will conflict a bit with this one, but I can resolve the conflicts when the first one lands.

> >    static double FASTCALL fneg(double a)           { return -a; }
> 
> AFAIK, FASTCALL only applies to integer arguments, so this is pointless, but
> harmless, I guess.

It's useful for i2f/u2f, and it's easier (due to the SF_CALLINFO macro) if they all use the same calling convention.

> >             if ((op >= LIR_feq && op <= LIR_fge))
> 
> Time to add is_fcmp(), methinks, but that's scope creep...

That's in bug 542932.

> > LIR.h:     extern const SoftFloatOps softFloatOps;
> > LIR.cpp:   const SoftFloatOps softFloatOps;
> 
> any reason this shouldn't be static to LIR.cpp?

It's used in TM's jstracer.cpp.

Comment 7

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/projects/nanojit-central/rev/485ae0bec97b

Comment 8

[Steven Johnson]

http://hg.mozilla.org/tamarin-redux/rev/3b058845fae6
http://hg.mozilla.org/tamarin-redux/rev/d248dab11ddc

Comment 9

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/tracemonkey/rev/fb8b00527584
http://hg.mozilla.org/tracemonkey/rev/d30ae27488ca

Comment 10

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/d30ae27488ca