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

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

Index: src/compiler/mips64/instruction-selector-mips64.cc
diff --git a/src/compiler/mips64/instruction-selector-mips64.cc b/src/compiler/mips64/instruction-selector-mips64.cc
index 6e937e20d7ae9fdc4c48245b4bf3f8e760504a8a..1fd00238b6ab8297cbff494bd6f7cd364d264d70 100644
--- a/src/compiler/mips64/instruction-selector-mips64.cc
+++ b/src/compiler/mips64/instruction-selector-mips64.cc
@@ -1840,10 +1840,70 @@ 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();
+  }
+}
+
+bool ToSimulateWord32Compare(Node* node) {

[ahaas, 2016/10/11 09:24:58]

I think you could even just inline this function and say
"if(IsNodeUnsigned(node->InputAt(0)) != IsNodeUnsigned(node->InputAt(1)))"

+  Node* left = node->InputAt(0);
+  Node* right = node->InputAt(1);
+
+  bool leftUnsigned = IsNodeUnsigned(left);
+  bool rightUnsigned = IsNodeUnsigned(right);
+
+  return (leftUnsigned && !rightUnsigned) || (!leftUnsigned && rightUnsigned);

[miran.karic, 2016/10/06 10:24:21]

You can just have
return IsNodeUnsigned(left) != IsNodeUnsigned(right);

+}
+
+// Shared routine for multiple word compare operations.
+void VisitSimulateWord32Compare(InstructionSelector* selector, Node* node,
+                                InstructionCode opcode, FlagsContinuation* cont,
+                                bool commutative) {
+  Mips64OperandGenerator g(selector);
+  Node* left = node->InputAt(0);
+  Node* right = node->InputAt(1);
+  InstructionOperand leftOp = g.TempRegister();
+  InstructionOperand rightOp = g.TempRegister();
+
+  selector->Emit(kMips64Dshl, leftOp, g.UseRegister(left), g.TempImmediate(32));
+  selector->Emit(kMips64Dshl, rightOp, g.UseRegister(right),
+                 g.TempImmediate(32));
+
+  VisitCompare(selector, opcode, leftOp, rightOp, cont);
+}
 
 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 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 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.
+  if (ToSimulateWord32Compare(node)) {
+    VisitSimulateWord32Compare(selector, node, kMips64Cmp, cont, false);
+  } else {
+    VisitWordCompare(selector, node, kMips64Cmp, cont, false);
+  }
 }