Closed Bug 708063 Opened 10 years ago Closed 10 years ago

IonMonkey: Increase the range of vldr and vstr

Categories

(Core :: JavaScript Engine, defect)

ARM
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: mjrosenb, Unassigned)

References

Details

Attachments

(1 file)

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)
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+
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+
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.