Commit fb51aa40 authored by Thibaud Michaud's avatar Thibaud Michaud Committed by Commit Bot

[wasm][x64] Fix unordered floating-point select

Unordered floating-point (non-)equality is implemented using two flags
on x64: kUnorderedNotEqual as "not_equal OR parity_even" and
kUnorderedEqual as "equal AND parity_odd". Only the first flag was
checked.

This change fixes the kUnorderedNotEqual case by emitting a second
cmov to also move the "true" value if the parity_even flag is set. The
kUnorderedEqual case is covered by inverting the condition in the
instruction selector.

This should also be optimal according to the code emitted by clang -O3
for equivalent C++ code.

Drive-by: remove unused overload of EmitWithContinuation.

R=neis@chromium.org
CC=ahaas@chromium.org

Bug: chromium:1200184
Change-Id: Iae438d29fb5897ca910a154f140a5a6a904490ec
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2844651
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74122}
parent 0f683da3
......@@ -802,11 +802,6 @@ size_t InstructionSelector::AddInputsToFrameStateDescriptor(
return entries;
}
Instruction* InstructionSelector::EmitWithContinuation(
InstructionCode opcode, FlagsContinuation* cont) {
return EmitWithContinuation(opcode, 0, nullptr, 0, nullptr, cont);
}
Instruction* InstructionSelector::EmitWithContinuation(
InstructionCode opcode, InstructionOperand a, FlagsContinuation* cont) {
return EmitWithContinuation(opcode, 0, nullptr, 1, &a, cont);
......
......@@ -4407,18 +4407,35 @@ void CodeGenerator::AssembleArchSelect(Instruction* instr,
Condition cc = FlagsConditionToCondition(condition);
DCHECK_EQ(i.OutputRegister(), i.InputRegister(instr->InputCount() - 2));
size_t last_input = instr->InputCount() - 1;
// kUnorderedNotEqual can be implemented more efficiently than
// kUnorderedEqual. As the OR of two flags, it can be done with just two
// cmovs. If the condition was originally a kUnorderedEqual, expect the
// instruction selector to have inverted it and swapped the input.
DCHECK_NE(condition, kUnorderedEqual);
if (rep == MachineRepresentation::kWord32) {
if (HasRegisterInput(instr, last_input)) {
__ cmovl(cc, i.OutputRegister(), i.InputRegister(last_input));
if (condition == kUnorderedNotEqual) {
__ cmovl(parity_even, i.OutputRegister(), i.InputRegister(last_input));
}
} else {
__ cmovl(cc, i.OutputRegister(), i.InputOperand(last_input));
if (condition == kUnorderedNotEqual) {
__ cmovl(parity_even, i.OutputRegister(), i.InputOperand(last_input));
}
}
} else {
DCHECK_EQ(rep, MachineRepresentation::kWord64);
if (HasRegisterInput(instr, last_input)) {
__ cmovq(cc, i.OutputRegister(), i.InputRegister(last_input));
if (condition == kUnorderedNotEqual) {
__ cmovq(parity_even, i.OutputRegister(), i.InputRegister(last_input));
}
} else {
__ cmovq(cc, i.OutputRegister(), i.InputOperand(last_input));
if (condition == kUnorderedNotEqual) {
__ cmovq(parity_even, i.OutputRegister(), i.InputOperand(last_input));
}
}
}
}
......
......@@ -1890,8 +1890,14 @@ void VisitCompareWithMemoryOperand(InstructionSelector* selector,
opcode |= AddressingModeField::encode(addressing_mode);
inputs[input_count++] = right;
if (cont->IsSelect()) {
inputs[input_count++] = g.UseRegister(cont->false_value());
inputs[input_count++] = g.Use(cont->true_value());
if (opcode == kUnorderedEqual) {
cont->Negate();
inputs[input_count++] = g.UseRegister(cont->true_value());
inputs[input_count++] = g.Use(cont->false_value());
} else {
inputs[input_count++] = g.UseRegister(cont->false_value());
inputs[input_count++] = g.Use(cont->true_value());
}
}
selector->EmitWithContinuation(opcode, 0, nullptr, input_count, inputs, cont);
......@@ -1903,9 +1909,15 @@ void VisitCompare(InstructionSelector* selector, InstructionCode opcode,
FlagsContinuation* cont) {
if (cont->IsSelect()) {
X64OperandGenerator g(selector);
InstructionOperand inputs[] = {left, right,
g.UseRegister(cont->false_value()),
g.Use(cont->true_value())};
InstructionOperand inputs[4] = {left, right};
if (cont->condition() == kUnorderedEqual) {
cont->Negate();
inputs[2] = g.UseRegister(cont->true_value());
inputs[3] = g.Use(cont->false_value());
} else {
inputs[2] = g.UseRegister(cont->false_value());
inputs[3] = g.Use(cont->true_value());
}
selector->EmitWithContinuation(opcode, 0, nullptr, 4, inputs, cont);
return;
}
......
......@@ -436,6 +436,40 @@ TEST(RunWord64Select) {
}
}
TEST(RunSelectUnorderedEqual) {
BufferedRawMachineAssemblerTester<int64_t> m(
MachineType::Int64(), MachineType::Int64(), MachineType::Float32());
if (!m.machine()->Word64Select().IsSupported()) {
return;
}
Node* cmp = m.Float32Equal(m.Parameter(2), m.Float32Constant(0));
m.Return(m.Word64Select(cmp, m.Parameter(0), m.Parameter(1)));
constexpr int64_t input1 = 16;
constexpr int64_t input2 = 0x123456789abc;
CHECK_EQ(input1, m.Call(input1, input2, float{0}));
CHECK_EQ(input2, m.Call(input1, input2, float{1}));
CHECK_EQ(input2, m.Call(input1, input2, std::nanf("")));
}
TEST(RunSelectUnorderedNotEqual) {
BufferedRawMachineAssemblerTester<int64_t> m(
MachineType::Int64(), MachineType::Int64(), MachineType::Float32());
if (!m.machine()->Word64Select().IsSupported()) {
return;
}
Node* cmp = m.Float32NotEqual(m.Parameter(2), m.Float32Constant(0));
m.Return(m.Word64Select(cmp, m.Parameter(0), m.Parameter(1)));
constexpr int64_t input1 = 16;
constexpr int64_t input2 = 0x123456789abc;
CHECK_EQ(input2, m.Call(input1, input2, float{0}));
CHECK_EQ(input1, m.Call(input1, input2, float{1}));
CHECK_EQ(input1, m.Call(input1, input2, std::nanf("")));
}
namespace {
void FooForSelect() {}
} // namespace
......
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