MIPS64: Unify and improve Word32 compares to use same instructions as Word64 compares.
authordusan.milosavljevic <dusan.milosavljevic@imgtec.com>
Mon, 16 Mar 2015 11:00:00 +0000 (04:00 -0700)
committerCommit bot <commit-bot@chromium.org>
Mon, 16 Mar 2015 11:00:12 +0000 (11:00 +0000)
The CL enables the same instructions are selected for Word32 and Word64 compare
operations which is possible due to a fact 32-bit inputs and produced values
are always sign-extended.

TEST=
BUG=

Review URL: https://codereview.chromium.org/1005123002

Cr-Commit-Position: refs/heads/master@{#27212}

src/compiler/mips64/code-generator-mips64.cc
src/compiler/mips64/instruction-codes-mips64.h
src/compiler/mips64/instruction-selector-mips64.cc
src/mips64/simulator-mips64.cc
test/cctest/compiler/call-tester.h
test/unittests/compiler/mips64/instruction-selector-mips64-unittest.cc

index a9f9961..85b2ffb 100644 (file)
@@ -586,11 +586,9 @@ void CodeGenerator::AssembleArchInstruction(Instruction* instr) {
       __ Dror(i.OutputRegister(), i.InputRegister(0), i.InputOperand(1));
       break;
     case kMips64Tst:
-    case kMips64Tst32:
       // Pseudo-instruction used for cmp/branch. No opcode emitted here.
       break;
     case kMips64Cmp:
-    case kMips64Cmp32:
       // Pseudo-instruction used for cmp/branch. No opcode emitted here.
       break;
     case kMips64Mov:
@@ -826,25 +824,13 @@ void CodeGenerator::AssembleArchBranch(Instruction* instr, BranchInfo* branch) {
   // 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
+  // registers to compare pseudo-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() == kMips64Tst) {
     cc = FlagsConditionToConditionTst(branch->condition);
     __ And(at, i.InputRegister(0), i.InputOperand(1));
     __ Branch(tlabel, cc, at, Operand(zero_reg));
-  } else if (instr->arch_opcode() == kMips64Tst32) {
-    cc = FlagsConditionToConditionTst(branch->condition);
-    // Zero-extend registers on MIPS64 only 64-bit operand
-    // branch and compare op. is available.
-    // This is a disadvantage to perform 32-bit operation on MIPS64.
-    // Try to force globally in front-end Word64 representation to be preferred
-    // for MIPS64 even for Word32.
-    __ And(at, i.InputRegister(0), i.InputOperand(1));
-    __ Dext(at, at, 0, 32);
-    __ Branch(tlabel, cc, at, Operand(zero_reg));
   } else if (instr->arch_opcode() == kMips64Dadd ||
              instr->arch_opcode() == kMips64Dsub) {
     cc = FlagsConditionToConditionOvf(branch->condition);
@@ -857,42 +843,6 @@ void CodeGenerator::AssembleArchBranch(Instruction* instr, BranchInfo* branch) {
     __ Branch(tlabel, cc, i.InputRegister(0), i.InputOperand(1));
 
     if (!branch->fallthru) __ Branch(flabel);  // no fallthru to flabel.
-
-  } else if (instr->arch_opcode() == kMips64Cmp32) {
-    cc = FlagsConditionToConditionCmp(branch->condition);
-
-    switch (branch->condition) {
-      case kEqual:
-      case kNotEqual:
-      case kSignedLessThan:
-      case kSignedGreaterThanOrEqual:
-      case kSignedLessThanOrEqual:
-      case kSignedGreaterThan:
-        // Sign-extend registers on MIPS64 only 64-bit operand
-        // branch and compare op. is available.
-        __ sll(i.InputRegister(0), i.InputRegister(0), 0);
-        if (instr->InputAt(1)->IsRegister()) {
-          __ sll(i.InputRegister(1), i.InputRegister(1), 0);
-        }
-        break;
-      case kUnsignedLessThan:
-      case kUnsignedGreaterThanOrEqual:
-      case kUnsignedLessThanOrEqual:
-      case kUnsignedGreaterThan:
-        // Zero-extend registers on MIPS64 only 64-bit operand
-        // branch and compare op. is available.
-        __ Dext(i.InputRegister(0), i.InputRegister(0), 0, 32);
-        if (instr->InputAt(1)->IsRegister()) {
-          __ Dext(i.InputRegister(1), i.InputRegister(1), 0, 32);
-        }
-        break;
-      default:
-        UNSUPPORTED_COND(kMips64Cmp, branch->condition);
-        break;
-    }
-    __ Branch(tlabel, cc, i.InputRegister(0), i.InputOperand(1));
-
-    if (!branch->fallthru) __ Branch(flabel);  // no fallthru to flabel.
   } else if (instr->arch_opcode() == kMips64CmpD) {
     // TODO(dusmil) optimize unordered checks to use less instructions
     // even if we have to unfold BranchF macro.
@@ -966,14 +916,6 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr,
     __ 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() == kMips64Tst32) {
-    cc = FlagsConditionToConditionTst(condition);
-    // Zero-extend register on MIPS64 only 64-bit operand
-    // branch and compare op. is available.
-    __ And(at, i.InputRegister(0), i.InputOperand(1));
-    __ Dext(at, at, 0, 32);
-    __ Branch(USE_DELAY_SLOT, &done, cc, at, Operand(zero_reg));
-    __ li(result, Operand(1));  // In delay slot.
   } else if (instr->arch_opcode() == kMips64Dadd ||
              instr->arch_opcode() == kMips64Dsub) {
     cc = FlagsConditionToConditionOvf(condition);
@@ -987,42 +929,6 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr,
     cc = FlagsConditionToConditionCmp(condition);
     __ Branch(USE_DELAY_SLOT, &done, cc, left, right);
     __ li(result, Operand(1));  // In delay slot.
-  } else if (instr->arch_opcode() == kMips64Cmp32) {
-    Register left = i.InputRegister(0);
-    Operand right = i.InputOperand(1);
-    cc = FlagsConditionToConditionCmp(condition);
-
-    switch (condition) {
-      case kEqual:
-      case kNotEqual:
-      case kSignedLessThan:
-      case kSignedGreaterThanOrEqual:
-      case kSignedLessThanOrEqual:
-      case kSignedGreaterThan:
-        // Sign-extend registers on MIPS64 only 64-bit operand
-        // branch and compare op. is available.
-        __ sll(left, left, 0);
-        if (instr->InputAt(1)->IsRegister()) {
-          __ sll(i.InputRegister(1), i.InputRegister(1), 0);
-        }
-        break;
-      case kUnsignedLessThan:
-      case kUnsignedGreaterThanOrEqual:
-      case kUnsignedLessThanOrEqual:
-      case kUnsignedGreaterThan:
-        // Zero-extend registers on MIPS64 only 64-bit operand
-        // branch and compare op. is available.
-        __ Dext(left, left, 0, 32);
-        if (instr->InputAt(1)->IsRegister()) {
-          __ Dext(i.InputRegister(1), i.InputRegister(1), 0, 32);
-        }
-        break;
-      default:
-        UNSUPPORTED_COND(kMips64Cmp32, condition);
-        break;
-    }
-    __ Branch(USE_DELAY_SLOT, &done, cc, left, right);
-    __ li(result, Operand(1));  // In delay slot.
   } else if (instr->arch_opcode() == kMips64CmpD) {
     FPURegister left = i.InputDoubleRegister(0);
     FPURegister right = i.InputDoubleRegister(1);
index d1693cd..2e57820 100644 (file)
@@ -43,9 +43,7 @@ namespace compiler {
   V(Mips64Dror)                     \
   V(Mips64Mov)                      \
   V(Mips64Tst)                      \
-  V(Mips64Tst32)                    \
   V(Mips64Cmp)                      \
-  V(Mips64Cmp32)                    \
   V(Mips64CmpD)                     \
   V(Mips64AddD)                     \
   V(Mips64SubD)                     \
index 248b334..707d3d2 100644 (file)
@@ -57,35 +57,6 @@ class Mips64OperandGenerator FINAL : public OperandGenerator {
     }
   }
 
-
-  bool CanBeImmediate(Node* node, InstructionCode opcode,
-                      FlagsContinuation* cont) {
-    int64_t value;
-    if (node->opcode() == IrOpcode::kInt32Constant)
-      value = OpParameter<int32_t>(node);
-    else if (node->opcode() == IrOpcode::kInt64Constant)
-      value = OpParameter<int64_t>(node);
-    else
-      return false;
-    switch (ArchOpcodeField::decode(opcode)) {
-      case kMips64Cmp32:
-        switch (cont->condition()) {
-          case kUnsignedLessThan:
-          case kUnsignedGreaterThanOrEqual:
-          case kUnsignedLessThanOrEqual:
-          case kUnsignedGreaterThan:
-            // Immediate operands for unsigned 32-bit compare operations
-            // should not be sign-extended.
-            return is_uint15(value);
-          default:
-            return false;
-        }
-      default:
-        return is_int16(value);
-    }
-  }
-
-
  private:
   bool ImmediateFitsAddrMode1Instruction(int32_t imm) const {
     TRACE_UNIMPL();
@@ -830,10 +801,10 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
   Node* right = node->InputAt(1);
 
   // Match immediates on left or right side of comparison.
-  if (g.CanBeImmediate(right, opcode, cont)) {
+  if (g.CanBeImmediate(right, opcode)) {
     VisitCompare(selector, opcode, g.UseRegister(left), g.UseImmediate(right),
                  cont);
-  } else if (g.CanBeImmediate(left, opcode, cont)) {
+  } else if (g.CanBeImmediate(left, opcode)) {
     if (!commutative) cont->Commute();
     VisitCompare(selector, opcode, g.UseRegister(right), g.UseImmediate(left),
                  cont);
@@ -846,7 +817,7 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
 
 void VisitWord32Compare(InstructionSelector* selector, Node* node,
                         FlagsContinuation* cont) {
-  VisitWordCompare(selector, node, kMips64Cmp32, cont, false);
+  VisitWordCompare(selector, node, kMips64Cmp, cont, false);
 }
 
 
@@ -858,10 +829,10 @@ void VisitWord64Compare(InstructionSelector* selector, Node* node,
 }  // namespace
 
 
-void EmitWordCompareZero(InstructionSelector* selector, InstructionCode opcode,
-                         Node* value, FlagsContinuation* cont) {
+void EmitWordCompareZero(InstructionSelector* selector, Node* value,
+                         FlagsContinuation* cont) {
   Mips64OperandGenerator g(selector);
-  opcode = cont->Encode(opcode);
+  InstructionCode opcode = cont->Encode(kMips64Cmp);
   InstructionOperand const value_operand = g.UseRegister(value);
   if (cont->IsBranch()) {
     selector->Emit(opcode, g.NoOutput(), value_operand, g.TempImmediate(0),
@@ -877,13 +848,7 @@ void EmitWordCompareZero(InstructionSelector* selector, InstructionCode opcode,
 // Shared routine for word comparisons against zero.
 void VisitWordCompareZero(InstructionSelector* selector, Node* user,
                           Node* value, FlagsContinuation* cont) {
-  // Initially set comparison against 0 to be 64-bit variant for branches that
-  // cannot combine.
-  InstructionCode opcode = kMips64Cmp;
   while (selector->CanCover(user, value)) {
-    if (user->opcode() == IrOpcode::kWord32Equal) {
-      opcode = kMips64Cmp32;
-    }
     switch (value->opcode()) {
       case IrOpcode::kWord32Equal: {
         // Combine with comparisons against 0 by simply inverting the
@@ -893,7 +858,6 @@ void VisitWordCompareZero(InstructionSelector* selector, Node* user,
           user = value;
           value = m.left().node();
           cont->Negate();
-          opcode = kMips64Cmp32;
           continue;
         }
         cont->OverwriteAndNegateIfEqual(kEqual);
@@ -968,7 +932,6 @@ void VisitWordCompareZero(InstructionSelector* selector, Node* user,
         }
         break;
       case IrOpcode::kWord32And:
-        return VisitWordCompare(selector, value, kMips64Tst32, cont, true);
       case IrOpcode::kWord64And:
         return VisitWordCompare(selector, value, kMips64Tst, cont, true);
       default:
@@ -978,7 +941,7 @@ void VisitWordCompareZero(InstructionSelector* selector, Node* user,
   }
 
   // Continuation could not be combined with a compare, emit compare against 0.
-  EmitWordCompareZero(selector, opcode, value, cont);
+  EmitWordCompareZero(selector, value, cont);
 }
 
 
index 5ddad8d..cc5eaea 100644 (file)
@@ -1995,7 +1995,7 @@ void Simulator::ConfigureTypeRegister(Instruction* instr,
           *return_addr_reg = instr->RdValue();
           break;
         case SLL:
-          *alu_out = (int32_t)rt << sa;
+          *alu_out = static_cast<int32_t>(rt) << sa;
           break;
         case DSLL:
           *alu_out = rt << sa;
@@ -2007,12 +2007,14 @@ void Simulator::ConfigureTypeRegister(Instruction* instr,
           if (rs_reg == 0) {
             // Regular logical right shift of a word by a fixed number of
             // bits instruction. RS field is always equal to 0.
-            *alu_out = (uint32_t)rt_u >> sa;
+            // Sign-extend the 32-bit result.
+            *alu_out = static_cast<int32_t>(static_cast<uint32_t>(rt_u) >> sa);
           } else {
             // Logical right-rotate of a word by a fixed number of bits. This
             // is special case of SRL instruction, added in MIPS32 Release 2.
             // RS field is equal to 00001.
-            *alu_out = base::bits::RotateRight32((uint32_t)rt_u, sa);
+            *alu_out = static_cast<int32_t>(
+                base::bits::RotateRight32((uint32_t)rt_u, sa));
           }
           break;
         case DSRL:
@@ -2040,12 +2042,13 @@ void Simulator::ConfigureTypeRegister(Instruction* instr,
           if (sa == 0) {
             // Regular logical right-shift of a word by a variable number of
             // bits instruction. SA field is always equal to 0.
-            *alu_out = (uint32_t)rt_u >> rs;
+            *alu_out = static_cast<int32_t>((uint32_t)rt_u >> rs);
           } else {
             // Logical right-rotate of a word by a variable number of bits.
             // This is special case od SRLV instruction, added in MIPS32
             // Release 2. SA field is equal to 00001.
-            *alu_out = base::bits::RotateRight32((uint32_t)rt_u, rs_u);
+            *alu_out = static_cast<int32_t>(
+                base::bits::RotateRight32((uint32_t)rt_u, rs_u));
           }
           break;
         case DSRLV:
index ffafaf0..199c23c 100644 (file)
@@ -128,6 +128,22 @@ struct ParameterTraits<T*> {
   static uintptr_t Cast(void* r) { return reinterpret_cast<uintptr_t>(r); }
 };
 
+#if V8_TARGET_ARCH_MIPS64
+// Additional template specialization required for mips64 to sign-extend
+// parameters defined by calling convention.
+template <>
+struct ParameterTraits<int32_t> {
+  static int64_t Cast(int32_t r) { return static_cast<int64_t>(r); }
+};
+
+template <>
+struct ParameterTraits<uint32_t> {
+  static int64_t Cast(uint32_t r) {
+    return static_cast<int64_t>(static_cast<int32_t>(r));
+  }
+};
+#endif
+
 class CallHelper {
  public:
   explicit CallHelper(Isolate* isolate, MachineSignature* machine_sig)
@@ -214,6 +230,7 @@ class CallHelper {
     return static_cast<uintptr_t>(simulator->Call(f, 4, p1, p2, p3, p4));
   }
 
+
   template <typename R, typename F>
   R DoCall(F* f) {
     return ReturnValueTraits<R>::Cast(CallSimulator(FUNCTION_ADDR(f)));
index 4145333..ab95023 100644 (file)
@@ -166,29 +166,28 @@ const IntCmp kCmpInstructions[] = {
     {{&RawMachineAssembler::WordNotEqual, "WordNotEqual", kMips64Cmp,
       kMachInt64},
      1U},
-    {{&RawMachineAssembler::Word32Equal, "Word32Equal", kMips64Cmp32,
-      kMachInt32},
+    {{&RawMachineAssembler::Word32Equal, "Word32Equal", kMips64Cmp, kMachInt32},
      1U},
-    {{&RawMachineAssembler::Word32NotEqual, "Word32NotEqual", kMips64Cmp32,
+    {{&RawMachineAssembler::Word32NotEqual, "Word32NotEqual", kMips64Cmp,
       kMachInt32},
      1U},
-    {{&RawMachineAssembler::Int32LessThan, "Int32LessThan", kMips64Cmp32,
+    {{&RawMachineAssembler::Int32LessThan, "Int32LessThan", kMips64Cmp,
       kMachInt32},
      1U},
     {{&RawMachineAssembler::Int32LessThanOrEqual, "Int32LessThanOrEqual",
-      kMips64Cmp32, kMachInt32},
+      kMips64Cmp, kMachInt32},
      1U},
-    {{&RawMachineAssembler::Int32GreaterThan, "Int32GreaterThan", kMips64Cmp32,
+    {{&RawMachineAssembler::Int32GreaterThan, "Int32GreaterThan", kMips64Cmp,
       kMachInt32},
      1U},
     {{&RawMachineAssembler::Int32GreaterThanOrEqual, "Int32GreaterThanOrEqual",
-      kMips64Cmp32, kMachInt32},
+      kMips64Cmp, kMachInt32},
      1U},
-    {{&RawMachineAssembler::Uint32LessThan, "Uint32LessThan", kMips64Cmp32,
+    {{&RawMachineAssembler::Uint32LessThan, "Uint32LessThan", kMips64Cmp,
       kMachUint32},
      1U},
     {{&RawMachineAssembler::Uint32LessThanOrEqual, "Uint32LessThanOrEqual",
-      kMips64Cmp32, kMachUint32},
+      kMips64Cmp, kMachUint32},
      1U}};
 
 
@@ -753,7 +752,7 @@ TEST_F(InstructionSelectorTest, Word32EqualWithZero) {
     m.Return(m.Word32Equal(m.Parameter(0), m.Int32Constant(0)));
     Stream s = m.Build();
     ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(kMips64Cmp32, s[0]->arch_opcode());
+    EXPECT_EQ(kMips64Cmp, s[0]->arch_opcode());
     EXPECT_EQ(kMode_None, s[0]->addressing_mode());
     ASSERT_EQ(2U, s[0]->InputCount());
     EXPECT_EQ(1U, s[0]->OutputCount());
@@ -765,7 +764,7 @@ TEST_F(InstructionSelectorTest, Word32EqualWithZero) {
     m.Return(m.Word32Equal(m.Int32Constant(0), m.Parameter(0)));
     Stream s = m.Build();
     ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(kMips64Cmp32, s[0]->arch_opcode());
+    EXPECT_EQ(kMips64Cmp, s[0]->arch_opcode());
     EXPECT_EQ(kMode_None, s[0]->addressing_mode());
     ASSERT_EQ(2U, s[0]->InputCount());
     EXPECT_EQ(1U, s[0]->OutputCount());