MIPS64: Fix Word32Compare turbofan operator implementation when comparing...

MIPS64: Fix Word32Compare turbofan operator implementation when comparing signed with unsigned operand

MIPS64 doesn't support Word32 compare instructions. Instead it relies
that the values in registers are correctly sign-extended and uses
Word64 comparison instead. This behavior is correct in most cases,
but doesn't work when comparing signed with unsigned operands.
The solution proposed here tries to match a comparison of signed
with unsigned operand, and perform Word32Compare simulation only
in those cases. Unfortunately, the solution is not complete because
it might skip cases where Word32 compare simulation is needed, so
basically it is a hack.

BUG=
TEST=mjsunit/compiler/uint32

Review-Url: https://codereview.chromium.org/2391393003
Cr-Commit-Position: refs/heads/master@{#40398}

src/compiler/mips64/code-generator-mips64.cc

@@ -1866,6 +1866,10 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
     case kAtomicStoreWord32:
       ASSEMBLE_ATOMIC_STORE_INTEGER(sw);
       break;
+    case kMips64AssertEqual:
+      __ Assert(eq, static_cast<BailoutReason>(i.InputOperand(2).immediate()),
+                i.InputRegister(0), Operand(i.InputRegister(1)));
+      break;
   }
   return kSuccess;
 }  // NOLINT(readability/fn_size)

src/compiler/mips64/instruction-codes-mips64.h

@@ -169,7 +169,8 @@ namespace compiler {
   V(Mips64ByteSwap32)               \
   V(Mips64StackClaim)               \
   V(Mips64Seb)                      \
-  V(Mips64Seh)
+  V(Mips64Seh)                      \
+  V(Mips64AssertEqual)
 
 // Addressing modes represent the "shape" of inputs to an instruction.
 // Many instructions support multiple addressing modes. Addressing modes

src/compiler/mips64/instruction-selector-mips64.cc

@@ -1840,10 +1840,89 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
   }
 }
 
+bool IsNodeUnsigned(Node* n) {
+  NodeMatcher m(n);
+
+  if (m.IsLoad()) {
+    LoadRepresentation load_rep = LoadRepresentationOf(n->op());
+    return load_rep.IsUnsigned();
+  } else if (m.IsUnalignedLoad()) {
+    UnalignedLoadRepresentation load_rep =
+        UnalignedLoadRepresentationOf(n->op());
+    return load_rep.IsUnsigned();
+  } else {
+    return m.IsUint32Div() || m.IsUint32LessThan() ||
+           m.IsUint32LessThanOrEqual() || m.IsUint32Mod() ||
+           m.IsUint32MulHigh() || m.IsChangeFloat64ToUint32() ||
+           m.IsTruncateFloat64ToUint32() || m.IsTruncateFloat32ToUint32();
+  }
+}
+
+// Shared routine for multiple word compare operations.
+void VisitFullWord32Compare(InstructionSelector* selector, Node* node,
+                            InstructionCode opcode, FlagsContinuation* cont) {
+  Mips64OperandGenerator g(selector);
+  InstructionOperand leftOp = g.TempRegister();
+  InstructionOperand rightOp = g.TempRegister();
+
+  selector->Emit(kMips64Dshl, leftOp, g.UseRegister(node->InputAt(0)),
+                 g.TempImmediate(32));
+  selector->Emit(kMips64Dshl, rightOp, g.UseRegister(node->InputAt(1)),
+                 g.TempImmediate(32));
+
+  VisitCompare(selector, opcode, leftOp, rightOp, cont);
+}
+
+void VisitOptimizedWord32Compare(InstructionSelector* selector, Node* node,
+                                 InstructionCode opcode,
+                                 FlagsContinuation* cont) {
+  if (FLAG_debug_code) {
+    Mips64OperandGenerator g(selector);
+    InstructionOperand leftOp = g.TempRegister();
+    InstructionOperand rightOp = g.TempRegister();
+    InstructionOperand optimizedResult = g.TempRegister();
+    InstructionOperand fullResult = g.TempRegister();
+    FlagsCondition condition = cont->condition();
+    InstructionCode testOpcode = opcode |
+                                 FlagsConditionField::encode(condition) |
+                                 FlagsModeField::encode(kFlags_set);
+
+    selector->Emit(testOpcode, optimizedResult, g.UseRegister(node->InputAt(0)),
+                   g.UseRegister(node->InputAt(1)));
+
+    selector->Emit(kMips64Dshl, leftOp, g.UseRegister(node->InputAt(0)),
+                   g.TempImmediate(32));
+    selector->Emit(kMips64Dshl, rightOp, g.UseRegister(node->InputAt(1)),
+                   g.TempImmediate(32));
+    selector->Emit(testOpcode, fullResult, leftOp, rightOp);
+
+    selector->Emit(
+        kMips64AssertEqual, g.NoOutput(), optimizedResult, fullResult,
+        g.TempImmediate(BailoutReason::kUnsupportedNonPrimitiveCompare));
+  }
+
+  VisitWordCompare(selector, node, opcode, cont, false);
+}
 
 void VisitWord32Compare(InstructionSelector* selector, Node* node,
                         FlagsContinuation* cont) {
-  VisitWordCompare(selector, node, kMips64Cmp, cont, false);
+  // MIPS64 doesn't support Word32 compare instructions. Instead it relies
+  // that the values in registers are correctly sign-extended and uses
+  // Word64 comparison instead. This behavior is correct in most cases,
+  // but doesn't work when comparing signed with unsigned operands.
+  // We could simulate full Word32 compare in all cases but this would
+  // create an unnecessary overhead since unsigned integers are rarely
+  // used in JavaScript.
+  // The solution proposed here tries to match a comparison of signed
+  // with unsigned operand, and perform full Word32Compare only
+  // in those cases. Unfortunately, the solution is not complete because
+  // it might skip cases where Word32 full compare is needed, so
+  // basically it is a hack.
+  if (IsNodeUnsigned(node->InputAt(0)) != IsNodeUnsigned(node->InputAt(1))) {
+    VisitFullWord32Compare(selector, node, kMips64Cmp, cont);
+  } else {
+    VisitOptimizedWord32Compare(selector, node, kMips64Cmp, cont);
+  }
 }
 
 

test/unittests/compiler/mips64/instruction-selector-mips64-unittest.cc

@@ -320,10 +320,40 @@ TEST_P(InstructionSelectorCmpTest, Parameter) {
   StreamBuilder m(this, type, type, type);
   m.Return((m.*cmp.mi.constructor)(m.Parameter(0), m.Parameter(1)));
   Stream s = m.Build();
+
+  if (FLAG_debug_code &&
+      type.representation() == MachineRepresentation::kWord32) {
+    ASSERT_EQ(6, s.size());
+
+    EXPECT_EQ(cmp.mi.arch_opcode, s[0]->arch_opcode());
+    EXPECT_EQ(2U, s[0]->InputCount());
+    EXPECT_EQ(1U, s[0]->OutputCount());
+
+    EXPECT_EQ(kMips64Dshl, s[1]->arch_opcode());
+    EXPECT_EQ(2U, s[1]->InputCount());
+    EXPECT_EQ(1U, s[1]->OutputCount());
+
+    EXPECT_EQ(kMips64Dshl, s[2]->arch_opcode());
+    EXPECT_EQ(2U, s[2]->InputCount());
+    EXPECT_EQ(1U, s[2]->OutputCount());
+
+    EXPECT_EQ(cmp.mi.arch_opcode, s[3]->arch_opcode());
+    EXPECT_EQ(2U, s[3]->InputCount());
+    EXPECT_EQ(1U, s[3]->OutputCount());
+
+    EXPECT_EQ(kMips64AssertEqual, s[4]->arch_opcode());
+    EXPECT_EQ(3U, s[4]->InputCount());
+    EXPECT_EQ(0U, s[4]->OutputCount());
+
+    EXPECT_EQ(cmp.mi.arch_opcode, s[5]->arch_opcode());
+    EXPECT_EQ(2U, s[5]->InputCount());
+    EXPECT_EQ(1U, s[5]->OutputCount());
+  } else {
     ASSERT_EQ(cmp.expected_size, s.size());
     EXPECT_EQ(cmp.mi.arch_opcode, s[0]->arch_opcode());
     EXPECT_EQ(2U, s[0]->InputCount());
     EXPECT_EQ(1U, s[0]->OutputCount());
+  }
 }
 
 INSTANTIATE_TEST_CASE_P(InstructionSelectorTest, InstructionSelectorCmpTest,