Closed Bug 1129202 Opened 11 years ago Closed 6 years ago

Implement CanonicalNumericIndexString and use it for typed arrays

Categories

(Core :: JavaScript Engine, task, P3)

Product:
Core
Component:
JavaScript Engine
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla72
Tracking Flags:
Tracking Status
firefox72 --- fixed

People

(Reporter: evilpies, Assigned: anba)

References

(Blocks 1 open bug)

Details

Attachments

(10 files, 1 obsolete file)

Bug 1129202 - Part 1: Allow "Infinity" and "NaN" for canonical numeric index strings. r=jandem!
47 bytes, text/x-phabricator-request
Details | Review
Bug 1129202 - Part 2: Move IsInteger to jsnum.h to allow reusing it. r=jandem!
47 bytes, text/x-phabricator-request
Details | Review
Bug 1129202 - Part 3: Change StringIsTypedArrayIndex's return type to allow passing an error. r=jandem!
47 bytes, text/x-phabricator-request
Details | Review
Bug 1129202 - Part 4: Allow numbers with fractional parts or exponents for canonical numeric indices. r=jandem!
47 bytes, text/x-phabricator-request
Details | Review
Bug 1129202 - Part 5: Enable now passing test262 tests. r=jandem!
47 bytes, text/x-phabricator-request
Details | Review
Bug 1129202 - Part 6: Add constant for maximum TypedArray length. r=jandem!
47 bytes, text/x-phabricator-request
Details | Review
Bug 1129202 - Part 7: Add MTypedArrayIndexToInt32 for accessing TypedArrays with any Double index. r=jandem!
47 bytes, text/x-phabricator-request
Details | Review
Bug 1129202 - Part 8: Handle TypedArray non-int32 indices in SetPropIRGenerator. r=jandem!
47 bytes, text/x-phabricator-request
Details | Review
Bug 1129202 - Part 9: Handle TypedArray non-int32 indices in HasPropIRGenerator. r=jandem!
47 bytes, text/x-phabricator-request
Details | Review
Bug 1129202 - Part 10: Handle TypedArray non-int32 indices in GetPropIRGenerator. r=jandem!
47 bytes, text/x-phabricator-request
Details | Review
ES6 imposes all sorts of stuff on array indexes in typed arrays. See for example [[HasProperty]] If IsDetachedBuffer(buffer) is true, throw a TypeError exception. If IsInteger(index) is false then return false If index = −0, return false. We have IsTypedArrayIndex, but that doesn't accept doubles, like CanonicalNumericIndexString After this is implemented the MOP needs to be audited and corrected.
Assignee: nobody → winter2718
By my reading this should be a valid test case: let a = new Int8Array([1]); assertEq(Object.getOwnPropertyDescriptor(a, "-0"), a[-0]); assertEq(Object.getOwnPropertyDescriptor(a, "0.0"), a[0.0]);
arg, two problems: * Object.getOwnPropertyDescriptor(a, x).value and these test cases are not valid because of step 4. in CanonicalNumericIndexString
Blocks: es6typedarray
No longer blocks: es6
Blocks: test262
Assignee: winter2718 → nobody
Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → INACTIVE
Status: RESOLVED → REOPENED
Resolution: INACTIVE → ---
Search for (String)IsTypedArrayIndex should find a lot of relevant places that need to be replaced by CanonicalNumericIndexString.
Blocks: 1486265
Priority: -- → P3

The assertion should make it easier to see why no length checks are used and
will make it easier to remember updating this code if we ever remove flat
strings.

Part 5 will call this function.

Depends on D39035

The next patch adds code which may fail on OOM, so StringIsTypedArrayIndex's
interface needs to be updated to allow passing on errors.

Depends on D39036

The next parts use this constant in static_asserts.

Depends on D39039

The definition of canonical numeric index strings means that any access into a TypedArray
with a Double index never reads from the prototype chain. Instead if the Double is not
representable as an Int32, the access is equivalent to an out-of-bounds access. That
means we can substitute any non-Int32 Double index with an arbitrary index which is OOB.
This helps to avoid deoptimising TypedArray accesses with non-Int32 Double values by
handling them as normal out-of-bounds accesses, which are already optimisable.

Depends on D39040

Handle and report out-of-bounds accesses in SetPropIRGenerator to ensure Ion won't
repeatedly bailout when out-of-bounds accesses happen.

Depends on D39041

That way CacheIR can still attach an IC and Baseline/Ion don't need to take the
slow path when a TypedArray access happens through a Double not representable as
an Int32.

Depends on D39043

That way CacheIR can still attach an IC and Baseline/Ion don't need to take the
slow path when a TypedArray access happens through a Double not representable as
an Int32.

Depends on D39044

Assignee: nobody → andrebargull
Status: REOPENED → ASSIGNED
Attachment #9080056 - Attachment description: Bug 1129202 - Part 1: Add assertion to check strings are null-terminated. → Bug 1129202 - Part 1: Add assertion to check strings are null-terminated. r=jandem!
Attachment #9080058 - Attachment description: Bug 1129202 - Part 2: Allow "Infinity" and "NaN" for canonical numeric index strings. → Bug 1129202 - Part 2: Allow "Infinity" and "NaN" for canonical numeric index strings. r=jandem!
Attachment #9080059 - Attachment description: Bug 1129202 - Part 3: Move IsInteger to jsnum.h to allow reusing it. → Bug 1129202 - Part 3: Move IsInteger to jsnum.h to allow reusing it. r=jandem!
Attachment #9080060 - Attachment description: Bug 1129202 - Part 4: Change StringIsTypedArrayIndex's return type to allow passing an error. → Bug 1129202 - Part 4: Change StringIsTypedArrayIndex's return type to allow passing an error. r=jandem!
Attachment #9080061 - Attachment description: Bug 1129202 - Part 5: Allow numbers with fractional parts or exponents for canonical numeric indices. → Bug 1129202 - Part 5: Allow numbers with fractional parts or exponents for canonical numeric indices. r=jandem!
Attachment #9080062 - Attachment description: Bug 1129202 - Part 6: Enable now passing test262 tests. → Bug 1129202 - Part 6: Enable now passing test262 tests. r=jandem!
Attachment #9080063 - Attachment description: Bug 1129202 - Part 7: Add constant for maximum TypedArray length. → Bug 1129202 - Part 7: Add constant for maximum TypedArray length. r=jandem!
Attachment #9080064 - Attachment description: Bug 1129202 - Part 8: Add MTypedArrayIndexToInt32 for accessing TypedArrays with any Double index. → Bug 1129202 - Part 8: Add MTypedArrayIndexToInt32 for accessing TypedArrays with any Double index. r=jandem!
Attachment #9080066 - Attachment description: Bug 1129202 - Part 9: Handle TypedArray out-of-bounds access in SetPropIRGenerator to avoid repeated Ion bailouts. → Bug 1129202 - Part 9: Handle TypedArray out-of-bounds access in SetPropIRGenerator to avoid repeated Ion bailouts. r=jandem!
Attachment #9080067 - Attachment description: Bug 1129202 - Part 10: Handle TypedArray out-of-bounds access in HasPropIRGenerator. → Bug 1129202 - Part 10: Handle TypedArray out-of-bounds access in HasPropIRGenerator. r=jandem!
Attachment #9080068 - Attachment description: Bug 1129202 - Part 11: Handle TypedArray out-of-bounds access in GetPropIRGenerator. → Bug 1129202 - Part 11: Handle TypedArray out-of-bounds access in GetPropIRGenerator. r=jandem!

Filed bug 1573803 for Phabricator not displaying the change sets.

Type: defect → task

Hey I'm seeing this stack for the first time now because the patches are in draft state! Let me know if you want me to review :)

I'll probably request review when the BigInt CacheIR changes have landed.

Depends on: 1590731
Attachment #9080058 - Attachment description: Bug 1129202 - Part 2: Allow "Infinity" and "NaN" for canonical numeric index strings. r=jandem! → Bug 1129202 - Part 1: Allow "Infinity" and "NaN" for canonical numeric index strings. r=jandem!
Attachment #9080059 - Attachment description: Bug 1129202 - Part 3: Move IsInteger to jsnum.h to allow reusing it. r=jandem! → Bug 1129202 - Part 2: Move IsInteger to jsnum.h to allow reusing it. r=jandem!
Attachment #9080060 - Attachment description: Bug 1129202 - Part 4: Change StringIsTypedArrayIndex's return type to allow passing an error. r=jandem! → Bug 1129202 - Part 3: Change StringIsTypedArrayIndex's return type to allow passing an error. r=jandem!
Attachment #9080061 - Attachment description: Bug 1129202 - Part 5: Allow numbers with fractional parts or exponents for canonical numeric indices. r=jandem! → Bug 1129202 - Part 4: Allow numbers with fractional parts or exponents for canonical numeric indices. r=jandem!
Attachment #9080062 - Attachment description: Bug 1129202 - Part 6: Enable now passing test262 tests. r=jandem! → Bug 1129202 - Part 5: Enable now passing test262 tests. r=jandem!
Attachment #9080063 - Attachment description: Bug 1129202 - Part 7: Add constant for maximum TypedArray length. r=jandem! → Bug 1129202 - Part 6: Add constant for maximum TypedArray length. r=jandem!
Attachment #9080064 - Attachment description: Bug 1129202 - Part 8: Add MTypedArrayIndexToInt32 for accessing TypedArrays with any Double index. r=jandem! → Bug 1129202 - Part 7: Add MTypedArrayIndexToInt32 for accessing TypedArrays with any Double index. r=jandem!
Attachment #9080066 - Attachment description: Bug 1129202 - Part 9: Handle TypedArray out-of-bounds access in SetPropIRGenerator to avoid repeated Ion bailouts. r=jandem! → Bug 1129202 - Part 8: Handle TypedArray non-int32 indices in SetPropIRGenerator. r=jandem!
Attachment #9080067 - Attachment description: Bug 1129202 - Part 10: Handle TypedArray out-of-bounds access in HasPropIRGenerator. r=jandem! → Bug 1129202 - Part 9: Handle TypedArray non-int32 indices in HasPropIRGenerator. r=jandem!
Attachment #9080068 - Attachment description: Bug 1129202 - Part 11: Handle TypedArray out-of-bounds access in GetPropIRGenerator. r=jandem! → Bug 1129202 - Part 10: Handle TypedArray non-int32 indices in GetPropIRGenerator. r=jandem!
Attachment #9080056 - Attachment is obsolete: true
Pushed by archaeopteryx@coole-files.de: https://hg.mozilla.org/integration/autoland/rev/eafcc6ace2fb Part 1: Allow "Infinity" and "NaN" for canonical numeric index strings. r=jandem https://hg.mozilla.org/integration/autoland/rev/2c9bedcdac37 Part 2: Move IsInteger to jsnum.h to allow reusing it. r=jandem https://hg.mozilla.org/integration/autoland/rev/041413289d85 Part 3: Change StringIsTypedArrayIndex's return type to allow passing an error. r=jandem https://hg.mozilla.org/integration/autoland/rev/77c80b5f79ea Part 4: Allow numbers with fractional parts or exponents for canonical numeric indices. r=jandem https://hg.mozilla.org/integration/autoland/rev/055d1cad290a Part 5: Enable now passing test262 tests. r=jandem https://hg.mozilla.org/integration/autoland/rev/288125d24d23 Part 6: Add constant for maximum TypedArray length. r=jandem https://hg.mozilla.org/integration/autoland/rev/825e8843d0b1 Part 7: Add MTypedArrayIndexToInt32 for accessing TypedArrays with any Double index. r=jandem https://hg.mozilla.org/integration/autoland/rev/371d2efe156c Part 8: Handle TypedArray non-int32 indices in SetPropIRGenerator. r=jandem https://hg.mozilla.org/integration/autoland/rev/cbeae82d295c Part 9: Handle TypedArray non-int32 indices in HasPropIRGenerator. r=jandem https://hg.mozilla.org/integration/autoland/rev/7b07bb35e7b2 Part 10: Handle TypedArray non-int32 indices in GetPropIRGenerator. r=jandem
Blocks: 1632672
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: