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

[wasm-simd][ia32][x64] Only use registers for shuffles

Shuffles have pattern matching clauses which, depending on the
instruction used, can require src0 or src1 to be register or not.
However we do not have 16-byte alignment for SIMD operands yet, so it
will segfault when we use an SSE SIMD instruction with unaligned
operands.

This patch fixes all the shuffle cases to always use a register for the
input nodes, and it does so by ignoring the values of src0_needs_reg and
src1_needs_reg. When we eventually have memory alignment, we can
re-enable this check, without mucking around too much in the logic in
each shuffle match clause.

Bug: v8:9198
Change-Id: I264e136f017353019f19954c62c88206f7b90656
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2504849Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Reviewed-by: 's avatarAdam Klein <adamk@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70848}
parent 145c6f7b
......@@ -2720,6 +2720,9 @@ void InstructionSelector::VisitI8x16Shuffle(Node* node) {
// AVX and swizzles don't generally need DefineSameAsFirst to avoid a move.
bool no_same_as_first = use_avx || is_swizzle;
// We generally need UseRegister for input0, Use for input1.
// TODO(v8:9198): We don't have 16-byte alignment for SIMD operands yet, but
// we retain this logic (continue setting these in the various shuffle match
// clauses), but ignore it when selecting registers or slots.
bool src0_needs_reg = true;
bool src1_needs_reg = false;
ArchOpcode opcode = kIA32I8x16Shuffle; // general shuffle is the default
......@@ -2826,16 +2829,18 @@ void InstructionSelector::VisitI8x16Shuffle(Node* node) {
Node* input0 = node->InputAt(0);
InstructionOperand dst =
no_same_as_first ? g.DefineAsRegister(node) : g.DefineSameAsFirst(node);
InstructionOperand src0 =
src0_needs_reg ? g.UseRegister(input0) : g.Use(input0);
// TODO(v8:9198): Use src0_needs_reg when we have memory alignment for SIMD.
InstructionOperand src0 = g.UseRegister(input0);
USE(src0_needs_reg);
int input_count = 0;
InstructionOperand inputs[2 + kMaxImms + kMaxTemps];
inputs[input_count++] = src0;
if (!is_swizzle) {
Node* input1 = node->InputAt(1);
inputs[input_count++] =
src1_needs_reg ? g.UseRegister(input1) : g.Use(input1);
// TODO(v8:9198): Use src1_needs_reg when we have memory alignment for SIMD.
inputs[input_count++] = g.UseRegister(input1);
USE(src1_needs_reg);
}
for (int i = 0; i < imm_count; ++i) {
inputs[input_count++] = g.UseImmediate(imms[i]);
......
......@@ -3437,6 +3437,9 @@ void InstructionSelector::VisitI8x16Shuffle(Node* node) {
// Swizzles don't generally need DefineSameAsFirst to avoid a move.
bool no_same_as_first = is_swizzle;
// We generally need UseRegister for input0, Use for input1.
// TODO(v8:9198): We don't have 16-byte alignment for SIMD operands yet, but
// we retain this logic (continue setting these in the various shuffle match
// clauses), but ignore it when selecting registers or slots.
bool src0_needs_reg = true;
bool src1_needs_reg = false;
ArchOpcode opcode = kX64I8x16Shuffle; // general shuffle is the default
......@@ -3545,16 +3548,18 @@ void InstructionSelector::VisitI8x16Shuffle(Node* node) {
Node* input0 = node->InputAt(0);
InstructionOperand dst =
no_same_as_first ? g.DefineAsRegister(node) : g.DefineSameAsFirst(node);
InstructionOperand src0 =
src0_needs_reg ? g.UseUniqueRegister(input0) : g.UseUnique(input0);
// TODO(v8:9198): Use src0_needs_reg when we have memory alignment for SIMD.
InstructionOperand src0 = g.UseUniqueRegister(input0);
USE(src0_needs_reg);
int input_count = 0;
InstructionOperand inputs[2 + kMaxImms + kMaxTemps];
inputs[input_count++] = src0;
if (!is_swizzle) {
Node* input1 = node->InputAt(1);
inputs[input_count++] =
src1_needs_reg ? g.UseUniqueRegister(input1) : g.UseUnique(input1);
// TODO(v8:9198): Use src1_needs_reg when we have memory alignment for SIMD.
inputs[input_count++] = g.UseUniqueRegister(input1);
USE(src1_needs_reg);
}
for (int i = 0; i < imm_count; ++i) {
inputs[input_count++] = g.UseImmediate(imms[i]);
......
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