Closed Bug 1673604 Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: JavaScript Engine, task, P3)

task

Tracking

()

RESOLVED FIXED
84 Branch
Tracking Status
firefox84 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(9 files)

(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details

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.

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.

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

Depends on D94830

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

Depends on D94831

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

@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.

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

Depends on D94949

Keywords: leave-open

For consistency with the other fooValue methods.

Pushed by jdemooij@mozilla.com:
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

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

Blocks: 1673867
Pushed by jdemooij@mozilla.com:
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
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: