Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add Float16Array to all generic TypedArray tests #3849

Merged
merged 5 commits into from
Jan 10, 2024
Merged

add Float16Array to all generic TypedArray tests #3849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bdca54a
add Float16Array to all generic TypedArray tests
bakkot Jun 11, 2023
9fd5419
add helpers
bakkot Nov 19, 2023
c1cf1d3
address todo
bakkot Nov 20, 2023
0964566
use correct constructors
bakkot Nov 20, 2023
e5d20a7
fix missing paren
bakkot Dec 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 48 additions & 13 deletions harness/testTypedArray.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,23 @@
description: |
Collection of functions used to assert the correctness of TypedArray objects.
defines:
- typedArrayConstructors
- floatArrayConstructors
- intArrayConstructors
- typedArrayConstructors
- TypedArray
- testWithTypedArrayConstructors
- nonAtomicsFriendlyTypedArrayConstructors
- testWithAtomicsFriendlyTypedArrayConstructors
- testWithNonAtomicsFriendlyTypedArrayConstructors
- testTypedArrayConversions
---*/

/**
* Array containing every typed array constructor.
*/
var typedArrayConstructors = [
var floatArrayConstructors = [
Float64Array,
Float32Array,
Float32Array
];

var intArrayConstructors = [
Int32Array,
Int16Array,
Int8Array,
Expand All @@ -29,8 +30,17 @@ var typedArrayConstructors = [
Uint8ClampedArray
];

var floatArrayConstructors = typedArrayConstructors.slice(0, 2);
var intArrayConstructors = typedArrayConstructors.slice(2, 7);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intArrayConstructors did not contain Uint8ClampedArray but it is included now. This seems to be an intended change. Unfortunately, some Atomics tests (like built-ins/Atomics/add/good-views.js) still use intArrayConstructors. In other words, these tests are incorrect now. They should be rewritten to use testWithAtomicsFriendlyTypedArrayConstructors(), I guess.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the catch. Would you mind opening an issue for that, please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that was not an intended change, I just didn't realize that the .slice above stopped before the end of the list.

In fact the .slice was previously also omitting Uint8Array, which seems wrong. I've left it in, but opened #3984 to remove Uint8ClampedArray.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the original idea was to include Uint8ClampedArray in intArrayConstructors. The author just haven't realized that the second argument in typedArrayConstructors.slice(2, 7); is not the length but an (excluded) end index. That's why Uint8Array and Uint8ClampedArray were missing by accident. I find it correct to include Uint8ClampedArray in this array (it is a typed array with int elements). Some tests may skip Uint8ClampedArray unnecessarily when it is not included. That's why I though that the change was intentional and that just the problematic Atomics tests (that use intArrayConstructors without excluding Uint8ClampedArray) should be fixed/updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iamstolis Do you happen to have a list of such tests? It's a bit painful to track them down manually.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following tests started to fail (when run against graal-js engine). So, this is the correct list, hopefully.

Unexpectedly failed 'built-ins/Atomics/add/bad-range.js'
Unexpectedly failed 'built-ins/Atomics/add/good-views.js'
Unexpectedly failed 'built-ins/Atomics/and/bad-range.js'
Unexpectedly failed 'built-ins/Atomics/and/good-views.js'
Unexpectedly failed 'built-ins/Atomics/compareExchange/bad-range.js'
Unexpectedly failed 'built-ins/Atomics/compareExchange/good-views.js'
Unexpectedly failed 'built-ins/Atomics/exchange/bad-range.js'
Unexpectedly failed 'built-ins/Atomics/exchange/good-views.js'
Unexpectedly failed 'built-ins/Atomics/load/bad-range.js'
Unexpectedly failed 'built-ins/Atomics/load/good-views.js'
Unexpectedly failed 'built-ins/Atomics/or/bad-range.js'
Unexpectedly failed 'built-ins/Atomics/or/good-views.js'
Unexpectedly failed 'built-ins/Atomics/store/bad-range.js'
Unexpectedly failed 'built-ins/Atomics/store/good-views.js'
Unexpectedly failed 'built-ins/Atomics/sub/bad-range.js'
Unexpectedly failed 'built-ins/Atomics/sub/good-views.js'
Unexpectedly failed 'built-ins/Atomics/xor/bad-range.js'
Unexpectedly failed 'built-ins/Atomics/xor/good-views.js'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iamstolis Thanks, see #3984.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I run the tests from the current revision (62294b592e332bc16da6114e461fd7a8ed90daab) of #3984 against graal-js and I haven't seen any unexpected failures.

// Float16Array is a newer feature
// adding it to this list unconditionally would cause implementations lacking it to fail every test which uses it
if (typeof Float16Array !== 'undefined') {
floatArrayConstructors.push(Float16Array);
}

/**
* Array containing every non-bigint typed array constructor.
*/

var typedArrayConstructors = floatArrayConstructors.concat(intArrayConstructors);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR included Float16Array in typedArrayConstructors. Unfortunately, it failed to provide byteConversionValues.expected.Float16 (in harness/byteConversionValues.js). So, the tests that use testTypedArrayConversions(byteConversionValues, ...) fail when Float16Array is enabled. This seems to affect the following tests:

built-ins/TypedArray/prototype/fill/fill-values-conversion-operations.js
built-ins/TypedArray/prototype/map/return-new-typedarray-conversion-operation.js
built-ins/TypedArray/prototype/set/array-arg-src-tonumber-value-conversions.js
built-ins/TypedArray/prototype/set/typedarray-arg-set-values-diff-buffer-other-type-conversions-sab.js
built-ins/TypedArray/prototype/set/typedarray-arg-set-values-diff-buffer-other-type-conversions.js
built-ins/TypedArrayConstructors/ctors/object-arg/conversion-operation.js
built-ins/TypedArrayConstructors/internals/DefineOwnProperty/conversion-operation.js
built-ins/TypedArrayConstructors/internals/Set/conversion-operation.js

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iamstolis Please open an issue - if no-one happens to have time to fix this right when they see it, it'll likely get lost out of their notifications.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I have created #3989


/**
* The %TypedArray% intrinsic constructor function.
Expand Down Expand Up @@ -63,18 +73,15 @@ function testWithTypedArrayConstructors(f, selected) {
}
}

var nonAtomicsFriendlyTypedArrayConstructors = floatArrayConstructors.concat([Uint8ClampedArray]);
/**
* Calls the provided function for every non-"Atomics Friendly" typed array constructor.
*
* @param {typedArrayConstructorCallback} f - the function to call for each typed array constructor.
* @param {Array} selected - An optional Array with filtered typed arrays
*/
function testWithNonAtomicsFriendlyTypedArrayConstructors(f) {
testWithTypedArrayConstructors(f, [
Float64Array,
Float32Array,
Uint8ClampedArray
]);
testWithTypedArrayConstructors(f, nonAtomicsFriendlyTypedArrayConstructors);
}

/**
Expand Down Expand Up @@ -120,3 +127,31 @@ function testTypedArrayConversions(byteConversionValues, fn) {
});
});
}

/**
* Checks if the given argument is one of the float-based TypedArray constructors.
*
* @param {constructor} ctor - the value to check
* @returns {boolean}
*/
function isFloatTypedArrayConstructor(arg) {
return floatArrayConstructors.indexOf(arg) !== -1;
}

/**
* Determines the precision of the given float-based TypedArray constructor.
*
* @param {constructor} ctor - the value to check
* @returns {string} "half", "single", or "double" for Float16Array, Float32Array, and Float64Array respectively.
*/
function floatTypedArrayConstructorPrecision(FA) {
if (typeof Float16Array !== "undefined" && FA === Float16Array) {
return "half";
} else if (FA === Float32Array) {
return "single";
} else if (FA === Float64Array) {
return "double";
} else {
throw new Error("Malformed test - floatTypedArrayConstructorPrecision called with non-float TypedArray");
}
}
1 change: 1 addition & 0 deletions test/built-ins/Array/prototype/concat/Array.prototype.concat_large-typed-array.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ function concatTypedArray(type, elems, modulo) {
assert.compareArray([].concat(ta), expected);
}

// Float16Array cannot be included in this because it cannot precisely represent integers above 2048
var max = [Math.pow(2, 8), Math.pow(2, 16), Math.pow(2, 32), false, false];
[
Uint8Array,
Expand Down
10 changes: 8 additions & 2 deletions test/built-ins/Array/prototype/concat/Array.prototype.concat_small-typed-array.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,18 @@ function concatTypedArray(type, elems, modulo) {
assert.compareArray([].concat(ta), expected);
}
var max = [Math.pow(2, 8), Math.pow(2, 16), Math.pow(2, 32), false, false];
[
var TAs = [
Uint8Array,
Uint16Array,
Uint32Array,
Float32Array,
Float64Array
].forEach(function(ctor, i) {
];
if (typeof Float16Array !== 'undefined') {
max.push(false);
TAs.push(Float16Array);
}

TAs.forEach(function(ctor, i) {
concatTypedArray(ctor, 1, max[i]);
});
7 changes: 2 additions & 5 deletions test/built-ins/Atomics/add/validate-arraytype-before-index-coercion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ info: |
a. If typeName is not "Int8Array", "Uint8Array", "Int16Array", "Uint16Array", "Int32Array",
or "Uint32Array", throw a TypeError exception.
...
includes: [testTypedArray.js]
features: [Atomics, TypedArray]
---*/

Expand All @@ -31,11 +32,7 @@ var index = {
}
};

var badArrayTypes = [
Uint8ClampedArray, Float32Array, Float64Array
];

for (var badArrayType of badArrayTypes) {
for (var badArrayType of nonAtomicsFriendlyTypedArrayConstructors) {
var typedArray = new badArrayType(new SharedArrayBuffer(8));
assert.throws(TypeError, function() {
Atomics.add(typedArray, index, 0);
Expand Down
7 changes: 2 additions & 5 deletions test/built-ins/Atomics/add/validate-arraytype-before-value-coercion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ info: |
a. If typeName is not "Int8Array", "Uint8Array", "Int16Array", "Uint16Array", "Int32Array",
or "Uint32Array", throw a TypeError exception.
...
includes: [testTypedArray.js]
features: [Atomics, TypedArray]
---*/

Expand All @@ -31,11 +32,7 @@ var value = {
}
};

var badArrayTypes = [
Uint8ClampedArray, Float32Array, Float64Array
];

for (var badArrayType of badArrayTypes) {
for (var badArrayType of nonAtomicsFriendlyTypedArrayConstructors) {
var typedArray = new badArrayType(new SharedArrayBuffer(8));
assert.throws(TypeError, function() {
Atomics.add(typedArray, 0, value);
Expand Down
7 changes: 2 additions & 5 deletions test/built-ins/Atomics/and/validate-arraytype-before-index-coercion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ info: |
a. If typeName is not "Int8Array", "Uint8Array", "Int16Array", "Uint16Array", "Int32Array",
or "Uint32Array", throw a TypeError exception.
...
includes: [testTypedArray.js]
features: [Atomics, TypedArray]
---*/

Expand All @@ -31,11 +32,7 @@ var index = {
}
};

var badArrayTypes = [
Uint8ClampedArray, Float32Array, Float64Array
];

for (var badArrayType of badArrayTypes) {
for (var badArrayType of nonAtomicsFriendlyTypedArrayConstructors) {
var typedArray = new badArrayType(new SharedArrayBuffer(8));
assert.throws(TypeError, function() {
Atomics.and(typedArray, index, 0);
Expand Down
7 changes: 2 additions & 5 deletions test/built-ins/Atomics/and/validate-arraytype-before-value-coercion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ info: |
a. If typeName is not "Int8Array", "Uint8Array", "Int16Array", "Uint16Array", "Int32Array",
or "Uint32Array", throw a TypeError exception.
...
includes: [testTypedArray.js]
features: [Atomics, TypedArray]
---*/

Expand All @@ -31,11 +32,7 @@ var value = {
}
};

var badArrayTypes = [
Uint8ClampedArray, Float32Array, Float64Array
];

for (var badArrayType of badArrayTypes) {
for (var badArrayType of nonAtomicsFriendlyTypedArrayConstructors) {
var typedArray = new badArrayType(new SharedArrayBuffer(8));
assert.throws(TypeError, function() {
Atomics.and(typedArray, 0, value);
Expand Down
7 changes: 2 additions & 5 deletions test/built-ins/Atomics/compareExchange/validate-arraytype-before-expectedValue-coercion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ info: |
a. If typeName is not "Int8Array", "Uint8Array", "Int16Array", "Uint16Array", "Int32Array",
or "Uint32Array", throw a TypeError exception.
...
includes: [testTypedArray.js]
features: [Atomics, TypedArray]
---*/

Expand All @@ -28,11 +29,7 @@ var expectedValue = {
}
};

var badArrayTypes = [
Uint8ClampedArray, Float32Array, Float64Array
];

for (var badArrayType of badArrayTypes) {
for (var badArrayType of nonAtomicsFriendlyTypedArrayConstructors) {
var typedArray = new badArrayType(new SharedArrayBuffer(8));
assert.throws(TypeError, function() {
Atomics.compareExchange(typedArray, 0, expectedValue, 0);
Expand Down
7 changes: 2 additions & 5 deletions test/built-ins/Atomics/compareExchange/validate-arraytype-before-index-coercion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ info: |
a. If typeName is not "Int8Array", "Uint8Array", "Int16Array", "Uint16Array", "Int32Array",
or "Uint32Array", throw a TypeError exception.
...
includes: [testTypedArray.js]
features: [Atomics, TypedArray]
---*/

Expand All @@ -28,11 +29,7 @@ var index = {
}
};

var badArrayTypes = [
Uint8ClampedArray, Float32Array, Float64Array
];

for (var badArrayType of badArrayTypes) {
for (var badArrayType of nonAtomicsFriendlyTypedArrayConstructors) {
var typedArray = new badArrayType(new SharedArrayBuffer(8));
assert.throws(TypeError, function() {
Atomics.compareExchange(typedArray, index, 0, 0);
Expand Down
7 changes: 2 additions & 5 deletions .../built-ins/Atomics/compareExchange/validate-arraytype-before-replacementValue-coercion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ info: |
a. If typeName is not "Int8Array", "Uint8Array", "Int16Array", "Uint16Array", "Int32Array",
or "Uint32Array", throw a TypeError exception.
...
includes: [testTypedArray.js]
features: [Atomics, TypedArray]
---*/

Expand All @@ -28,11 +29,7 @@ var replacementValue = {
}
};

var badArrayTypes = [
Uint8ClampedArray, Float32Array, Float64Array
];

for (var badArrayType of badArrayTypes) {
for (var badArrayType of nonAtomicsFriendlyTypedArrayConstructors) {
var typedArray = new badArrayType(new SharedArrayBuffer(8));
assert.throws(TypeError, function() {
Atomics.compareExchange(typedArray, 0, 0, replacementValue);
Expand Down
7 changes: 2 additions & 5 deletions test/built-ins/Atomics/exchange/validate-arraytype-before-index-coercion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ info: |
a. If typeName is not "Int8Array", "Uint8Array", "Int16Array", "Uint16Array", "Int32Array",
or "Uint32Array", throw a TypeError exception.
...
includes: [testTypedArray.js]
features: [Atomics, TypedArray]
---*/

Expand All @@ -31,11 +32,7 @@ var index = {
}
};

var badArrayTypes = [
Uint8ClampedArray, Float32Array, Float64Array
];

for (var badArrayType of badArrayTypes) {
for (var badArrayType of nonAtomicsFriendlyTypedArrayConstructors) {
var typedArray = new badArrayType(new SharedArrayBuffer(8));
assert.throws(TypeError, function() {
Atomics.exchange(typedArray, index, 0);
Expand Down
7 changes: 2 additions & 5 deletions test/built-ins/Atomics/exchange/validate-arraytype-before-value-coercion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ info: |
a. If typeName is not "Int8Array", "Uint8Array", "Int16Array", "Uint16Array", "Int32Array",
or "Uint32Array", throw a TypeError exception.
...
includes: [testTypedArray.js]
features: [Atomics, TypedArray]
---*/

Expand All @@ -31,11 +32,7 @@ var value = {
}
};

var badArrayTypes = [
Uint8ClampedArray, Float32Array, Float64Array
];

for (var badArrayType of badArrayTypes) {
for (var badArrayType of nonAtomicsFriendlyTypedArrayConstructors) {
var typedArray = new badArrayType(new SharedArrayBuffer(8));
assert.throws(TypeError, function() {
Atomics.exchange(typedArray, 0, value);
Expand Down
7 changes: 2 additions & 5 deletions test/built-ins/Atomics/load/validate-arraytype-before-index-coercion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ info: |
a. If typeName is not "Int8Array", "Uint8Array", "Int16Array", "Uint16Array", "Int32Array",
or "Uint32Array", throw a TypeError exception.
...
includes: [testTypedArray.js]
features: [Atomics, TypedArray]
---*/

Expand All @@ -31,11 +32,7 @@ var index = {
}
};

var badArrayTypes = [
Uint8ClampedArray, Float32Array, Float64Array
];

for (var badArrayType of badArrayTypes) {
for (var badArrayType of nonAtomicsFriendlyTypedArrayConstructors) {
var typedArray = new badArrayType(new SharedArrayBuffer(8));
assert.throws(TypeError, function() {
Atomics.load(typedArray, index);
Expand Down
9 changes: 9 additions & 0 deletions test/built-ins/Atomics/notify/non-int32-typedarray-throws.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ assert.throws(TypeError, function() {
Atomics.notify(view, poisoned, poisoned);
}, '`const view = new Float32Array( new SharedArrayBuffer(Float32Array.BYTES_PER_ELEMENT * 4) ); Atomics.notify(view, poisoned, poisoned)` throws TypeError');

if (typeof Float16Array !== 'undefined') {
assert.throws(TypeError, function() {
const view = new Float16Array(
new SharedArrayBuffer(Float16Array.BYTES_PER_ELEMENT * 2)
);
Atomics.notify(view, poisoned, poisoned);
}, '`const view = new Float16Array( new SharedArrayBuffer(Float16Array.BYTES_PER_ELEMENT * 2) ); Atomics.notify(view, poisoned, poisoned)` throws TypeError');
}

assert.throws(TypeError, function() {
const view = new Int16Array(
new SharedArrayBuffer(Int16Array.BYTES_PER_ELEMENT * 2)
Expand Down
6 changes: 2 additions & 4 deletions test/built-ins/Atomics/notify/validate-arraytype-before-count-coercion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ info: |
a. If typeName is not "Int8Array", "Uint8Array", "Int16Array", "Uint16Array", "Int32Array",
or "Uint32Array", throw a TypeError exception.
...
includes: [testTypedArray.js]
features: [Atomics, TypedArray]
---*/

Expand All @@ -28,10 +29,7 @@ var count = {
}
};

var badArrayTypes = [
Int8Array, Uint8Array, Int16Array, Uint16Array, Uint32Array,
Uint8ClampedArray, Float32Array, Float64Array
];
var badArrayTypes = typedArrayConstructors.filter(function(TA) { return TA !== Int32Array; });

for (var badArrayType of badArrayTypes) {
var typedArray = new badArrayType(new SharedArrayBuffer(8));
Expand Down
6 changes: 2 additions & 4 deletions test/built-ins/Atomics/notify/validate-arraytype-before-index-coercion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ info: |
a. If typeName is not "Int8Array", "Uint8Array", "Int16Array", "Uint16Array", "Int32Array",
or "Uint32Array", throw a TypeError exception.
...
includes: [testTypedArray.js]
features: [Atomics, TypedArray]
---*/

Expand All @@ -28,10 +29,7 @@ var index = {
}
};

var badArrayTypes = [
Int8Array, Uint8Array, Int16Array, Uint16Array, Uint32Array,
Uint8ClampedArray, Float32Array, Float64Array
];
var badArrayTypes = typedArrayConstructors.filter(function(TA) { return TA !== Int32Array; });

for (var badArrayType of badArrayTypes) {
var typedArray = new badArrayType(new SharedArrayBuffer(8));
Expand Down
Loading