Rockbox

Tasklist

FS#11335 - Make ARM assembly functions thumb-friendly

Attached to Project: Rockbox
Opened by Rafaël Carré (funman) - Sunday, 30 May 2010, 21:37 GMT
Last edited by Rafaël Carré (funman) - Friday, 11 June 2010, 04:42 GMT
Task Type Patches
Category Battery/Charging
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Release 3.4
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

A lot of functions use return instructions which won't set the T bit if the caller was running in thumb:

- ldm reg, { ..., pc } works on armv5 but not armv4t

- ldr pc, function works on ARMv5 and +

- add/sub/bic/orr/mov pc, lr works on armv7 but not before

reference is http://webcache.googleusercontent.com/search?q=cache:saLD8BqZXhQJ:www.riscosopen.org/wiki/documentation/pages/ARMv7%2Bcompatibility%2Bprimer+thumb+interwork+ldm+ldr+armv7&cd=1&hl=fr&ct=clnk

DDI0100E (ARMv5 architecture reference manual) says of mov:
If <Rd> is R15, the SPSR of the current mode is copied to the CPSR


This patch changes all occurences of these instructions which could be run on armv4t (e.g. PP, AMSv1, ..).

I don't think I have missed any, I used grep -E "(((ldr|mov) *)|(ldm.*))pc" to find all the occurences.

I could have changed some code which would not run on ARMv4 though.



Cost of this change on ldm (the most used)
- 4 more bytes per return
- 1 more cycle on ARM7TDMI, no effect on newer CPUs



See  FS#6734  for how to build thumb code
This task depends upon

Closed by  Rafaël Carré (funman)
Friday, 11 June 2010, 04:42 GMT
Reason for closing:  Accepted
Additional comments about closing:  r26756
Comment by Rafaël Carré (funman) - Sunday, 30 May 2010, 21:38 GMT
I didn't change any crt0.S

I think we can return from vectors with anything, the spsr will be restored with the T bit (must check)
Comment by Rafaël Carré (funman) - Sunday, 30 May 2010, 22:03 GMT
Exceptions which return already take care of restoring CPSR from SPSR so the T bit stays set

e200v1 built with -mthumb still doesn't boot though
Comment by Rafaël Carré (funman) - Sunday, 30 May 2010, 22:07 GMT
Tested OK on fuzev1 (armv4) and clipv2 (armv5)
Comment by Rafaël Carré (funman) - Monday, 31 May 2010, 14:43 GMT
All files checked
Modify some comments
Remove unrelated diffs

% grep -rE "(((mov|ldr) *)|(ldm.*))pc" *|cut -d: -f1|sort|uniq

apps/codecs/demac/libdemac/udiv32_arm.S # ARMv5+
apps/codecs/libtremor/mapping0.c # false positive (ldmia %[pcm])
apps/dsp_arm_v6.S # ARMv6+
apps/plugins/mpegplayer/idct_armv6.S # ARMv6+
apps/plugins/mpegplayer/motion_comp_arm_s.S # branches to local ARM functions
apps/recorder/jpeg_idct_arm.S # ARMv5+

firmware/target/arm/as3525/sansa-clipplus/lcd-as-clip-plus.S # ARMv5
firmware/target/arm/as3525/sansa-clipv2/lcd-as-clipv2.S # ARMv5

# crt0 ignored: entered in ARM state, returning from exceptions restore SPSR into CPSR
firmware/target/arm/at91sam/lyre_proto1/crt0.S
firmware/target/arm/crt0-pp-bl.S
firmware/target/arm/crt0-pp.S
firmware/target/arm/crt0.S
firmware/target/arm/imx31/crt0.S
firmware/target/arm/pnx0101/crt0-pnx0101.S
firmware/target/arm/s3c2440/crt0.S
firmware/target/arm/s5l8700/crt0.S
firmware/target/arm/tcc77x/crt0.S
firmware/target/arm/tcc780x/crt0.S

firmware/target/arm/support-arm.S # ARMv5+
firmware/target/arm/s3c2440/system-s3c2440.c # return from exception (irq), restore SPSR into CPSR

# tools and bootloaders ignored

rbutil/mkamsboot/dualboot/dualboot.S
rbutil/mkamsboot/dualboot/nrv2e_d8.S
rbutil/mktccboot/mktccboot.c

bootloader/creativezvm.c
bootloader/ipodnano2g.c

Still not booting on e200v1 (built with -mthumb) - not sure why
Comment by Rafaël Carré (funman) - Saturday, 05 June 2010, 17:23 GMT
sync to r26588
Comment by Rafaël Carré (funman) - Saturday, 05 June 2010, 21:46 GMT
problem on PP is a problem with the linker, i think this patch could go in

i'll make some test_codec run to confirm it hasn't any noticeable impact
Comment by Rafaël Carré (funman) - Wednesday, 09 June 2010, 12:36 GMT
Results on fuzev1 built with eabi: last column is the % of time needed to decode with the patch, compared to without the patch

| *Nero AAC-HE* |||||
| 64kaache.m4a | 164.17% realtime | Decode time - 107.21s | 151.06MHz | 100.85%|
| *Cook (RA)* |||||
| cook_mono_64.ra | 1306.83% realtime | Decode time - 13.46s | 18.97MHz | 100.00%|
| cook_stereo_32.ra | 1417.40% realtime | Decode time - 12.41s | 17.49MHz | 100.00%|
| cook_stereo_64.ra | 694.15% realtime | Decode time - 25.34s | 35.72MHz | 99.97%|
| cook_stereo_96.ra | 678.36% realtime | Decode time - 25.93s | 36.55MHz | 100.00%|
| *MP3* |||||
| lame_096.mp3 | 833.53% realtime | Decode time - 21.11s | 29.75MHz | 100.23%|
| lame_128.mp3 | 637.30% realtime | Decode time - 27.61s | 38.91MHz | 100.26%|
| *Musepack* |||||
| mpc_096.mpc | 903.90% realtime | Decode time - 19.46s | 27.43MHz | 100.00%|
| mpc_128.mpc | 860.98% realtime | Decode time - 20.43s | 28.80MHz | 99.97%|
| *AAC-LC* |||||
| nero_096.m4a | 571.11% realtime | Decode time - 30.81s | 43.42MHz | - |
| nero_128.m4a | 550.21% realtime | Decode time - 31.98s | 45.07MHz | - |
| *Vorbis* |||||
| vorbis_096.ogg | 658.55% realtime | Decode time - 26.71s | 37.65MHz | - |
| *WMA Standard* |||||
| wma_128.wma | 652.33% realtime | Decode time - 27.21s | 38.01MHz | - |
| wma_96.wma | 659.60% realtime | Decode time - 26.91s | 37.59MHz | - |
Comment by Rafaël Carré (funman) - Friday, 11 June 2010, 04:29 GMT
Replace ldm/ldr which pop pc by ldrpc/ldmpc macros

Loading...