708063 - IonMonkey: Increase the range of vldr and vstr

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Marty Rosenberg [:mjrosenb]]

Attachment: Miscellaneous fixes!

There are some other miscellaneous fixes + new features in here.
Notably, making OSR *not* bail out randomly

Comment 1

[Sean Stangl [:sstangl]]

Comment on attachment 579424 [details] [diff] [review]
Miscellaneous fixes!

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

Leaving the ARM changes to jbramley.

::: js/src/jsinterp.cpp
@@ +1965,5 @@
>          ion::MethodStatus status =
>              ion::CanEnterAtBranch(cx, script, regs.fp(), regs.pc);
>          if (status == ion::Method_Compiled) {
> +            interpReturnOK = ion::SideCannon(cx, regs.fp(), regs.pc);
> +            if (interpReturnOK)

Good catch.

Comment 2

[Jacob Bramley [:jbramley]]

Comment on attachment 579424 [details] [diff] [review]
Miscellaneous fixes!

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

::: js/src/ion/arm/Assembler-arm.cpp
@@ +529,5 @@
> +        DeferredData *deferred = data_[i];
> +        //Bind(code, deferred->label(), data + deferred->offset());
> +        deferred->copy(code, data + deferred->offset());
> +    }
> +

As discussed, change the comment to indiciate that this _is_ necessary, for switch statements.

::: js/src/ion/arm/MacroAssembler-arm.cpp
@@ +926,5 @@
> +MacroAssemblerARM::ma_vdtr(LoadStore ls, Operand &addr, FloatRegister rt, Condition cc)
> +{
> +    int off = addr.disp();
> +    if ((off & 3) != 0)
> +        JS_NOT_REACHED("non-multiple of 4 offsets not implemented.");

Why not just JS_ASSERT((off & 3) == 0) ?

@@ +939,5 @@
> +    int neg_bottom = (0x100 << 2) - bottom;
> +    // at this point, both off - bottom and off + neg_bottom will be reasonable-ish
> +    // quantities.
> +    if (off < 0) {
> +        Operand2 sub_off = Imm8(-(off-bottom)); // sub_off = bottom - off

This looks like it will break if off is 0x80000000. (It turns up in fuzz tests sometimes.)

@@ +1327,1 @@
>          ma_str(ScratchRegister, ToPayload(dest));

Reduce the indentation.