608803 - Remove ARM Soft-Float support

Status: RESOLVED WONTFIX

(Tamarin Graveyard :: Baseline JIT (CodegenLIR), defect, P3)

Description

[Steven Johnson]

ARM softfloat support has never shipped in any Tamarin or TraceMonkey product, and is questionable as to whether it currently works or not, and is not planned for any future product. It should be removed.

Comment 1

[Werner Sharp]

Because I like to annoy Steven, I'll mention that of course it works.  Android's default build config has been ARMv5 (forever?) and that exercises all the SoftFloat code.  

Not that I'm against removing it, but maybe it's better to just let it rot away?  In a year, if we needed it for some obscure reason, which is a better alternative...try to get it working from what is there or go back from year old code and add it back?

Comment 2

[Edwin Smith]

(In reply to comment #1)
> but maybe it's better to just let it rot away?

If we keep it, we test it and keep it working; otherwise remove it -- dead rotting code just makes the codebase sucky, better to get rid of it.

> which is a better try to get it working from what is there or go back
> from year old code and add it back?

Adding it back seems like a better option to me.  In the mean time we will have benefitted from a less constrained codebase and fewer oddball platforms to test, which probably adds up to more than the time it would take to add it back.  Adding it back may result in something simpler, not having the historical baggage that the current code does).

Comment 3

[Trevor Baker]

Not only _should_ it be working on android, we also have a softfloat config in testing on arm-linux.

ie.  http://tamarin-builds.mozilla.org/tamarin-redux/builders/linux-arm2-test/builds/32/steps/Testsuite_Release-softfloat/logs/stdio

Comment 4

[Dan Smith]

FPU is required to run Tamarin on ARM now.

Comment 5

[Edwin Smith]

can this land in brennan if someone does the work?

Comment 6

[Dan Smith]

(In reply to Edwin Smith from comment #5)
> can this land in brennan if someone does the work?

Checking with product teams and product mgmt.  I think it can go in Brannan  but awaiting confirmation.

Comment 7

[Dan Smith]

Retargeting for Brannan.

Comment 8

[Edwin Smith]

arm cpu decision logic is roughly:

  soft_float = !arm_vfp
  arm_vfp = arm_arch >= 7

ergo, removing softfloat means all support for <v7 can be removed.  that's potentially a lot of code (good) but better to measure twice, cut once.  

so - again, confirmation - ok to remove all v4, v5, v6-specific ARM code?

Comment 9

[Lars T Hansen]

I've asked for verification that ARMv7-A is the minimum requirement, and Thibault says he'll try to obtain that.  (It's what I've heard previously.)

Comment 10

[Jory A. Pratt [:anarchy]]

Attachment: properly support arms in spidermonkey

This will atleast fix the javascript side of things. Here is a quote of an arms developer ..

"First of all armv7 has hardware floating point unit and forcing
-float-abi=softfp would make gcc fail miserably on a hardfp (hardware floating
point) system.
Secondly, at least for armv7 (but i also guess it's the same for other arm
arches), TARGET_CPU is something like "armv7" and not just "arm", so we really
need to use findstring rather than doing exact matching.
Failing to do so has the consequence of some source files not being included
for compilation, resulting in linking failure (undefined symbols). -Fabio Erculiani @ [e-mail]"

Comment 11

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to Jory A. Pratt from comment #10)
> Created attachment 592800 [details] [diff] [review]
> properly support arms in spidermonkey
> 
> This will atleast fix the javascript side of things. Here is a quote of an
> arms developer ..

Jory: FYI: this is not the correct bug to post this patch.  The Tamarin project does not include the files in your patch.

Comment 12

[Edwin Smith]

Attachment: WIP

This patch was made before float/float4 work, so needs to be redone.  The idea is to require ARMv7 at vm-build-time, and strip out all ARM softfloat support.

Comment 13

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to Edwin Smith from comment #12)
> [...] The
> idea is to require ARMv7 at vm-build-time, and strip out all ARM softfloat
> support.

I never remember the in's and out's of the different ARM systems' handling of floating point; I just wanted to make sure we either continue to support Tegra2, or formally drop support for it.  See also Bug 696380.  We have gotten burned by failing to internally test Tegra2 targets in the recent past.

(Note that I do not know if stripping out all ARM softfloat support actually implies dropping Tegra2.)

Comment 14

[Edwin Smith]

Yep. This can't land until we have buyin that we only support armv7, which implies vfp. 

The tegra2 snag is this: it's armv7, and vfp, but not NEON. 

Neon is the SIMD instruction set. So supporting tegra2 and float4 requires finesse but basic hardware floating point support is there.

Comment 15

[Siarhei Siamashka]

What about http://www.raspberrypi.org/ which uses ARM11 processor (armv6 + vfp)?

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Tamarin is a dead project now. Mass WONTFIX.

Comment 17

[Sylvestre Ledru [:Sylvestre]]

Tamarin isn't maintained anymore. WONTFIX remaining bugs.