Skip to content

Commit

Permalink
Bug 1669938 - Promote fp rounding wasm SIMD instructions to accepted …
Browse files Browse the repository at this point in the history
…status. r=jseward

Background: WebAssembly/simd#232

For all the rounding SIMD instructions:

- remove the internal 'Experimental' opcode suffix in the C++ code
- remove the guard on experimental Wasm instructions in all the C++ decoders
- move the test cases from simd/experimental.js to simd/ad-hack.js

I have checked that current V8 and wasm-tools use the same opcode
mappings.  V8 in turn guarantees the correct mapping for LLVM and
binaryen.

Drive-by bug fix: the test predicate for f64 square root was wrong, it
would round its argument to float.  This did not matter for the test
inputs we had but started to matter when I added more difficult inputs
for testing rounding.

Differential Revision: https://phabricator.services.mozilla.com/D92926
  • Loading branch information
Lars T Hansen committed Oct 14, 2020
1 parent e015128 commit dec2b41
Show file tree
Hide file tree
Showing 9 changed files with 68 additions and 109 deletions.
21 changes: 19 additions & 2 deletions js/src/jit-test/tests/wasm/simd/ad-hack.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ BigInt64Array.rectify = (x) => BigInt(x);

Float32Array.inputs = [[1, -1, 1e10, -1e10],
[-1, -2, -1e10, 1e10],
[5.1, -1.1, -4.3, -0],
...permute([1, -10, NaN, Infinity])];
Float32Array.rectify = (x) => Math.fround(x);

Expand Down Expand Up @@ -812,8 +813,16 @@ function iabs(bits) { return (a) => zero_extend(a < 0 ? -a : a, bits) }
function fneg(a) { return -a }
function fabs(a) { return Math.abs(a) }
function fsqrt(a) { return Math.fround(Math.sqrt(Math.fround(a))) }
function sqrt(a) { return Math.sqrt(Math.fround(a)) }
function dsqrt(a) { return Math.sqrt(a) }
function bitnot(a) { return (~a) & 255 }
function ffloor(x) { return Math.fround(Math.floor(x)) }
function fceil(x) { return Math.fround(Math.ceil(x)) }
function ftrunc(x) { return Math.fround(Math.sign(x)*Math.floor(Math.abs(x))) }
function fnearest(x) { return Math.fround(Math.round(x)) }
function dfloor(x) { return Math.floor(x) }
function dceil(x) { return Math.ceil(x) }
function dtrunc(x) { return Math.sign(x)*Math.floor(Math.abs(x)) }
function dnearest(x) { return Math.round(x) }

for ( let [op, memtype, rop, resultmemtype] of
[['i8x16.neg', Int8Array, ineg(8)],
Expand All @@ -828,7 +837,15 @@ for ( let [op, memtype, rop, resultmemtype] of
['f32x4.abs', Float32Array, fabs],
['f64x2.abs', Float64Array, fabs],
['f32x4.sqrt', Float32Array, fsqrt],
['f64x2.sqrt', Float64Array, sqrt],
['f64x2.sqrt', Float64Array, dsqrt],
['f32x4.ceil', Float32Array, fceil],
['f32x4.floor', Float32Array, ffloor],
['f32x4.trunc', Float32Array, ftrunc],
['f32x4.nearest', Float32Array, fnearest],
['f64x2.ceil', Float64Array, dceil],
['f64x2.floor', Float64Array, dfloor],
['f64x2.trunc', Float64Array, dtrunc],
['f64x2.nearest', Float64Array, dnearest],
['v128.not', Uint8Array, bitnot],
])
{
Expand Down
44 changes: 0 additions & 44 deletions js/src/jit-test/tests/wasm/simd/experimental.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,6 @@ function iota(len) {
function pmin(x, y) { return y < x ? y : x }
function pmax(x, y) { return x < y ? y : x }

function ffloor(x) { return Math.fround(Math.floor(x)) }
function fceil(x) { return Math.fround(Math.ceil(x)) }
function ftrunc(x) { return Math.fround(Math.sign(x)*Math.floor(Math.abs(x))) }
function fnearest(x) { return Math.fround(Math.round(x)) }

function dfloor(x) { return Math.floor(x) }
function dceil(x) { return Math.ceil(x) }
function dtrunc(x) { return Math.sign(x)*Math.floor(Math.abs(x)) }
function dnearest(x) { return Math.round(x) }

const v2vSig = {args:[], ret:VoidCode};

function V128Load(addr) {
Expand Down Expand Up @@ -138,40 +128,6 @@ ins.exports.run();
var result = get(mem32, 0, 4);
assertSame(result, ans);

// Rounding, https://github.com/WebAssembly/simd/pull/232

var fxs = [5.1, -1.1, -4.3, 0];
var dxs = [5.1, -1.1];

for ( let [opcode, xs, operator] of [[F32x4CeilCode, fxs, fceil],
[F32x4FloorCode, fxs, ffloor],
[F32x4TruncCode, fxs, ftrunc],
[F32x4NearestCode, fxs, fnearest],
[F64x2CeilCode, dxs, dceil],
[F64x2FloorCode, dxs, dfloor],
[F64x2TruncCode, dxs, dtrunc],
[F64x2NearestCode, dxs, dnearest]] ) {
var k = xs.length;
var ans = xs.map(operator);

var ins = wasmEval(moduleWithSections([
sigSection([v2vSig]),
declSection([0]),
memorySection(1),
exportSection([{funcIndex: 0, name: "run"},
{memIndex: 0, name: "mem"}]),
bodySection([
funcBody({locals:[],
body: [...V128StoreExpr(0, [...V128Load(16),
SimdPrefix, varU32(opcode)])]})])]));

var mem = new (k == 4 ? Float32Array : Float64Array)(ins.exports.mem.buffer);
set(mem, k, xs);
ins.exports.run();
var result = get(mem, 0, k);
assertSame(result, ans);
}

// Zero-extending SIMD load, https://github.com/WebAssembly/simd/pull/237

for ( let [opcode, k, log2align, cons, cast] of [[V128Load32ZeroCode, 4, 2, Int32Array, Number],
Expand Down
3 changes: 1 addition & 2 deletions js/src/jit/MacroAssembler.h
Original file line number Diff line number Diff line change
Expand Up @@ -2674,8 +2674,7 @@ class MacroAssembler : public MacroAssemblerSpecific {
inline void widenDotInt16x8(FloatRegister rhs, FloatRegister lhsDest)
DEFINED_ON(x86_shared, arm64);

// Floating point rounding (experimental as of August, 2020)
// https://github.com/WebAssembly/simd/pull/232
// Floating point rounding

inline void ceilFloat32x4(FloatRegister src, FloatRegister dest)
DEFINED_ON(x86_shared, arm64);
Expand Down
16 changes: 8 additions & 8 deletions js/src/jit/x86-shared/CodeGenerator-x86-shared.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3136,28 +3136,28 @@ void CodeGenerator::visitWasmUnarySimd128(LWasmUnarySimd128* ins) {
case wasm::SimdOp::I32x4Abs:
masm.absInt32x4(src, dest);
break;
case wasm::SimdOp::F32x4CeilExperimental:
case wasm::SimdOp::F32x4Ceil:
masm.ceilFloat32x4(src, dest);
break;
case wasm::SimdOp::F32x4FloorExperimental:
case wasm::SimdOp::F32x4Floor:
masm.floorFloat32x4(src, dest);
break;
case wasm::SimdOp::F32x4TruncExperimental:
case wasm::SimdOp::F32x4Trunc:
masm.truncFloat32x4(src, dest);
break;
case wasm::SimdOp::F32x4NearestExperimental:
case wasm::SimdOp::F32x4Nearest:
masm.nearestFloat32x4(src, dest);
break;
case wasm::SimdOp::F64x2CeilExperimental:
case wasm::SimdOp::F64x2Ceil:
masm.ceilFloat64x2(src, dest);
break;
case wasm::SimdOp::F64x2FloorExperimental:
case wasm::SimdOp::F64x2Floor:
masm.floorFloat64x2(src, dest);
break;
case wasm::SimdOp::F64x2TruncExperimental:
case wasm::SimdOp::F64x2Trunc:
masm.truncFloat64x2(src, dest);
break;
case wasm::SimdOp::F64x2NearestExperimental:
case wasm::SimdOp::F64x2Nearest:
masm.nearestFloat64x2(src, dest);
break;
default:
Expand Down
24 changes: 8 additions & 16 deletions js/src/wasm/WasmBaselineCompile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15037,29 +15037,21 @@ bool BaseCompiler::emitBody() {
CHECK_NEXT(dispatchVectorUnary(AbsI16x8));
case uint32_t(SimdOp::I32x4Abs):
CHECK_NEXT(dispatchVectorUnary(AbsI32x4));
case uint32_t(SimdOp::F32x4CeilExperimental):
CHECK_SIMD_EXPERIMENTAL();
case uint32_t(SimdOp::F32x4Ceil):
CHECK_NEXT(dispatchVectorUnary(CeilF32x4));
case uint32_t(SimdOp::F32x4FloorExperimental):
CHECK_SIMD_EXPERIMENTAL();
case uint32_t(SimdOp::F32x4Floor):
CHECK_NEXT(dispatchVectorUnary(FloorF32x4));
case uint32_t(SimdOp::F32x4TruncExperimental):
CHECK_SIMD_EXPERIMENTAL();
case uint32_t(SimdOp::F32x4Trunc):
CHECK_NEXT(dispatchVectorUnary(TruncF32x4));
case uint32_t(SimdOp::F32x4NearestExperimental):
CHECK_SIMD_EXPERIMENTAL();
case uint32_t(SimdOp::F32x4Nearest):
CHECK_NEXT(dispatchVectorUnary(NearestF32x4));
case uint32_t(SimdOp::F64x2CeilExperimental):
CHECK_SIMD_EXPERIMENTAL();
case uint32_t(SimdOp::F64x2Ceil):
CHECK_NEXT(dispatchVectorUnary(CeilF64x2));
case uint32_t(SimdOp::F64x2FloorExperimental):
CHECK_SIMD_EXPERIMENTAL();
case uint32_t(SimdOp::F64x2Floor):
CHECK_NEXT(dispatchVectorUnary(FloorF64x2));
case uint32_t(SimdOp::F64x2TruncExperimental):
CHECK_SIMD_EXPERIMENTAL();
case uint32_t(SimdOp::F64x2Trunc):
CHECK_NEXT(dispatchVectorUnary(TruncF64x2));
case uint32_t(SimdOp::F64x2NearestExperimental):
CHECK_SIMD_EXPERIMENTAL();
case uint32_t(SimdOp::F64x2Nearest):
CHECK_NEXT(dispatchVectorUnary(NearestF64x2));
case uint32_t(SimdOp::I8x16Shl):
CHECK_NEXT(dispatchVectorVariableShift(ShiftLeftI8x16));
Expand Down
16 changes: 8 additions & 8 deletions js/src/wasm/WasmConstants.h
Original file line number Diff line number Diff line change
Expand Up @@ -642,14 +642,14 @@ enum class SimdOp {
I64x2Mul = 0xd5,
// MinS = 0xd6
// MinU = 0xd7
F32x4CeilExperimental = 0xd8,
F32x4FloorExperimental = 0xd9,
F32x4TruncExperimental = 0xda,
F32x4NearestExperimental = 0xdb,
F64x2CeilExperimental = 0xdc,
F64x2FloorExperimental = 0xdd,
F64x2TruncExperimental = 0xde,
F64x2NearestExperimental = 0xdf,
F32x4Ceil = 0xd8,
F32x4Floor = 0xd9,
F32x4Trunc = 0xda,
F32x4Nearest = 0xdb,
F64x2Ceil = 0xdc,
F64x2Floor = 0xdd,
F64x2Trunc = 0xde,
F64x2Nearest = 0xdf,
F32x4Abs = 0xe0,
F32x4Neg = 0xe1,
// Round = 0xe2
Expand Down
18 changes: 8 additions & 10 deletions js/src/wasm/WasmIonCompile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4950,16 +4950,14 @@ static bool EmitBodyExprs(FunctionCompiler& f) {
case uint32_t(SimdOp::I8x16Abs):
case uint32_t(SimdOp::I16x8Abs):
case uint32_t(SimdOp::I32x4Abs):
CHECK(EmitUnarySimd128(f, SimdOp(op.b1)));
case uint32_t(SimdOp::F32x4CeilExperimental):
case uint32_t(SimdOp::F32x4FloorExperimental):
case uint32_t(SimdOp::F32x4TruncExperimental):
case uint32_t(SimdOp::F32x4NearestExperimental):
case uint32_t(SimdOp::F64x2CeilExperimental):
case uint32_t(SimdOp::F64x2FloorExperimental):
case uint32_t(SimdOp::F64x2TruncExperimental):
case uint32_t(SimdOp::F64x2NearestExperimental):
CHECK_SIMD_EXPERIMENTAL();
case uint32_t(SimdOp::F32x4Ceil):
case uint32_t(SimdOp::F32x4Floor):
case uint32_t(SimdOp::F32x4Trunc):
case uint32_t(SimdOp::F32x4Nearest):
case uint32_t(SimdOp::F64x2Ceil):
case uint32_t(SimdOp::F64x2Floor):
case uint32_t(SimdOp::F64x2Trunc):
case uint32_t(SimdOp::F64x2Nearest):
CHECK(EmitUnarySimd128(f, SimdOp(op.b1)));
case uint32_t(SimdOp::I8x16AnyTrue):
case uint32_t(SimdOp::I16x8AnyTrue):
Expand Down
16 changes: 8 additions & 8 deletions js/src/wasm/WasmOpIter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -462,14 +462,14 @@ OpKind wasm::Classify(OpBytes op) {
case SimdOp::I8x16Abs:
case SimdOp::I16x8Abs:
case SimdOp::I32x4Abs:
case SimdOp::F32x4CeilExperimental:
case SimdOp::F32x4FloorExperimental:
case SimdOp::F32x4TruncExperimental:
case SimdOp::F32x4NearestExperimental:
case SimdOp::F64x2CeilExperimental:
case SimdOp::F64x2FloorExperimental:
case SimdOp::F64x2TruncExperimental:
case SimdOp::F64x2NearestExperimental:
case SimdOp::F32x4Ceil:
case SimdOp::F32x4Floor:
case SimdOp::F32x4Trunc:
case SimdOp::F32x4Nearest:
case SimdOp::F64x2Ceil:
case SimdOp::F64x2Floor:
case SimdOp::F64x2Trunc:
case SimdOp::F64x2Nearest:
WASM_SIMD_OP(OpKind::Unary);
case SimdOp::I8x16Shl:
case SimdOp::I8x16ShrS:
Expand Down
19 changes: 8 additions & 11 deletions js/src/wasm/WasmValidate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1088,17 +1088,14 @@ static bool DecodeFunctionBodyExprs(const ModuleEnvironment& env,
case uint32_t(SimdOp::I8x16Abs):
case uint32_t(SimdOp::I16x8Abs):
case uint32_t(SimdOp::I32x4Abs):
CHECK(iter.readUnary(ValType::V128, &nothing));

case uint32_t(SimdOp::F32x4CeilExperimental):
case uint32_t(SimdOp::F32x4FloorExperimental):
case uint32_t(SimdOp::F32x4TruncExperimental):
case uint32_t(SimdOp::F32x4NearestExperimental):
case uint32_t(SimdOp::F64x2CeilExperimental):
case uint32_t(SimdOp::F64x2FloorExperimental):
case uint32_t(SimdOp::F64x2TruncExperimental):
case uint32_t(SimdOp::F64x2NearestExperimental):
CHECK_SIMD_EXPERIMENTAL();
case uint32_t(SimdOp::F32x4Ceil):
case uint32_t(SimdOp::F32x4Floor):
case uint32_t(SimdOp::F32x4Trunc):
case uint32_t(SimdOp::F32x4Nearest):
case uint32_t(SimdOp::F64x2Ceil):
case uint32_t(SimdOp::F64x2Floor):
case uint32_t(SimdOp::F64x2Trunc):
case uint32_t(SimdOp::F64x2Nearest):
CHECK(iter.readUnary(ValType::V128, &nothing));

case uint32_t(SimdOp::I8x16Shl):
Expand Down

0 comments on commit dec2b41

Please sign in to comment.