792134 - Generate THUMB instead of ARM code on armv6

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

Our armv7 builds are currently built with -mthumb but our armv6 builds are not. We should build armv6 with -mthumb as well (and ensure the resulting code is smaller)

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Mozconfig

I started a build with the attached mozconfig (basically a copy of my usual mozconfig but for armv6 and with thumb enabled) and it died while building some of the JS code. I'll attach a build log output file.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Build log output file

Comment 3

[(not currently active) Ted Mielczarek]

Right, the main reason we didn't do this at the time was because we actually have a bunch of code that doesn't build as thumb-1.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Also, just for reference, I built fennec for ARMv7 with and without thumb enabled. The thumb APKs was only ~407 KiB smaller, but once I extracted the libraries inside there was a much larger size difference (with the lion's share going to a 16 MiB savings in libxul.so):

kats@kgupta-pc thumb$ (find . -type f | while read file; do SIZE=$(cat $file | wc -c); NFILE=$(echo $file | sed -e 's/-v7a//'); NSIZE=$(cat ../nothumb/$NFILE | wc -c); if [ $SIZE -ne $NSIZE ]; then echo "$((NSIZE - SIZE)) $file"; fi; done) | sort -rn
16434464 ./libxul.so
516092 ./libnss3.so
344064 ./libmozsqlite3.so
135168 ./libfreebl3.so
126664 ./lib/armeabi-v7a/libmozglue.so
103008 ./libssl3.so
102400 ./libsoftokn3.so
88324 ./libnspr4.so
65532 ./libsmime3.so
45056 ./libnssckbi.so
42344 ./libnssutil3.so
5976 ./lib/armeabi-v7a/libomxplugin.so
4440 ./libplc4.so
2800 ./libplds4.so
1828 ./libmozalloc.so
1760 ./lib/armeabi-v7a/libplugin-container.so
1144 ./libxpcom.so
5 ./omni.ja
-12 ./META-INF/MANIFEST.MF
-12 ./META-INF/CERT.SF

Comment 5

[(not currently active) Ted Mielczarek]

It's hard to draw comparisons there, since thumb1 and thumb2 are not the same. We'd have to invest some effort to make things compile with thumb1 to even test.

Comment 6

[Mike Hommey [:glandium]]

All ARMv7 support thumb-2, but not all ARMv6 support thumb-2. Random example: the HTC Magic has an ARM1136 ARMv6, which doesn't support thumb-2. These CPUs however do support thumb-1, and it has different size patterns compared to thumb-2. Also, the additional instructions available in armv7 and thumb-2 make for more compact code.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #5)
> It's hard to draw comparisons there, since thumb1 and thumb2 are not the
> same. We'd have to invest some effort to make things compile with thumb1 to
> even test.

Fair enough. I'll take a crack at doing this. I'll probably just disable stuff that doesn't build as thumb1 for a first cut. I assume there isn't so much of that stuff that it will make a significant difference.

Comment 8

[Mike Hommey [:glandium]]

I'm not even sure there are phones with ARM1156, supporting thumb-2.

Comment 9

[Mike Hommey [:glandium]]

Another note, on older cores thumb-1 may actually be slower than arm instructions.

Comment 10

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Mike Hommey [:glandium] from comment #9)
> Another note, on older cores thumb-1 may actually be slower than arm
> instructions.

Not just older cores, thumb-1 is slower in most cases AFAIK. Still worth the investigation though.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch to disable a bunch of thumb2-only code

I built with the attached patch with thumb and non-thumb. Diffs show that libxul is ~8.7 megs smaller with thumb.

kats@kgupta-pc armv6-thumb$ (find . -type f | while read file; do SIZE=$(cat $file | wc -c); NFILE=$(echo $file | sed -e 's/-v7a//'); NSIZE=$(cat ../armv6-nothumb/$NFILE | wc -c); if [ $SIZE -ne $NSIZE ]; then echo "$((NSIZE - SIZE)) $file"; fi; done) | sort -rn
8794032 ./libxul.so
315380 ./libnss3.so
209776 ./libmozsqlite3.so
61872 ./libssl3.so
61432 ./libsoftokn3.so
58728 ./lib/armeabi-v7a/libmozglue.so
50100 ./libnspr4.so
36844 ./libfreebl3.so
33220 ./libsmime3.so
28668 ./libnssckbi.so
23268 ./libnssutil3.so
2872 ./lib/armeabi-v7a/libomxplugin.so
2164 ./libplc4.so
1384 ./libmozalloc.so
1376 ./lib/armeabi-v7a/libplugin-container.so
1248 ./libplds4.so
1156 ./libxpcom.so
-10 ./omni.ja
-12 ./META-INF/MANIFEST.MF
-12 ./META-INF/CERT.SF

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch to provide fallbacks for thumb2 code

Here is a better patch that (I think) should provide reasonable fallback code paths for all of the thumb2 stuff. Compiles but completely untested.

Comment 13

[Kartikaya Gupta (email:[e-mail])]

Adding a see-also bug I split off for one of the issues I found while working on this, as it has some relevant discussion that I don't want to lose a pointer to.

Comment 14

[Kartikaya Gupta (email:[e-mail])]

We're dropping support for armv6.