IonMonkey: GETELEM: Fast path for typed arrays

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Comment 1

6 years ago
Created attachment 607885 [details] [diff] [review]
Patch

The patch is mostly modeled after the dense array instructions and the JM fast paths. Some notes:

- The *Hole instruction uses an OOL VM call to handle the out-of-bounds case. This should be very uncommon since typed arrays don't have an "initialized length" < length case. Better name welcome, the *Hole suffix matches the dense array *Hole instruction but typed arrays don't have holes - I couldn't think of a better name (with the same length) though.

- Uint32Array is the most complicated array type, because reading from it can result in either an int32 or double value. If the instruction is typed as int32, and we read a double, we have to bailout. Furthermore, we need a temp register for convertUint32ToDouble.

- x86/x64 only for now, Marty is willing to add the ARM macro-assembler methods we need. I will add some temporary #ifdef's to make sure the code compiles on ARM in the meantime.
Attachment #607885 - Flags: review?(dvander)
Comment on attachment 607885 [details] [diff] [review]
Patch

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

Nice!

::: js/src/ion/shared/Assembler-shared.h
@@ +59,5 @@
> +ScaleFromShift(int shift)
> +{
> +    switch (shift) {
> +      case 1:
> +       return TimesOne;

nit: misindent

::: js/src/ion/shared/Assembler-x86-shared.h
@@ +274,5 @@
> +    }
> +    void cvtss2sd(const FloatRegister &src, const FloatRegister &dest) {
> +        masm.cvtss2sd_rr(src.code(), dest.code());
> +    }
> +    void movzbl(const Operand &src, const Register &dest) {

nit: Yesterday I renamed load16 to movzxh or something. I like your convention better, so feel free to rename it.
Attachment #607885 - Flags: review?(dvander) → review+
(Assignee)

Comment 3

6 years ago
Pushed with nits fixed (I used the same names as JSC uses for the instructions):

https://hg.mozilla.org/projects/ionmonkey/rev/b5f9937c5d5c
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.