Closed Bug 1201934 Opened 4 years ago Closed 4 years ago

Remove SIMD shiftRightArithmeticByScalar and shiftRightLogicalByScalar


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
firefox47 --- fixed


(Reporter: jujjyl, Assigned: jolesen)



(Keywords: dev-doc-complete, Whiteboard: [DocArea=JS])


(1 file)

SIMD.js recently renamed the function in the spec, see . Please rename the function in the SpiderMonkey implementation, so that SpiderMonkey will validate as asm.js in its SIMD.js support.

See and and
Keywords: dev-doc-needed
Whiteboard: [DocArea=JS]
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.
I think it would make sense to make this change as part of Bug 1233111 which introduces the unsigned types.
Depends on: 1233111
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.
Assignee: nobody → jolesen
Summary: Rename SIMD.js shiftRightLogicalByScalar to shiftRightByScalar. → Remove SIMD shiftRightArithmeticByScalar and shiftRightLogicalByScalar
Blocks: 894105
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
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.
Attachment #8718498 - Flags: review?(sunfish)
Depends on: 1244889
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),;
> -        assertEqX4(SIMD.Int32x4.shiftRightLogicalByScalar(v, 1),;
> -        assertEqX4(SIMD.Int32x4.shiftRightLogicalByScalar(v, 2),;
> -        assertEqX4(SIMD.Int32x4.shiftRightLogicalByScalar(v, 31),;
> -        assertEqX4(SIMD.Int32x4.shiftRightLogicalByScalar(v, 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),;
> -        assertEqX4(SIMD.Int32x4.shiftRightArithmeticByScalar(v, c),;
> -        assertEqX4(SIMD.Int32x4.shiftRightLogicalByScalar(v, c),;

Ditto :).
Attachment #8718498 - Flags: review?(sunfish) → review+
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.