1673604 - Use PrivateValue instead of Int32Value for ArrayBuffer{View} length and offset slots

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Jan de Mooij [:jandem]]

Length, byteLength and byteOffset values for ArrayBuffer and ArrayBufferView objects should be stored as PrivateValue so that we can easily store values > INT32_MAX on 64-bit platforms.

This will still cast values to Int32Value and won't have proper JIT support, but it's a nice step in the right direction.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1673604 part 1 - Store ArrayBuffer byteLength value as PrivateValue instead of Int32Value. r?anba!

This will let us store larger byteLength values in the future.

Adds a new CacheIR and MIR instruction for loading this value. The instruction
names contain Int32 because in the future this will have to bail out for non-int32
values on 64-bit platforms once we can create large ArrayBuffers. At that point we
could add a different instruction for loading the byteLength as double.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1673604 part 2 - Remove unused JS_TYPEDARRAYLAYOUT_LENGTH_SLOT and JS_TYPEDARRAYLAYOUT_BYTEOFFSET_SLOT. r?anba!

This will let us change the slot representation without having to worry about
self-hosted code accessing these slots directly.

Depends on D94830

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1673604 part 3 - Store ArrayBufferView byteOffset value as PrivateValue instead of Int32Value. r?anba!

The accessors will need more work in the future to expose the offset value in a
typesafe way without implicit casts.

Depends on D94831

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1673604 part 4 - Store ArrayBufferView length value as PrivateValue instead of Int32Value. r?anba!

Similar to previous patches. Most of the loadArrayBufferViewLengthInt32 calls will
have to be changed in the future to load the full word-size length, but at least
we now get assertion failures when larger length values show up.

Depends on D94832

Comment 5

[Jan de Mooij [:jandem]]

@anba: I added you as reviewer because you're familiar with the VM and JIT code for this, but if you're busy I can find someone else. Hopefully the C++ changes we can do on top of this will be easier for others to review.

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1673604 part 5 - Clean up byteOffset/byteOffsetValue accessors. r?anba!

• Make byteOffsetValue non-static.
• Implementing byteOffsetValue using byteOffset is now more natural than the other way around.
• Move methods to the ArrayBufferViewObject base class.

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Bug 1673604 part 6 - Clean up byteLength/byteLengthValue accessors. r?anba!

Depends on D94947

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Bug 1673604 part 7 - Clean up length/lengthValue accessors. r?anba!

Depends on D94949

Comment 9

[Jan de Mooij [:jandem]]

Attachment: Bug 1673604 part 8 - Make ArrayBufferViewObject::bufferValue non-static. r?anba!

For consistency with the other fooValue methods.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/404b9af13176
part 1 - Store ArrayBuffer byteLength value as PrivateValue instead of Int32Value. r=anba
https://hg.mozilla.org/integration/autoland/rev/35ef3ae2f456
part 2 - Remove unused JS_TYPEDARRAYLAYOUT_LENGTH_SLOT and JS_TYPEDARRAYLAYOUT_BYTEOFFSET_SLOT. r=anba
https://hg.mozilla.org/integration/autoland/rev/2b4e130218f1
part 3 - Store ArrayBufferView byteOffset value as PrivateValue instead of Int32Value. r=anba
https://hg.mozilla.org/integration/autoland/rev/f4bf6ef21d3b
part 4 - Store ArrayBufferView length value as PrivateValue instead of Int32Value. r=anba

Comment 11

[Jan de Mooij [:jandem]]

Attachment: Bug 1673604 part 9 - Store SharedArrayBufferObject byteLength value as PrivateValue instead of PrivateUint32Value. r?lth!

This matches ArrayBufferObject and will let us store larger length values.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bae725214f5a
part 5 - Clean up byteOffset/byteOffsetValue accessors. r=anba
https://hg.mozilla.org/integration/autoland/rev/f57c2c9b28e2
part 6 - Clean up byteLength/byteLengthValue accessors. r=anba
https://hg.mozilla.org/integration/autoland/rev/0eb8a63ace86
part 7 - Clean up length/lengthValue accessors. r=anba
https://hg.mozilla.org/integration/autoland/rev/6c07a795cfd0
part 8 - Make ArrayBufferViewObject::bufferValue non-static. r=anba
https://hg.mozilla.org/integration/autoland/rev/c14664508e61
part 9 - Store SharedArrayBufferObject byteLength value as PrivateValue instead of PrivateUint32Value. r=lth

Comment 13

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/404b9af13176
https://hg.mozilla.org/mozilla-central/rev/35ef3ae2f456
https://hg.mozilla.org/mozilla-central/rev/2b4e130218f1
https://hg.mozilla.org/mozilla-central/rev/f4bf6ef21d3b

Comment 14

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/bae725214f5a
https://hg.mozilla.org/mozilla-central/rev/f57c2c9b28e2
https://hg.mozilla.org/mozilla-central/rev/0eb8a63ace86
https://hg.mozilla.org/mozilla-central/rev/6c07a795cfd0
https://hg.mozilla.org/mozilla-central/rev/c14664508e61