[wasm-simd][scalar-lowering] Fix lowering of functions returning f32x4

Functions with v128 in their signatures are always lowered to 4 word32.
So if a return happens to be have an input that is a f32x4 operation, we
get a register allocator error because it tries to fit a float into a
general register. To fix that we need to do some checks when lowering
kReturn, and for each input node, if we are returning a v128, and it is
to be lowered into 4 f32 nodes, we bitcast the floats to ints.

Bug: v8:10507
Change-Id: Iea2fdfc4057304ebf0898e6f7091124629c589f0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2391331
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Reviewed-by: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69705}
This commit is contained in:
Ng Zhi An 2020-09-03 10:28:44 -07:00 committed by Commit Bot
parent 8654df00f4
commit 1f7cb7e1c1
3 changed files with 61 additions and 0 deletions

View File

@ -1187,6 +1187,24 @@ void SimdScalarLowering::LowerNode(Node* node) {
break;
}
case IrOpcode::kReturn: {
// V128 return types are lowered to i32x4, so if the inputs to kReturn are
// not Word32, we need to cast them.
int return_arity = static_cast<int>(signature()->return_count());
for (int i = 0; i < return_arity; i++) {
if (signature()->GetReturn(i) != MachineRepresentation::kSimd128) {
continue;
}
// Input 1 of kReturn is not used.
Node* input = node->InputAt(i + 1);
if (HasReplacement(0, input)) {
// f64x2 isn't handled anywhere yet, so we ignore it here for now.
Replacement rep = replacements_[input->id()];
if (rep.type == SimdType::kFloat32x4) {
Node** rep = GetReplacementsWithType(input, rep_type);
ReplaceNode(input, rep, NumLanes(rep_type));
}
}
}
DefaultLowering(node);
int new_return_count = GetReturnCountAfterLoweringSimd128(signature());
if (static_cast<int>(signature()->return_count()) != new_return_count) {
@ -1214,6 +1232,21 @@ void SimdScalarLowering::LowerNode(Node* node) {
}
size_t return_arity = call_descriptor->ReturnCount();
if (return_arity == 1) {
// We access the additional return values through projections.
// Special case for return_arity 1, with multi-returns, we would have
// already built projections for each return value, and will be handled
// by the following code.
Node* rep_node[kNumLanes32];
for (int i = 0; i < kNumLanes32; ++i) {
rep_node[i] =
graph()->NewNode(common()->Projection(i), node, graph()->start());
}
ReplaceNode(node, rep_node, kNumLanes32);
break;
}
ZoneVector<Node*> projections(return_arity, zone());
NodeProperties::CollectValueProjections(node, projections.data(),
return_arity);

View File

@ -7,6 +7,7 @@
#include "test/cctest/cctest.h"
#include "test/cctest/wasm/wasm-run-utils.h"
#include "test/common/wasm/flag-utils.h"
#include "test/common/wasm/test-signatures.h"
#include "test/common/wasm/wasm-macro-gen.h"
namespace v8 {
@ -42,6 +43,28 @@ WASM_SIMD_TEST(I8x16ToF32x4) {
CHECK_EQ(expected, actual);
}
WASM_SIMD_TEST(F32x4) {
// Check that functions that return F32x4 are correctly lowered into 4 int32
// nodes. The signature of such functions are always lowered to 4 Word32, and
// if the last operation before the return was a f32x4, it will need to be
// bitcasted from float to int.
TestSignatures sigs;
WasmRunner<uint32_t, uint32_t> r(execution_tier, lower_simd);
// A simple function that just calls f32x4.neg on the param.
WasmFunctionCompiler& fn = r.NewFunction(sigs.s_s());
BUILD(fn, WASM_SIMD_UNOP(kExprF32x4Neg, WASM_GET_LOCAL(0)));
// TODO(v8:10507)
// Use i32x4 splat since scalar lowering has a problem with f32x4 as a param
// to a function call, the lowering is not correct yet.
BUILD(r,
WASM_SIMD_I32x4_EXTRACT_LANE(
0, WASM_CALL_FUNCTION(fn.function_index(),
WASM_SIMD_I32x4_SPLAT(WASM_GET_LOCAL(0)))));
CHECK_EQ(0x80000001, r.Call(1));
}
} // namespace test_run_wasm_simd
} // namespace wasm
} // namespace internal

View File

@ -48,6 +48,7 @@ class TestSignatures {
sig_v_e(0, 1, kExternRefTypes4),
sig_v_c(0, 1, kFuncTypes4),
sig_s_i(1, 1, kSimd128IntTypes4),
sig_s_s(1, 1, kSimd128Types4),
sig_ii_v(2, 0, kIntTypes4),
sig_iii_v(3, 0, kIntTypes4) {
// I used C++ and you won't believe what happened next....
@ -62,6 +63,7 @@ class TestSignatures {
for (int i = 1; i < 4; i++) kIntDoubleTypes4[i] = kWasmF64;
for (int i = 1; i < 4; i++) kIntExternRefTypes4[i] = kWasmExternRef;
for (int i = 1; i < 4; i++) kIntFuncRefTypes4[i] = kWasmFuncRef;
for (int i = 0; i < 4; i++) kSimd128Types4[i] = kWasmS128;
for (int i = 1; i < 4; i++) kIntSimd128Types4[i] = kWasmS128;
for (int i = 0; i < 4; i++) kSimd128IntTypes4[i] = kWasmS128;
kIntLongTypes4[0] = kWasmI32;
@ -109,6 +111,7 @@ class TestSignatures {
FunctionSig* v_e() { return &sig_v_e; }
FunctionSig* v_c() { return &sig_v_c; }
FunctionSig* s_i() { return &sig_s_i; }
FunctionSig* s_s() { return &sig_s_s; }
FunctionSig* ii_v() { return &sig_ii_v; }
FunctionSig* iii_v() { return &sig_iii_v; }
@ -134,6 +137,7 @@ class TestSignatures {
ValueType kIntDoubleTypes4[4];
ValueType kIntExternRefTypes4[4];
ValueType kIntFuncRefTypes4[4];
ValueType kSimd128Types4[4];
ValueType kIntSimd128Types4[4];
ValueType kSimd128IntTypes4[4];
@ -173,6 +177,7 @@ class TestSignatures {
FunctionSig sig_v_e;
FunctionSig sig_v_c;
FunctionSig sig_s_i;
FunctionSig sig_s_s;
FunctionSig sig_ii_v;
FunctionSig sig_iii_v;