MIPS: [turbofan] Improve test and equality compare with zero and constants.

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

Index: src/compiler/mips/instruction-selector-mips.cc
diff --git a/src/compiler/mips/instruction-selector-mips.cc b/src/compiler/mips/instruction-selector-mips.cc
index 3c4b378553e69a9e66060fcff69dfaafb2971062..7745717ecbd7316027bf070f51a2c6ab4fe6c512 100644
--- a/src/compiler/mips/instruction-selector-mips.cc
+++ b/src/compiler/mips/instruction-selector-mips.cc
@@ -44,11 +44,10 @@ class MipsOperandGenerator final : public OperandGenerator {
         return is_uint16(value);
       case kMipsLdc1:
       case kMipsSdc1:
-      case kCheckedLoadFloat32:
       case kCheckedLoadFloat64:

[paul.l..., 2015/10/12 03:53:45]

Why do you remove the kCheckedLoadFloat32/kCheckedStoreFloat32cases here? They
have the same requirement for 16-bit immediate.

Also, none of the kCheckedL/SFloatNN cases are in mips64 CanBeImmediate(), and I
think they should be. (the assembler does take care of this now, but that should
not be done there, and will be removed per issue 182)

[dusan.milosavljevic, 2015/10/12 14:41:52]

Yes, the requirement for kCheckedLoadFloat32/kCheckedStoreFloat32 cases still
holds, it is under default case (is_int16()).  No need for loading and storing
floats to be in same case for storing and loading doubles.

-      case kCheckedStoreFloat32:
       case kCheckedStoreFloat64:
-        return is_int16(value + kIntSize);
+        return std::numeric_limits<int16_t>::min() <= (value + kIntSize) &&
+               std::numeric_limits<int16_t>::max() >= (value + kIntSize);

[paul.l..., 2015/10/12 03:53:45]

Can you explain why this is better than is_int16(value + kIntSize)? The old way
conveys the purpose more clearly. If the new condition does fix some edge-case,
you should do it in mips64 version also, no?

[dusan.milosavljevic, 2015/10/12 14:41:52]

Old check does not handle  minimum offset edge case (min_int16 - 4), where
negative offset 16-bit range can be exceeded.  Mips64 code does not require
this, because for loading and storing doubles ldc1 and sdc1 instructions are
used instead of lwc1/swc1 pairs.

       default:
         return is_int16(value);
     }
@@ -826,6 +825,16 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
   // Match immediates on left or right side of comparison.
   if (g.CanBeImmediate(right, opcode)) {
     switch (cont->condition()) {
+      case kEqual:
+      case kNotEqual:
+        if (cont->IsSet()) {
+          VisitCompare(selector, opcode, g.UseRegister(left),
+                       g.UseImmediate(right), cont);
+        } else {
+          VisitCompare(selector, opcode, g.UseRegister(left),
+                       g.UseRegister(right), cont);
+        }
+        break;
       case kSignedLessThan:
       case kSignedGreaterThanOrEqual:
       case kUnsignedLessThan:
@@ -840,6 +849,16 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
   } else if (g.CanBeImmediate(left, opcode)) {
     if (!commutative) cont->Commute();
     switch (cont->condition()) {
+      case kEqual:
+      case kNotEqual:
+        if (cont->IsSet()) {
+          VisitCompare(selector, opcode, g.UseRegister(right),
+                       g.UseImmediate(left), cont);
+        } else {
+          VisitCompare(selector, opcode, g.UseRegister(right),
+                       g.UseRegister(left), cont);
+        }
+        break;
       case kSignedLessThan:
       case kSignedGreaterThanOrEqual:
       case kUnsignedLessThan: