AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[wasm-simd] Fix extract lane unsigned extend

Browse Source 
The interpreter is missing a static cast when extracting lanes smaller
than int32_t and doing an unsigned extend. The array in Simd128 is
signed, so a direct cast to uint32_t will be a signed extension. The fix
is to, in the unsigned case, cast to unsigned (of the appropriate size)
first, then cast to uint32_t.

Change-Id: Ifabb5b9690f08ad505ac94b84908db0970581818
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2216721
Reviewed-by: Deepti Gandluri <gdeepti@chromium.org>
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68029}
This commit is contained in:
Ng Zhi An 2020-05-27 14:07:49 -07:00 committed by Commit Bot
parent 5f90cfd754
commit dfdef88547
2 changed files with 35 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
src/wasm/wasm-interpreter.cc
View File

@ -2221,22 +2221,38 @@ class ThreadImpl {
EXTRACT_LANE_CASE(I64x2, i64x2)
EXTRACT_LANE_CASE(I32x4, i32x4)
#undef EXTRACT_LANE_CASE
#define EXTRACT_LANE_EXTEND_CASE(format, name, sign, type) \
case kExpr##format##ExtractLane##sign: { \
SimdLaneImmediate<Decoder::kNoValidate> imm(decoder, code->at(pc), \
opcode_length); \
*len += 1; \
WasmValue val = Pop(); \
Simd128 s = val.to_s128(); \
auto ss = s.to_##name(); \
Push(WasmValue(static_cast<type>(ss.val[LANE(imm.lane, ss)]))); \
return true; \
// Unsigned extracts require a bit more care. The underlying array in
// Simd128 is signed (see wasm-value.h), so when casted to uint32_t it
// will be signed extended, e.g. int8_t -> int32_t -> uint32_t. So for
// unsigned extracts, we will cast it int8_t -> uint8_t -> uint32_t. We
// add the DCHECK to ensure that if the array type changes, we know to
// change this function.
#define EXTRACT_LANE_EXTEND_CASE(format, name, sign, extended_type) \
case kExpr##format##ExtractLane##sign: { \
SimdLaneImmediate<Decoder::kNoValidate> imm(decoder, code->at(pc), \
opcode_length); \
*len += 1; \
WasmValue val = Pop(); \
Simd128 s = val.to_s128(); \
auto ss = s.to_##name(); \
auto res = ss.val[LANE(imm.lane, ss)]; \
DCHECK(std::is_signed<decltype(res)>::value); \
if (std::is_unsigned<extended_type>::value) { \
using unsigned_type = std::make_unsigned<decltype(res)>::type; \
Push(WasmValue( \
static_cast<extended_type>(static_cast<unsigned_type>(res)))); \
} else { \
Push(WasmValue(static_cast<extended_type>(res))); \
} \
return true; \
}
EXTRACT_LANE_EXTEND_CASE(I16x8, i16x8, S, int32_t)
EXTRACT_LANE_EXTEND_CASE(I16x8, i16x8, U, uint32_t)
EXTRACT_LANE_EXTEND_CASE(I8x16, i8x16, S, int32_t)
EXTRACT_LANE_EXTEND_CASE(I8x16, i8x16, U, uint32_t)
#undef EXTRACT_LANE_EXTEND_CASE
#define BINOP_CASE(op, name, stype, count, expr) \
case kExpr##op: { \
WasmValue v2 = Pop(); \

9
test/cctest/wasm/test-run-wasm-simd.cc
View File

@ -3658,6 +3658,15 @@ WASM_SIMD_TEST_NO_LOWERING(I16x8GtUMixed) {
UnsignedGreater);
}
WASM_SIMD_TEST_NO_LOWERING(I16x8ExtractLaneU_I8x16Splat) {
// Test that we are correctly signed/unsigned extending when extracting.
WasmRunner<int32_t, int32_t> r(execution_tier, lower_simd);
byte simd_val = r.AllocateLocal(kWasmS128);
BUILD(r, WASM_SET_LOCAL(simd_val, WASM_SIMD_I8x16_SPLAT(WASM_GET_LOCAL(0))),
WASM_SIMD_I16x8_EXTRACT_LANE_U(0, WASM_GET_LOCAL(simd_val)));
CHECK_EQ(0xfafa, r.Call(0xfa));
}
#define WASM_EXTRACT_I16x8_TEST(Sign, Type) \
WASM_SIMD_TEST(I16X8ExtractLane##Sign) { \
WasmRunner<int32_t, int32_t> r(execution_tier, lower_simd); \