Commit ff9cc7e0 authored by Ng Zhi An's avatar Ng Zhi An Committed by Commit Bot

[wasm-simd][scalar-lowering] Fix v128.const lowering

v128.const was incorrectly always lowered to 4 word32 nodes, regardless
of what the lowered type was set to be.

In the test case, v128.const was consumed by i8x16.eq, so the lowered
typed of v128.const node was set to SimdType::kInt8x16, but it was still
lowered as a SimdType::kInt32x4, and then later crashes when lowering
the comparisons.

Bug: v8:10507
Change-Id: I24f16c94968cd8b6c7cd5d400d1a0046da3d47da
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2391919
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Reviewed-by: 's avatarBill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69755}
parent 1554c7ee
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "src/compiler/simd-scalar-lowering.h" #include "src/compiler/simd-scalar-lowering.h"
#include "src/codegen/machine-type.h" #include "src/codegen/machine-type.h"
#include "src/common/globals.h"
#include "src/compiler/diamond.h" #include "src/compiler/diamond.h"
#include "src/compiler/linkage.h" #include "src/compiler/linkage.h"
#include "src/compiler/machine-operator.h" #include "src/compiler/machine-operator.h"
...@@ -1095,16 +1096,45 @@ void SimdScalarLowering::LowerNode(Node* node) { ...@@ -1095,16 +1096,45 @@ void SimdScalarLowering::LowerNode(Node* node) {
int num_lanes = NumLanes(rep_type); int num_lanes = NumLanes(rep_type);
switch (node->opcode()) { switch (node->opcode()) {
case IrOpcode::kS128Const: { case IrOpcode::kS128Const: {
// Lower 128.const to 4 Int32Constant. // We could use GetReplacementsWithType for all this, but it adds a lot of
// nodes, so sign extend the immediates ourselves here.
DCHECK_EQ(0, node->InputCount()); DCHECK_EQ(0, node->InputCount());
constexpr int kNumLanes = kSimd128Size / sizeof(uint32_t); Node** rep_node = zone()->NewArray<Node*>(num_lanes);
uint32_t val[kNumLanes]; S128ImmediateParameter params = S128ImmediateParameterOf(node->op());
memcpy(val, S128ImmediateParameterOf(node->op()).data(), kSimd128Size);
Node** rep_node = zone()->NewArray<Node*>(kNumLanes); // For all the small ints below, we have a choice of static_cast or bit
for (int i = 0; i < kNumLanes; ++i) { // twiddling, clang seems to be able to optimize either
rep_node[i] = mcgraph_->Int32Constant(val[i]); // (https://godbolt.org/z/9c65o8) so use static_cast for clarity.
switch (rep_type) {
case SimdType::kInt8x16: {
for (int i = 0; i < num_lanes; ++i) {
rep_node[i] = mcgraph_->Int32Constant(
static_cast<int32_t>(static_cast<int8_t>(params.data()[i])));
}
break;
}
case SimdType::kInt16x8: {
int16_t val[kNumLanes16];
memcpy(val, params.data(), kSimd128Size);
for (int i = 0; i < num_lanes; ++i) {
rep_node[i] = mcgraph_->Int32Constant(static_cast<int32_t>(val[i]));
}
break;
}
case SimdType::kInt32x4:
case SimdType::kFloat32x4: {
uint32_t val[kNumLanes32];
memcpy(val, params.data(), kSimd128Size);
for (int i = 0; i < num_lanes; ++i) {
rep_node[i] = mcgraph_->Int32Constant(val[i]);
}
break;
}
default: {
UNIMPLEMENTED();
}
} }
ReplaceNode(node, rep_node, kNumLanes); ReplaceNode(node, rep_node, num_lanes);
break; break;
} }
case IrOpcode::kStart: { case IrOpcode::kStart: {
......
...@@ -65,6 +65,27 @@ WASM_SIMD_TEST(F32x4) { ...@@ -65,6 +65,27 @@ WASM_SIMD_TEST(F32x4) {
CHECK_EQ(0x80000001, r.Call(1)); CHECK_EQ(0x80000001, r.Call(1));
} }
WASM_SIMD_TEST(I8x16Eq_ToTest_S128Const) {
// Test implementation of S128Const in scalar lowering, this test case was
// causing a crash.
TestSignatures sigs;
WasmRunner<uint32_t> r(execution_tier, lower_simd);
byte c1[16] = {0x00, 0x00, 0x80, 0xbf, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x80, 0x3f, 0x00, 0x00, 0x00, 0x40};
byte c2[16] = {0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00, 0x00,
0x01, 0x01, 0x01, 0x01, 0x02, 0x02, 0x02, 0x02};
byte c3[16] = {0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
BUILD(r,
WASM_SIMD_BINOP(kExprI8x16Eq, WASM_SIMD_CONSTANT(c1),
WASM_SIMD_CONSTANT(c2)),
WASM_SIMD_CONSTANT(c3), WASM_SIMD_OP(kExprI8x16Eq),
WASM_SIMD_OP(kExprI8x16ExtractLaneS), TO_BYTE(4));
CHECK_EQ(0xffffffff, r.Call());
}
} // namespace test_run_wasm_simd } // namespace test_run_wasm_simd
} // namespace wasm } // namespace wasm
} // namespace internal } // namespace internal
......
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