MIPS: Improve compare and branch combining.

src/compiler/mips/code-generator-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/compiler/code-generator.h"
 #include "src/compiler/code-generator-impl.h"
 #include "src/compiler/gap-resolver.h"
 #include "src/compiler/node-matchers.h"
 #include "src/compiler/node-properties-inl.h"
 #include "src/mips/macro-assembler-mips.h"
@@ skipping 222 matching lines @@
         __ srav(i.OutputRegister(), i.InputRegister(0), i.InputRegister(1));
       } else {
         int32_t imm = i.InputOperand(1).immediate();
         __ sra(i.OutputRegister(), i.InputRegister(0), imm);
       }
       break;
     case kMipsRor:
       __ Ror(i.OutputRegister(), i.InputRegister(0), i.InputOperand(1));
       break;
     case kMipsTst:
       // Psuedo-instruction used for tst/branch.

[paul.l..., 2014/11/04 19:04:27]

Suggest adding "No opcode emitted here." to comment, as below.

Please either add back the 'break' or put in a comment: // Fall-through.

I don't think either approach will significantly change the generated code, so
it probably best to use explicit break.

[dusmil.imgtec, 2014/11/04 19:46:11]

Done.

-      __ And(kCompareReg, i.InputRegister(0), i.InputOperand(1));
-      break;
     case kMipsCmp:
       // Psuedo-instruction used for cmp/branch. No opcode emitted here.
       break;
     case kMipsMov:
       // TODO(plind): Should we combine mov/li like this, or use separate instr?
       //    - Also see x64 ASSEMBLE_BINOP & RegisterOrOperandType
       if (HasRegisterInput(instr, 0)) {
         __ mov(i.OutputRegister(), i.InputRegister(0));
       } else {
         __ li(i.OutputRegister(), i.InputOperand(0));
@@ skipping 155 matching lines @@
   // MIPS does not have condition code flags, so compare and branch are
   // implemented differently than on the other arch's. The compare operations
   // emit mips psuedo-instructions, which are handled here by branch
   // instructions that do the actual comparison. Essential that the input
   // registers to compare psuedo-op are not modified before this branch op, as
   // they are tested here.
   // TODO(plind): Add CHECK() to ensure that test/cmp and this branch were
   //    not separated by other instructions.
 
   if (instr->arch_opcode() == kMipsTst) {
-    // The kMipsTst psuedo-instruction emits And to 'kCompareReg' register.
     switch (condition) {
       case kNotEqual:
         cc = ne;
         break;
       case kEqual:
         cc = eq;
         break;
       default:
         UNSUPPORTED_COND(kMipsTst, condition);
         break;
     }
-    __ Branch(tlabel, cc, kCompareReg, Operand(zero_reg));
+    __ And(at, i.InputRegister(0), i.InputOperand(1));
+    __ Branch(tlabel, cc, at, Operand(zero_reg));
 
   } else if (instr->arch_opcode() == kMipsAddOvf ||
              instr->arch_opcode() == kMipsSubOvf) {
     // kMipsAddOvf, SubOvf emit negative result to 'kCompareReg' on overflow.
     switch (condition) {
       case kOverflow:
         cc = lt;
         break;
       case kNotOverflow:
         cc = ge;
@@ skipping 106 matching lines @@
   // implemented differently than on the other arch's. The compare operations
   // emit mips psuedo-instructions, which are checked and handled here.
 
   // For materializations, we use delay slot to set the result true, and
   // in the false case, where we fall thru the branch, we reset the result
   // false.
 
   // TODO(plind): Add CHECK() to ensure that test/cmp and this branch were
   //    not separated by other instructions.
   if (instr->arch_opcode() == kMipsTst) {
-    // The kMipsTst psuedo-instruction emits And to 'kCompareReg' register.
     switch (condition) {
       case kNotEqual:
         cc = ne;
         break;
       case kEqual:
         cc = eq;
         break;
       default:
         UNSUPPORTED_COND(kMipsTst, condition);
         break;
     }
-    __ Branch(USE_DELAY_SLOT, &done, cc, kCompareReg, Operand(zero_reg));
+    __ And(at, i.InputRegister(0), i.InputOperand(1));
+    __ Branch(USE_DELAY_SLOT, &done, cc, at, Operand(zero_reg));
     __ li(result, Operand(1));  // In delay slot.
 
   } else if (instr->arch_opcode() == kMipsAddOvf ||
              instr->arch_opcode() == kMipsSubOvf) {
     // kMipsAddOvf, SubOvf emits negative result to 'kCompareReg' on overflow.
     switch (condition) {
       case kOverflow:
         cc = lt;
         break;
       case kNotOverflow:
@@ skipping 379 matching lines @@
     }
   }
   MarkLazyDeoptSite();
 }
 
 #undef __
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8