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

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

 // Copyright 2014 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/base/adapters.h"
 #include "src/base/bits.h"
 #include "src/compiler/instruction-selector-impl.h"
 #include "src/compiler/node-matchers.h"
 #include "src/compiler/node-properties.h"
 
@@ skipping 26 matching lines @@
     int32_t value = m.Value();
     switch (ArchOpcodeField::decode(opcode)) {
       case kMipsShl:
       case kMipsSar:
       case kMipsShr:
         return is_uint5(value);
       case kMipsXor:
         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);
     }
   }
 
  private:
   bool ImmediateFitsAddrMode1Instruction(int32_t imm) const {
     TRACE_UNIMPL();
     return false;
   }
@@ skipping 757 matching lines @@
 void VisitWordCompare(InstructionSelector* selector, Node* node,
                       InstructionCode opcode, FlagsContinuation* cont,
                       bool commutative) {
   MipsOperandGenerator g(selector);
   Node* left = node->InputAt(0);
   Node* right = node->InputAt(1);
 
   // 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:
       case kUnsignedGreaterThanOrEqual:
         VisitCompare(selector, opcode, g.UseRegister(left),
                      g.UseImmediate(right), cont);
         break;
       default:
         VisitCompare(selector, opcode, g.UseRegister(left),
                      g.UseRegister(right), cont);
     }
   } 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:
       case kUnsignedGreaterThanOrEqual:
         VisitCompare(selector, opcode, g.UseRegister(right),
                      g.UseImmediate(left), cont);
         break;
       default:
         VisitCompare(selector, opcode, g.UseRegister(right),
                      g.UseRegister(left), cont);
@@ skipping 273 matching lines @@
       IsFp64Mode()) {
     flags |= MachineOperatorBuilder::kFloat64RoundDown |
              MachineOperatorBuilder::kFloat64RoundTruncate;
   }
   return flags;
 }
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8