858083 - IonMonkey: (ARM) ARMv6 has some issues with baseline

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Marty Rosenberg [:mjrosenb]]

The baseline compiler currently explicitly uses MOVW/MOVT, which is problematic because armv6 doesn't have these instructions.  Due to a minor clerical error on my part, most, if not all armv7 builds think they are armv6 for purposes of asserting if they can generate movw/movt.
STR
echo 'print("hello world");' | ./js --ion-eager

Comment 1

[Robert Kaiser]

see https://crash-stats.mozilla.com/report/list?signature=EnterBaseline&product=FennecAndroid

Comment 2

[Kannan Vijayan [:djvj]]

Where does baseline explicitly reference movw/movt instructions?  IIRC the MacroAssembler is used for everything, which should abstract over that..

Comment 3

[Chris Peterson [:cpeterson]]

This bug is the #1 topcrash for Nightly 23 on Android.

Comment 4

[Marty Rosenberg [:mjrosenb]]

Attachment: /home/mjrosenb/patches/fixBC_ARMv6-r0.patch

This doesn't fix BC completely on ARM, expect another bug shortly.

Comment 5

[Jacob Bramley [:jbramley]]

Comment on attachment 733821 [details] [diff] [review]
/home/mjrosenb/patches/fixBC_ARMv6-r0.patch

Review of attachment 733821 [details] [diff] [review]:
-----------------------------------------------------------------

Minor changes only, as discussed.

::: js/src/ion/arm/MacroAssembler-arm.cpp
@@ +3268,2 @@
>      addPendingJump(bo, target->raw(), Relocation::IONCODE);
> +    ma_movPatchable(Imm32(uint32_t(target->raw())), ScratchRegister, Always, L_LDR);

As discussed, this should use MOVW/T on ARMv7.

::: js/src/ion/arm/MacroAssembler-arm.h
@@ +563,5 @@
>      // this instruction.
>      CodeOffsetLabel toggledCall(IonCode *target, bool enabled);
>  
>      static size_t ToggledCallSize() {
>          // Size of a movw, movt, nop/blx instruction.

The comment needs to be updated to match.

@@ +564,5 @@
>      CodeOffsetLabel toggledCall(IonCode *target, bool enabled);
>  
>      static size_t ToggledCallSize() {
>          // Size of a movw, movt, nop/blx instruction.
> +        return 8;

As discussed, this size will change for ARMv7, where MOVW/MOVT is used.

Comment 6

[Marty Rosenberg [:mjrosenb]]

landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/6f83b4eedf82

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6f83b4eedf82