From 1dd5de6f08fac297f3bd1537f0ecb44914a5f3e4 Mon Sep 17 00:00:00 2001 From: Jakob Stoklund Olesen Date: Tue, 15 Mar 2016 09:47:19 -0700 Subject: [PATCH 1/3] Coerce lane indexes with ToNumber(). SIMD functions that take a lane index (extractLane, replaceLane, swizzle, shuffle) should use ToNumber() to coerce the lane index to a number before checking that the index is an integer in range. This behavior corresponds to the SIMDToLane() function in the SIMD.js specification. This fixes issues #329 and #237. --- src/ecmascript_simd.js | 60 ++++++++++++++-------------- src/ecmascript_simd_tests.js | 77 +++++++++++++++++++++++++----------- 2 files changed, 82 insertions(+), 55 deletions(-) diff --git a/src/ecmascript_simd.js b/src/ecmascript_simd.js index a3de907..732c686 100644 --- a/src/ecmascript_simd.js +++ b/src/ecmascript_simd.js @@ -68,10 +68,6 @@ function convertArray(buffer, array) { // Utility functions. -function isInt32(o) { - return (o | 0) === o; -} - function isTypedArray(o) { return (o instanceof Int8Array) || (o instanceof Uint8Array) || @@ -107,17 +103,19 @@ function clamp(a, min, max) { // SIMD implementation functions function simdCoerceIndex(index) { - index = +index; - if (index != Math.floor(index)) - throw new RangeError("SIMD index must be an integer"); - return index; + index = +index; + if (index != Math.floor(index)) + throw new RangeError("SIMD index must be an integer"); + return index; } -function simdCheckLaneIndex(index, lanes) { - if (!isInt32(index)) - throw new TypeError('Lane index must be an int32'); - if (index < 0 || index >= lanes) +function simdCoerceLaneIndex(index, lanes) { + index = +index; + if (!(index >= 0 && index < lanes)) throw new RangeError('Lane index must be in bounds'); + if (index != Math.floor(index)) + throw new RangeError('Lane index must be an integer'); + return index; } // Global lanes array for constructing SIMD values. @@ -155,7 +153,7 @@ function simdSplat(type, s) { function simdReplaceLane(type, a, i, s) { a = type.fn.check(a); - simdCheckLaneIndex(i, type.lanes); + i = simdCoerceLaneIndex(i, type.lanes); for (var j = 0; j < type.lanes; j++) lanes[j] = type.fn.extractLane(a, j); lanes[i] = s; @@ -196,8 +194,8 @@ function simdSelect(type, selector, a, b) { function simdSwizzle(type, a, indices) { a = type.fn.check(a); for (var i = 0; i < indices.length; i++) { - simdCheckLaneIndex(indices[i], type.lanes); - lanes[i] = type.fn.extractLane(a, indices[i]); + var idx = simdCoerceLaneIndex(indices[i], type.lanes); + lanes[i] = type.fn.extractLane(a, idx); } return simdCreate(type); } @@ -206,10 +204,10 @@ function simdShuffle(type, a, b, indices) { a = type.fn.check(a); b = type.fn.check(b); for (var i = 0; i < indices.length; i++) { - simdCheckLaneIndex(indices[i], 2 * type.lanes); - lanes[i] = indices[i] < type.lanes ? - type.fn.extractLane(a, indices[i]) : - type.fn.extractLane(b, indices[i] - type.lanes); + var idx = simdCoerceLaneIndex(indices[i], 2 * type.lanes); + lanes[i] = idx < type.lanes ? + type.fn.extractLane(a, idx) : + type.fn.extractLane(b, idx - type.lanes); } return simdCreate(type); } @@ -355,7 +353,7 @@ if (typeof SIMD.Float32x4 === "undefined" || SIMD.Float32x4.extractLane = function(v, i) { v = SIMD.Float32x4.check(v); - simdCheckLaneIndex(i, 4); + i = simdCoerceLaneIndex(i, 4); return v.s_[i]; } } @@ -386,7 +384,7 @@ if (typeof SIMD.Int32x4 === "undefined" || SIMD.Int32x4.extractLane = function(v, i) { v = SIMD.Int32x4.check(v); - simdCheckLaneIndex(i, 4); + i = simdCoerceLaneIndex(i, 4); return v.s_[i]; } } @@ -415,7 +413,7 @@ if (typeof SIMD.Int16x8 === "undefined" || SIMD.Int16x8.extractLane = function(v, i) { v = SIMD.Int16x8.check(v); - simdCheckLaneIndex(i, 8); + i = simdCoerceLaneIndex(i, 8); return v.s_[i]; } } @@ -447,7 +445,7 @@ if (typeof SIMD.Int8x16 === "undefined" || SIMD.Int8x16.extractLane = function(v, i) { v = SIMD.Int8x16.check(v); - simdCheckLaneIndex(i, 16); + i = simdCoerceLaneIndex(i, 16); return v.s_[i]; } } @@ -480,7 +478,7 @@ if (typeof SIMD.Uint32x4 === "undefined" || SIMD.Uint32x4.extractLane = function(v, i) { v = SIMD.Uint32x4.check(v); - simdCheckLaneIndex(i, 4); + i = simdCoerceLaneIndex(i, 4); return v.s_[i]; } } @@ -509,7 +507,7 @@ if (typeof SIMD.Uint16x8 === "undefined" || SIMD.Uint16x8.extractLane = function(v, i) { v = SIMD.Uint16x8.check(v); - simdCheckLaneIndex(i, 8); + i = simdCoerceLaneIndex(i, 8); return v.s_[i]; } } @@ -541,7 +539,7 @@ if (typeof SIMD.Uint8x16 === "undefined" || SIMD.Uint8x16.extractLane = function(v, i) { v = SIMD.Uint8x16.check(v); - simdCheckLaneIndex(i, 16); + i = simdCoerceLaneIndex(i, 16); return v.s_[i]; } } @@ -574,7 +572,7 @@ if (typeof SIMD.Bool32x4 === "undefined" || SIMD.Bool32x4.extractLane = function(v, i) { v = SIMD.Bool32x4.check(v); - simdCheckLaneIndex(i, 4); + i = simdCoerceLaneIndex(i, 4); return v.s_[i]; } } @@ -591,7 +589,7 @@ if (typeof SIMD.Bool16x8 === "undefined" || SIMD.Bool16x8.extractLane = function(v, i) { v = SIMD.Bool16x8.check(v); - simdCheckLaneIndex(i, 8); + i = simdCoerceLaneIndex(i, 8); return v.s_[i]; } } @@ -611,7 +609,7 @@ if (typeof SIMD.Bool8x16 === "undefined" || SIMD.Bool8x16.extractLane = function(v, i) { v = SIMD.Bool8x16.check(v); - simdCheckLaneIndex(i, 16); + i = simdCoerceLaneIndex(i, 16); return v.s_[i]; } } @@ -828,7 +826,7 @@ if (typeof simdPhase2 !== 'undefined') { SIMD.Float64x2.extractLane = function(v, i) { v = SIMD.Float64x2.check(v); - simdCheckLaneIndex(i, 2); + i = simdCoerceLaneIndex(i, 2); return v.s_[i]; } } @@ -857,7 +855,7 @@ if (typeof simdPhase2 !== 'undefined') { SIMD.Bool64x2.extractLane = function(v, i) { v = SIMD.Bool64x2.check(v); - simdCheckLaneIndex(i, 2); + i = simdCoerceLaneIndex(i, 2); return v.s_[i]; } } diff --git a/src/ecmascript_simd_tests.js b/src/ecmascript_simd_tests.js index 33ceffa..75f7ae4 100644 --- a/src/ecmascript_simd_tests.js +++ b/src/ecmascript_simd_tests.js @@ -420,15 +420,30 @@ function testReplaceLane(type) { } } + // Test lane index coercion. + // ToNumber (index) must be an integer in range. + function validLaneIndex(testIndex, intIndex) { + var v = simdConvert(type.interestingValues[type.interestingValues.length - 1]); + var result = type.fn.replaceLane(a, testIndex, v); + checkValue(type, result, + function(i) { + return i == testIndex ? v :type.fn.extractLane(a, i); + }); + } + validLaneIndex(null, 0); + validLaneIndex(false, 0); + validLaneIndex(true, 1); + validLaneIndex("0", 0); + validLaneIndex("00", 0); + validLaneIndex(" +1e0", 1); + function testIndexCheck(index) { throws(function() { type.fn.replaceLane(a, index, 0); }); } testIndexCheck(type.lanes); testIndexCheck(13.37); - testIndexCheck(null); testIndexCheck(undefined); testIndexCheck({}); - testIndexCheck(true); testIndexCheck('yo'); testIndexCheck(-1); testIndexCheck(128); @@ -578,22 +593,29 @@ function testSwizzle(type) { var result = type.fn.swizzle.apply(type.fn, [a].concat(indices)); checkValue(type, result, function(index) { return type.fn.extractLane(a, type.lanes - index - 1); }); - function testIndexCheck(index) { + function testIndexCheck(expectValid, index) { for (var i = 0; i < type.lanes; i++) { var args = [a].concat(indices); args[i + 1] = index; - throws(function() { type.fn.swizzle.apply(type.fn, args); }); + if (expectValid) + type.fn.swizzle.apply(type.fn, args); + else + throws(function() { type.fn.swizzle.apply(type.fn, args); }); } } - testIndexCheck(type.lanes); - testIndexCheck(13.37); - testIndexCheck(null); - testIndexCheck(undefined); - testIndexCheck({}); - testIndexCheck(true); - testIndexCheck('yo'); - testIndexCheck(-1); - testIndexCheck(128); + testIndexCheck(true, null); + testIndexCheck(true, true); + testIndexCheck(true, false); + testIndexCheck(true, "0"); + testIndexCheck(true, "00"); + testIndexCheck(true, " +1e0"); + testIndexCheck(false, type.lanes); + testIndexCheck(false, 13.37); + testIndexCheck(false, undefined); + testIndexCheck(false, {}); + testIndexCheck(false, 'yo'); + testIndexCheck(false, -1); + testIndexCheck(false, 128); } function testShuffle(type) { @@ -621,22 +643,29 @@ function testShuffle(type) { var result = type.fn.shuffle.apply(type.fn, [a, b].concat(indices)); checkValue(type, result, function(index) { return type.fn.extractLane(b, index); }); - function testIndexCheck(index) { + function testIndexCheck(expectValid, index) { for (var i = 0; i < type.lanes; i++) { var args = [a, b].concat(indices); args[i + 2] = index; - throws(function() { type.fn.shuffle.apply(type.fn, args); }); + if (expectValid) + type.fn.shuffle.apply(type.fn, args); + else + throws(function() { type.fn.shuffle.apply(type.fn, args); }); } } - testIndexCheck(2 * type.lanes); - testIndexCheck(13.37); - testIndexCheck(null); - testIndexCheck(undefined); - testIndexCheck({}); - testIndexCheck(true); - testIndexCheck('yo'); - testIndexCheck(-1); - testIndexCheck(128); + testIndexCheck(true, null); + testIndexCheck(true, true); + testIndexCheck(true, false); + testIndexCheck(true, "0"); + testIndexCheck(true, "00"); + testIndexCheck(true, " +1e0"); + testIndexCheck(false, 2 * type.lanes); + testIndexCheck(false, 13.37); + testIndexCheck(false, undefined); + testIndexCheck(false, {}); + testIndexCheck(false, 'yo'); + testIndexCheck(false, -1); + testIndexCheck(false, 128); } function testLoad(type, name, count) { From 14d6406f50a312ee8453273c2820ef60fa69856d Mon Sep 17 00:00:00 2001 From: Jakob Stoklund Olesen Date: Tue, 15 Mar 2016 15:11:36 -0700 Subject: [PATCH 2/3] Reuse the simdCoerceIndex() for coercing lane indexes. --- src/ecmascript_simd.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/ecmascript_simd.js b/src/ecmascript_simd.js index 732c686..025800b 100644 --- a/src/ecmascript_simd.js +++ b/src/ecmascript_simd.js @@ -110,11 +110,9 @@ function simdCoerceIndex(index) { } function simdCoerceLaneIndex(index, lanes) { - index = +index; + index = simdCoerceIndex(index); if (!(index >= 0 && index < lanes)) throw new RangeError('Lane index must be in bounds'); - if (index != Math.floor(index)) - throw new RangeError('Lane index must be an integer'); return index; } From 842d52de34b03131e00a39bde70ecbb83976c124 Mon Sep 17 00:00:00 2001 From: Jakob Stoklund Olesen Date: Tue, 15 Mar 2016 15:12:35 -0700 Subject: [PATCH 3/3] Remove the intIndex argument which was unused. --- src/ecmascript_simd_tests.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/ecmascript_simd_tests.js b/src/ecmascript_simd_tests.js index 75f7ae4..20bcabc 100644 --- a/src/ecmascript_simd_tests.js +++ b/src/ecmascript_simd_tests.js @@ -422,7 +422,7 @@ function testReplaceLane(type) { // Test lane index coercion. // ToNumber (index) must be an integer in range. - function validLaneIndex(testIndex, intIndex) { + function validLaneIndex(testIndex) { var v = simdConvert(type.interestingValues[type.interestingValues.length - 1]); var result = type.fn.replaceLane(a, testIndex, v); checkValue(type, result, @@ -430,12 +430,12 @@ function testReplaceLane(type) { return i == testIndex ? v :type.fn.extractLane(a, i); }); } - validLaneIndex(null, 0); - validLaneIndex(false, 0); - validLaneIndex(true, 1); - validLaneIndex("0", 0); - validLaneIndex("00", 0); - validLaneIndex(" +1e0", 1); + validLaneIndex(null); + validLaneIndex(false); + validLaneIndex(true); + validLaneIndex("0"); + validLaneIndex("00"); + validLaneIndex(" +1e0"); function testIndexCheck(index) { throws(function() { type.fn.replaceLane(a, index, 0); });