480796 - figure out whether the ARM chip has VFP or v6t2 instructions

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: dynamically detect vfp and v6t2

We need to do this dynamically; it's doable on both linux and Windows CE, though pretty ugly on linux.  Seems to work, though.

This removes all the #ifdef NJ_ARM_VFPs, because that code is fine to have; the softfloat filter gets rid of all the floating point instructions, so there's no reason to not have that code.

(Note that we don't yet use armv6t2 movt/movw yet, but now we can.)

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Er, pretend that fprintf isn't there.

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: updated patch; builds on top of series in 481761

Updated patch -- fixed to apply on top of the series in bug 481761.

Comment 3

[Andreas Gal :gal]

Comment on attachment 365785 [details] [diff] [review]
updated patch; builds on top of series in 481761

Sorry. Totally forgot about this patch in my queue.

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 365785 [details] [diff] [review]
updated patch; builds on top of series in 481761

Attached wrong (non-updated) patch.

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: updated

Updated; uses setjmp/longjmp correctly to get out of the sighandler.

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: updated, again

Updated and tested on real hardware.

Moved hardware flags to AvmCore::config, like sse2 and cmov, and moved detection to jstracer.  We do detection in the linux-approved way now, checking the hwcaps bits in /proc/self/auxv.

Comment 7

[Andreas Gal :gal]

Makefile changes need to be reviewed by someone else. Looks good otherwise. The formating changes are a bit unfortunate. It will make merging a lot harder.

Comment 8

[[e-mail] (:bhearsum)]

Comment on attachment 368150 [details] [diff] [review]
updated, again

You probably want ted or bsmedberg to review this instead of me.

Comment 9

[Edwin Smith]

Comment on attachment 368150 [details] [diff] [review]
updated, again

detection - cool

It is valuable to be able to hardcode whether or not to use soft float.  If config.soft_float is a static const, that might be enough, and still allow the ifdef elimination (which is also nice). 

thoughts?

Comment 10

[Edwin Smith]

Comment on attachment 368150 [details] [diff] [review]
updated, again

adding rick since he's integrating vfp support in tamarin as we speak

Comment 11

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #9)
> (From update of attachment 368150 [details] [diff] [review])
> detection - cool
> 
> It is valuable to be able to hardcode whether or not to use soft float.  If
> config.soft_float is a static const, that might be enough, and still allow the
> ifdef elimination (which is also nice). 

Ah, to eliminate the branches and the other vfp code paths at compile time?  How about adding a NJ_FORCE_SOFTFLOAT define that will flip that member to static const, and then that can be set if necessary?  I can do that before checking in.

Comment 12

[Rick Reitmaier]

Comment on attachment 368150 [details] [diff] [review]
updated, again

eek ; just landed a patch that will require re-basing, since I'm sure we will collide.

I like the idea of getting rid of the ifdefs for vfp/softfloat, leaving us the ability to make it dynamic or static (at compile time).  Sounds good.

I guess the only issue might be paths (e.g. switches) where we have to leave the code in there for both cases.  Not sure that we'll have that much bloat though, so it should be ok.

And compilers that are not savy enough to do the code elimination.  Do we have any data on this?

Some other notes on the patch:
(1) Can we hang  variable off of Assembler? 
(2) Looks like some debugging cruft remaining...see fprintf(stderr, "foo")

Comment 13

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

I've got an updated patch that's now stuck in my VM, since I sem to have forgotten to attach it -- will do so shortly.  It adds NJ_FORCE_SOFTFLOAT and NJ_FORCE_ARMV6T2 that turns the config variables into static consts.

(In reply to comment #12)
> (From update of attachment 368150 [details] [diff] [review])
> eek ; just landed a patch that will require re-basing, since I'm sure we will
> collide.
> 
> I like the idea of getting rid of the ifdefs for vfp/softfloat, leaving us the
> ability to make it dynamic or static (at compile time).  Sounds good.
> 
> I guess the only issue might be paths (e.g. switches) where we have to leave
> the code in there for both cases.  Not sure that we'll have that much bloat
> though, so it should be ok.
>
> And compilers that are not savy enough to do the code elimination.  Do we have
> any data on this?

Not sure, but I hope that all the ones in use are smart enough to do so or I think we have bigger problems :)  Even if we have a bunch of dead code, as long as the compiler can put in an unconditional branch over it, it shouldn't be that bad; I'd be surprised if there was more than few kb of code.

> Some other notes on the patch:
> (1) Can we hang  variable off of Assembler? 

Which variable?  I followed the same form as the current sse2 and other bits are in the config.

> (2) Looks like some debugging cruft remaining...see fprintf(stderr, "foo")

Huh whoops, I forgot to get rid of that.  Was trying to track down an assert, thanks for catching that.. will get rid of it.

Comment 14

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Checked in as http://hg.mozilla.org/tracemonkey/rev/a2752927811d

Comment 15

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/29c580815be5