Use PrivateValue instead of Int32Value for ArrayBuffer{View} length and offset slots
Categories
(Core :: JavaScript Engine, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox84 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
References
Details
Attachments
(9 files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
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.
Assignee | ||
Comment 1•4 years ago
|
||
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.
Assignee | ||
Comment 2•4 years ago
|
||
This will let us change the slot representation without having to worry about
self-hosted code accessing these slots directly.
Depends on D94830
Assignee | ||
Comment 3•4 years ago
|
||
The accessors will need more work in the future to expose the offset value in a
typesafe way without implicit casts.
Depends on D94831
Assignee | ||
Comment 4•4 years ago
|
||
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
Assignee | ||
Comment 5•4 years ago
|
||
@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.
Assignee | ||
Comment 6•4 years ago
|
||
- Make byteOffsetValue non-static.
- Implementing byteOffsetValue using byteOffset is now more natural than the other way around.
- Move methods to the ArrayBufferViewObject base class.
Assignee | ||
Comment 7•4 years ago
|
||
Depends on D94947
Assignee | ||
Comment 8•4 years ago
|
||
Depends on D94949
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 9•4 years ago
|
||
For consistency with the other fooValue methods.
Comment 10•4 years ago
|
||
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
Assignee | ||
Comment 11•4 years ago
|
||
This matches ArrayBufferObject and will let us store larger length values.
Comment 12•4 years ago
|
||
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
Comment 13•4 years ago
|
||
bugherder |
Comment 14•4 years ago
|
||
bugherder |
Assignee | ||
Updated•4 years ago
|
Description
•