Closed Bug 858083 Opened 11 years ago Closed 11 years ago

IonMonkey: (ARM) ARMv6 has some issues with baseline

Categories

(Core :: JavaScript Engine, defect)

23 Branch
ARM
Android
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla23
Tracking Status
firefox22 --- unaffected
firefox23 --- fixed

People

(Reporter: mjrosenb, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression, topcrash, Whiteboard: [native-crash][startupcrash][ARMv6][fuzzblocker])

Crash Data

Attachments

(1 file)

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
Severity: normal → critical
Keywords: regression
OS: Linux → Android
Whiteboard: [native-crash][startupcrash][ARMv6]
Version: unspecified → 23 Branch
Where does baseline explicitly reference movw/movt instructions?  IIRC the MacroAssembler is used for everything, which should abstract over that..
This bug is the #1 topcrash for Nightly 23 on Android.
Keywords: topcrash
tracking-fennec: --- → ?
This doesn't fix BC completely on ARM, expect another bug shortly.
Attachment #733821 - Flags: review?(Jacob.Bramley)
Whiteboard: [native-crash][startupcrash][ARMv6] → [native-crash][startupcrash][ARMv6][fuzzblocker]
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.
Attachment #733821 - Flags: review?(Jacob.Bramley) → review+
https://hg.mozilla.org/mozilla-central/rev/6f83b4eedf82
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: