If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Remove ARM Soft-Float support

NEW
Unassigned

Status

Tamarin
Baseline JIT (CodegenLIR)
P3
normal
7 years ago
5 years ago

People

(Reporter: Steven Johnson, Unassigned)

Tracking

unspecified
Q1 12 - Brannan
ARM
All
Bug Flags:
flashplayer-injection -
flashplayer-qrb +
flashplayer-bug -

Details

(Whiteboard: dead-code, has-patch)

Attachments

(2 attachments)

(Reporter)

Description

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

Updated

7 years ago
Whiteboard: Tracking

Comment 1

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

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

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

Updated

6 years ago
Flags: flashplayer-qrb+
Flags: flashplayer-injection-
Flags: flashplayer-bug-
Whiteboard: Tracking → dead-code
Target Milestone: --- → Q2 12 - Cyril

Comment 4

6 years ago
FPU is required to run Tamarin on ARM now.

Comment 5

6 years ago
can this land in brennan if someone does the work?

Updated

6 years ago
OS: Mac OS X → All
Hardware: x86 → ARM

Updated

6 years ago
Component: Virtual Machine → Baseline JIT (CodegenLIR)
QA Contact: vm → nanojit

Comment 6

6 years ago
(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

6 years ago
Retargeting for Brannan.
Priority: -- → P3
Target Milestone: Q2 12 - Cyril → Q1 12 - Brannan

Comment 8

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

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

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

"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 @ lxnay@gentoo.org"
(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.

Updated

6 years ago
Whiteboard: dead-code → dead-code, has-patch

Comment 12

6 years ago
Created attachment 607700 [details] [diff] [review]
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.
(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

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

6 years ago
What about http://www.raspberrypi.org/ which uses ARM11 processor (armv6 + vfp)?
You need to log in before you can comment on or make changes to this bug.