Commit 3e828fd2 authored by Ng Zhi An's avatar Ng Zhi An Committed by Commit Bot

Reland "[wasm-simd] Fix extract lane unsigned extend"

This is a reland of dfdef885

Original change's description:
> [wasm-simd] Fix extract lane unsigned extend
> 
> 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}

Change-Id: Ica7974a2f1f2a4f07b54cc68f9abcf5e121a9262
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2219414
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Reviewed-by: 's avatarDeepti Gandluri <gdeepti@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68063}
parent 109de468
......@@ -2215,22 +2215,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(); \
......
......@@ -3682,6 +3682,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); \
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment