1687441 - Support large array buffers in the JITs

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, task, P3)

Description

[Jan de Mooij [:jandem]]

With the patches in bug 1686936, the remaining places where we don't support large ArrayBuffers are mostly JSAPIs and JIT code.

JIT support isn't trivial because we will need to do 64-bit bounds checks and sign-extend 32-bit indexes for that. I hope this will be acceptable perf-wise, else we could do this in Warp code only in zones that have created large ArrayBuffer views.

Comment 1

[Jan de Mooij [:jandem]]

I have a working prototype for this that passes all jit-tests. I'm pretty happy with how it all turned out.

Needs more work and testing, but I'm going to start splitting it up and land preliminary pieces.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 1 - Add convertDoubleToPtr to the MacroAssembler. r?anba!

Based on convertDoubleToInt32.

On 32-bit platforms this is equivalent to convertDoubleToInt32 which will be
sufficient because the length limit doesn't change there. On 64-bit platforms
this will let us support large index values.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 2 - Add move32SignExtendToPtr to the MacroAssembler. r?anba!

Depends on D102426

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 3 - Add spectreBoundsCheckPtr to the MacroAssembler. r?anba!

Based on spectreBoundsCheck32.

Depends on D102427

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 4 - Templatize branchAddPtr and branchSubPtr. r?anba!

This matches branchAdd32 and branchSub32.

We will use branchSubPtr with Register and Imm32 in EmitDataViewBoundsCheck.

Depends on D102428

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 5 - Rename spectreMaskIndex to spectreMaskIndex32. r?anba!

The next patch will add spectreMaskIndexPtr.

Depends on D102429

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 6 - Add spectreMaskIndexPtr to the MacroAssembler. r?anba!

Baesd on spectreMaskIndex32 and cmp32Move32.

Depends on D102430

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f2bc0012c45
part 1 - Add convertDoubleToPtr to the MacroAssembler. r=anba
https://hg.mozilla.org/integration/autoland/rev/4c037df6bd90
part 2 - Add move32SignExtendToPtr to the MacroAssembler. r=anba
https://hg.mozilla.org/integration/autoland/rev/ca92ee4304ac
part 3 - Add spectreBoundsCheckPtr to the MacroAssembler. r=anba
https://hg.mozilla.org/integration/autoland/rev/d034802fb52f
part 4 - Templatize branchAddPtr and branchSubPtr. r=anba
https://hg.mozilla.org/integration/autoland/rev/52db39728006
part 5 - Rename spectreMaskIndex to spectreMaskIndex32. r=anba
https://hg.mozilla.org/integration/autoland/rev/609249cb8b2c
part 6 - Add spectreMaskIndexPtr to the MacroAssembler. r=anba

Comment 9

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/7f2bc0012c45
https://hg.mozilla.org/mozilla-central/rev/4c037df6bd90
https://hg.mozilla.org/mozilla-central/rev/ca92ee4304ac
https://hg.mozilla.org/mozilla-central/rev/d034802fb52f
https://hg.mozilla.org/mozilla-central/rev/52db39728006
https://hg.mozilla.org/mozilla-central/rev/609249cb8b2c

Comment 10

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 7 - Add NumberEqualsInt64 and NumberIsInt64 to MFBT. r?sg!

Tests based on the Int32 tests, with changes/tests for very large/small values.

Comment 11

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 8 - Don't use a type policy for the index argument for atomics. r?anba!

This matches what we do for similar cases like MLoadUnboxedScalar.

A later patch will change the operand type to MIRType::IntPtr and that's easier
if there's no type policy to worry about.

Depends on D102724

Comment 12

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 9 - Add MIRType::IntPtr. r?anba!

This is the type that will be used to represent indexes/offsets into TypedArray
and DataView objects.

Depends on D102725

Comment 13

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 10 - Add MCompare::Compare_UIntPtr. r?anba!

This will be used later for transpiling LoadTypedArrayElementExistsResult.

Depends on D102726

Comment 14

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 11 - Add MAdjustDataViewLength. r?anba!

The alternative is to add support for MIRType::IntPtr to MSub and MMinMax which
seemed more complicated.

This also matches the IC code in EmitDataViewBoundsCheck. A later patch will
change codegen to branchSubPtr.

Depends on D102727

Comment 15

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 12 - Add support for IntPtr MConstants. r?anba!

This will be useful for folding constant indexes to IntPtr in later patches.

Because we can't have different MConstant constructors with int64_t and intptr_t
arguments, reuse the one that takes int64_t by adding a MIRType argument to it.

Depends on D102728

Comment 16

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 13 - Add MInt32ToIntPtr. r?anba!

Depends on D102729

Comment 17

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 14 - Add IntPtrOperandId. r?anba!

This is similar to ValueTagOperandId. Later patches will use this type.

Depends on D102730

Comment 18

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 15 - MacroAssembler changes for loading buffer lengths/offsets. r?anba!

The variants that load an int32 value now take an optional Label that's used if
the index doesn't fit in an int32.

The Label argument is optional at this point so that we don't have to touch all
the callers, but later patches will change that.

Depends on D102731

Comment 19

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 16 - Guard ArrayBuffer byteLength fits in int32 for JIT fast paths. r?anba!

Eventually we should optimize the result-is-double case too.

Depends on D102732

Comment 20

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 17 - Guard ArrayBufferView byteOffset fits in int32 for JIT fast paths. r?anba!

Depends on D102733

Comment 21

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 18 - Guard ArrayBufferView length fits in int32 for JIT fast paths. r?anba!

This also adds the IntPtr implementation that will later be used for bounds checks.

Depends on D102734

Comment 22

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 19 - Add IntPtr support to MBoundsCheck and MSpectreMaskIndex. r?anba!

Depends on D102735

Comment 23

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 20 - Support bounds check optimizations when the index is MInt32ToIntPtr. r?anba!

Depends on D102736

Comment 24

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 21 - Change Atomics VM functions to take index as size_t instead of int32_t. r?anba!

Depends on D102737

Comment 25

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 23 - Add JIT support for large buffers. r?anba!

This switches all CacheIR and MIR instructions that use a TypedArray or DataView
index from Int32 to IntPtr.

The int32-index and non-int32-index cases in the GetElem/SetElem/In CacheIR generators
are now unified into a single method. This way it's easy to see that in-bounds Int32
and Double indexes are handled exactly the same way.

The IRGenerators call IRGenerator::guardToIntPtrIndex to convert key values to an
IntPtrOperandId. All TypedArray/DataView accesses optimized by CacheIR go through
there now.

The GuardToTypedArrayIndex CacheIR and MIR instruction is replaced with a similar
GuardNumberToIntPtrIndex instruction. It accepts a Number instead of a Value.

Depends on D102738

Comment 26

[Jan de Mooij [:jandem]]

Attachment: Bug 1687441 part 22 - Guard against int32 overflow in offset computations. r?anba!

Adds useRegisterOrIndexConstant to check this during lowering, and also uses
ArrayOffsetFitsInInt32 in codegen to assert no overflow.

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f48f084d1b07
part 7 - Add NumberEqualsInt64 and NumberIsInt64 to MFBT. r=sg
https://hg.mozilla.org/integration/autoland/rev/55faa30d1d32
part 8 - Don't use a type policy for the index argument for atomics. r=anba
https://hg.mozilla.org/integration/autoland/rev/cdd1a0427206
part 9 - Add MIRType::IntPtr. r=anba
https://hg.mozilla.org/integration/autoland/rev/242a6e9d5466
part 10 - Add MCompare::Compare_UIntPtr. r=anba
https://hg.mozilla.org/integration/autoland/rev/c8f373bd71a5
part 11 - Add MAdjustDataViewLength. r=anba
https://hg.mozilla.org/integration/autoland/rev/9dbff11bb583
part 12 - Add support for IntPtr MConstants. r=anba
https://hg.mozilla.org/integration/autoland/rev/11f74ecada8f
part 13 - Add MInt32ToIntPtr. r=anba

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/165c5f46f3b6
part 14 - Add IntPtrOperandId. r=anba
https://hg.mozilla.org/integration/autoland/rev/7a28b297920d
part 15 - MacroAssembler changes for loading buffer lengths/offsets. r=anba
https://hg.mozilla.org/integration/autoland/rev/10359c97a16d
part 16 - Guard ArrayBuffer byteLength fits in int32 for JIT fast paths. r=anba
https://hg.mozilla.org/integration/autoland/rev/4feb15e5cb8c
part 17 - Guard ArrayBufferView byteOffset fits in int32 for JIT fast paths. r=anba
https://hg.mozilla.org/integration/autoland/rev/32f439b0662a
part 18 - Guard ArrayBufferView length fits in int32 for JIT fast paths. r=anba

Comment 29

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/f48f084d1b07
https://hg.mozilla.org/mozilla-central/rev/55faa30d1d32
https://hg.mozilla.org/mozilla-central/rev/cdd1a0427206
https://hg.mozilla.org/mozilla-central/rev/242a6e9d5466
https://hg.mozilla.org/mozilla-central/rev/c8f373bd71a5
https://hg.mozilla.org/mozilla-central/rev/9dbff11bb583
https://hg.mozilla.org/mozilla-central/rev/11f74ecada8f

Comment 30

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/165c5f46f3b6
https://hg.mozilla.org/mozilla-central/rev/7a28b297920d
https://hg.mozilla.org/mozilla-central/rev/10359c97a16d
https://hg.mozilla.org/mozilla-central/rev/4feb15e5cb8c
https://hg.mozilla.org/mozilla-central/rev/32f439b0662a

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9263fbf533e0
part 19 - Add IntPtr support to MBoundsCheck and MSpectreMaskIndex. r=anba
https://hg.mozilla.org/integration/autoland/rev/ed004d2c8864
part 20 - Support bounds check optimizations when the index is MInt32ToIntPtr. r=anba,iain
https://hg.mozilla.org/integration/autoland/rev/44be7a953533
part 21 - Change Atomics VM functions to take index as size_t instead of int32_t. r=anba
https://hg.mozilla.org/integration/autoland/rev/29c981446c38
part 22 - Guard against int32 overflow in offset computations. r=anba
https://hg.mozilla.org/integration/autoland/rev/2610d2d33a73
part 23 - Add JIT support for large buffers. r=anba

Comment 32

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/9263fbf533e0
https://hg.mozilla.org/mozilla-central/rev/ed004d2c8864
https://hg.mozilla.org/mozilla-central/rev/44be7a953533
https://hg.mozilla.org/mozilla-central/rev/29c981446c38
https://hg.mozilla.org/mozilla-central/rev/2610d2d33a73