[wasm-simd] Fix I8x16UConvertI16x8 in interpreter

We were hitting an implementation defined behavior in this instruction:

- v is clamped to uint8_t::min and uint8_t::max
- then we static_cast<int8_t>(v)
- any values that don't fit in int8_t (> 127) hits and implementation
defined behavior

We reuse base::saturated_cast here instead to avoid this undefined
behavior.

Drive-by cleanup of test cases to make the signed/unsigned cases more
explicity.

Bug: v8:11372
Change-Id: I4e92cdfb685d74bd5436ba25f1c00db49a231221
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2659501
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72491}
This commit is contained in:
Ng Zhi An 2021-02-02 09:26:54 -08:00 committed by Commit Bot
parent ef3e9cb059
commit 2e9b0e90c2
2 changed files with 23 additions and 26 deletions

View File

@ -2757,8 +2757,8 @@ WASM_SIMD_TEST_NO_LOWERING(I8x16Popcnt) {
WASM_SIMD_TEST(I8x16ConvertI16x8) {
WasmRunner<int32_t, int32_t> r(execution_tier, lower_simd);
// Create output vectors to hold signed and unsigned results.
int8_t* g0 = r.builder().AddGlobal<int8_t>(kWasmS128);
int8_t* g1 = r.builder().AddGlobal<int8_t>(kWasmS128);
int8_t* g_s = r.builder().AddGlobal<int8_t>(kWasmS128);
uint8_t* g_u = r.builder().AddGlobal<uint8_t>(kWasmS128);
// Build fn to splat test value, perform conversions, and write the results.
byte value = 0;
byte temp1 = r.AllocateLocal(kWasmS128);
@ -2774,10 +2774,10 @@ WASM_SIMD_TEST(I8x16ConvertI16x8) {
FOR_INT16_INPUTS(x) {
r.Call(x);
int8_t expected_signed = base::saturated_cast<int8_t>(x);
int8_t expected_unsigned = base::saturated_cast<uint8_t>(x);
uint8_t expected_unsigned = base::saturated_cast<uint8_t>(x);
for (int i = 0; i < 16; i++) {
CHECK_EQ(expected_signed, ReadLittleEndianValue<int8_t>(&g0[i]));
CHECK_EQ(expected_unsigned, ReadLittleEndianValue<int8_t>(&g1[i]));
CHECK_EQ(expected_signed, ReadLittleEndianValue<int8_t>(&g_s[i]));
CHECK_EQ(expected_unsigned, ReadLittleEndianValue<uint8_t>(&g_u[i]));
}
}
}

View File

@ -2513,28 +2513,25 @@ class WasmInterpreterInternals {
CONVERT_CASE(F64x2PromoteLowF32x4, float4, f32x4, float2, 2, 0, float,
static_cast<double>(a))
#undef CONVERT_CASE
#define PACK_CASE(op, src_type, name, dst_type, count, ctype, dst_ctype) \
case kExpr##op: { \
WasmValue v2 = Pop(); \
WasmValue v1 = Pop(); \
src_type s1 = v1.to_s128().to_##name(); \
src_type s2 = v2.to_s128().to_##name(); \
dst_type res; \
int64_t min = std::numeric_limits<ctype>::min(); \
int64_t max = std::numeric_limits<ctype>::max(); \
for (size_t i = 0; i < count; ++i) { \
int64_t v = i < count / 2 ? s1.val[LANE(i, s1)] \
: s2.val[LANE(i - count / 2, s2)]; \
res.val[LANE(i, res)] = \
static_cast<dst_ctype>(std::max(min, std::min(max, v))); \
} \
Push(WasmValue(Simd128(res))); \
return true; \
#define PACK_CASE(op, src_type, name, dst_type, count, dst_ctype) \
case kExpr##op: { \
WasmValue v2 = Pop(); \
WasmValue v1 = Pop(); \
src_type s1 = v1.to_s128().to_##name(); \
src_type s2 = v2.to_s128().to_##name(); \
dst_type res; \
for (size_t i = 0; i < count; ++i) { \
int64_t v = i < count / 2 ? s1.val[LANE(i, s1)] \
: s2.val[LANE(i - count / 2, s2)]; \
res.val[LANE(i, res)] = base::saturated_cast<dst_ctype>(v); \
} \
Push(WasmValue(Simd128(res))); \
return true; \
}
PACK_CASE(I16x8SConvertI32x4, int4, i32x4, int8, 8, int16_t, int16_t)
PACK_CASE(I16x8UConvertI32x4, int4, i32x4, int8, 8, uint16_t, int16_t)
PACK_CASE(I8x16SConvertI16x8, int8, i16x8, int16, 16, int8_t, int8_t)
PACK_CASE(I8x16UConvertI16x8, int8, i16x8, int16, 16, uint8_t, int8_t)
PACK_CASE(I16x8SConvertI32x4, int4, i32x4, int8, 8, int16_t)
PACK_CASE(I16x8UConvertI32x4, int4, i32x4, int8, 8, uint16_t)
PACK_CASE(I8x16SConvertI16x8, int8, i16x8, int16, 16, int8_t)
PACK_CASE(I8x16UConvertI16x8, int8, i16x8, int16, 16, uint8_t)
#undef PACK_CASE
case kExprS128Select: {
int4 bool_val = Pop().to_s128().to_i32x4();