IonMonkey: Increase the range of vldr and vstr

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mjrosenb, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
Created attachment 579424 [details] [diff] [review]
Miscellaneous fixes!

There are some other miscellaneous fixes + new features in here.
Notably, making OSR *not* bail out randomly
Attachment #579424 - Flags: review?(dvander)
Attachment #579424 - Flags: review?(Jacob.Bramley)
(Reporter)

Updated

6 years ago
Attachment #579424 - Flags: review?(dvander) → review?(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.
Attachment #579424 - Flags: review?(sstangl) → review+

Updated

6 years ago
Blocks: 701993
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.
Attachment #579424 - Flags: review?(Jacob.Bramley) → review+
(Reporter)

Updated

6 years ago
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.