1201934 - Remove SIMD shiftRightArithmeticByScalar and shiftRightLogicalByScalar

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jukka Jylänki]

SIMD.js recently renamed the function in the spec, see https://github.com/tc39/ecmascript_simd/commit/97215fd3824168f8dd998b7f2d1a1e0bbcd0cc57 . Please rename the function in the SpiderMonkey implementation, so that SpiderMonkey will validate as asm.js in its SIMD.js support.

See https://github.com/juj/emscripten/commit/629e7f8675d232911cc28516577ed0c25345980d and https://github.com/juj/emscripten/commit/629e7f8675d232911cc28516577ed0c25345980d and https://github.com/kripken/emscripten-fastcomp/pull/117.

Comment 1

[Jukka Jylänki]

I postponed the change to rename these functions in Emscripten. Also, I notice that the function shiftRightArithmeticByScalar was also renamed to shiftRightByScalar in the spec, and the Logical vs Arithmetic is now determined by the SIMD type.

Comment 2

[Jakob Stoklund Olesen [:jolesen]]

I think it would make sense to make this change as part of Bug 1233111 which introduces the unsigned types.

Comment 3

[Jakob Stoklund Olesen [:jolesen]]

To clarify, bug 1233111 will add a shiftRightByScalar function on all of the integer SIMD types. It will leave in place the shiftRightArithmeticByScalar and shiftRightLogicalByScalar functions.

I will use this bug to remove shiftRightArithmeticByScalar and shiftRightLogicalByScalar after bug 1233111 has landed.

Comment 4

[Jukka Jylänki]

Emscripten has now been adapted to not use the shiftRightArithmetic/LogicalByScalar, but only uses the shiftRightByScalar version, so it's not referring to the old spec anymore. See https://github.com/kripken/emscripten/pull/4066

Comment 5

[Jakob Stoklund Olesen [:jolesen]]

Attachment: Remove SIMD shiftRight***ByScalar.

These functions have been replaced by the shiftRightByScalar function which
behaves differently on signed and unsigned SIMD types.

This patch applies on top of the patches from bug 1244889.

Comment 6

[Dan Gohman [:sunfish]]

Comment on attachment 8718498 [details] [diff] [review]
Remove SIMD shiftRight***ByScalar.

Review of attachment 8718498 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit-test/tests/SIMD/shift.js
@@ -41,5 @@
> -        assertEqX4(SIMD.Int32x4.shiftRightLogicalByScalar(v, 0),  a.map(ursh(0)));
> -        assertEqX4(SIMD.Int32x4.shiftRightLogicalByScalar(v, 1),  a.map(ursh(1)));
> -        assertEqX4(SIMD.Int32x4.shiftRightLogicalByScalar(v, 2),  a.map(ursh(2)));
> -        assertEqX4(SIMD.Int32x4.shiftRightLogicalByScalar(v, 31), a.map(ursh(31)));
> -        assertEqX4(SIMD.Int32x4.shiftRightLogicalByScalar(v, 32), a.map(ursh(32)));

Could we keep these tests, using SIMD.Uint32x4.fromInt32x4Bits(v).shiftRightByScalar? AFAIK this is the only non-asm.js coverage of these operations in jit-test.

@@ -54,5 @@
>          // Non constant shift counts
>          var c = shifts[i % shifts.length];
>          assertEqX4(SIMD.Int32x4.shiftLeftByScalar(v, c), a.map(lsh(c)));
> -        assertEqX4(SIMD.Int32x4.shiftRightArithmeticByScalar(v, c), a.map(rsh(c)));
> -        assertEqX4(SIMD.Int32x4.shiftRightLogicalByScalar(v, c), a.map(ursh(c)));

Ditto :).

Comment 7

[Jakob Stoklund Olesen [:jolesen]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d170c3f007e0

Comment 8

[Jakob Stoklund Olesen [:jolesen]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0af8ffcf68700145b0885e85ce396533e20430eb
Bug 1201934 - Remove SIMD shiftRight***ByScalar. r=sunfish

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0af8ffcf6870

Comment 10

[Florian Scholz (Open Web Docs)]

Redirected
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SIMD/shiftRightArithmeticByScalar and
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SIMD/shiftRightLogicalByScalar to
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SIMD/shiftRightByScalar