MIPS64: Remove optimizations with MADD and MSUB.

On Loongson 3A, MADD/MSUB instructions are actually fused MADD/MSUB and
they can cause failure in some of the tests. Since this optimization is
rarely used, and not used at all on MIPS64R6, MADD/MSUB instructions
are removed from the source base.

TEST=
BUG=

Change-Id: Ifbb5508a62731bb061f332864ffd1e210e97f963
Reviewed-on: https://chromium-review.googlesource.com/558066Reviewed-by: Ivica Bogosavljevic <ivica.bogosavljevic@imgtec.com>
Commit-Queue: Ivica Bogosavljevic <ivica.bogosavljevic@imgtec.com>
Cr-Commit-Position: refs/heads/master@{#46387}

src/compiler/mips64/code-generator-mips64.cc

@@ -1376,26 +1376,6 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
       __ sub_d(i.OutputDoubleRegister(), i.InputDoubleRegister(0),
                i.InputDoubleRegister(1));
       break;
-    case kMips64MaddS:
-      __ Madd_s(i.OutputFloatRegister(), i.InputFloatRegister(0),
-                i.InputFloatRegister(1), i.InputFloatRegister(2),
-                kScratchDoubleReg);
-      break;
-    case kMips64MaddD:
-      __ Madd_d(i.OutputDoubleRegister(), i.InputDoubleRegister(0),
-                i.InputDoubleRegister(1), i.InputDoubleRegister(2),
-                kScratchDoubleReg);
-      break;
-    case kMips64MsubS:
-      __ Msub_s(i.OutputFloatRegister(), i.InputFloatRegister(0),
-                i.InputFloatRegister(1), i.InputFloatRegister(2),
-                kScratchDoubleReg);
-      break;
-    case kMips64MsubD:
-      __ Msub_d(i.OutputDoubleRegister(), i.InputDoubleRegister(0),
-                i.InputDoubleRegister(1), i.InputDoubleRegister(2),
-                kScratchDoubleReg);
-      break;
     case kMips64MulD:
       // TODO(plind): add special case: right op is -1.0, see arm port.
       __ mul_d(i.OutputDoubleRegister(), i.InputDoubleRegister(0),

src/compiler/mips64/instruction-codes-mips64.h

@@ -85,10 +85,6 @@ namespace compiler {
   V(Mips64SqrtD)                    \
   V(Mips64MaxD)                     \
   V(Mips64MinD)                     \
-  V(Mips64MaddS)                    \
-  V(Mips64MaddD)                    \
-  V(Mips64MsubS)                    \
-  V(Mips64MsubD)                    \
   V(Mips64Float64RoundDown)         \
   V(Mips64Float64RoundTruncate)     \
   V(Mips64Float64RoundUp)           \

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

@@ -1494,84 +1494,28 @@ void InstructionSelector::VisitBitcastInt64ToFloat64(Node* node) {
 
 
 void InstructionSelector::VisitFloat32Add(Node* node) {
-  Mips64OperandGenerator g(this);
-  if (kArchVariant == kMips64r2) {  // Select Madd.S(z, x, y).
-    Float32BinopMatcher m(node);
-    if (m.left().IsFloat32Mul() && CanCover(node, m.left().node())) {
-      // For Add.S(Mul.S(x, y), z):
-      Float32BinopMatcher mleft(m.left().node());
-      Emit(kMips64MaddS, g.DefineAsRegister(node),
-           g.UseRegister(m.right().node()), g.UseRegister(mleft.left().node()),
-           g.UseRegister(mleft.right().node()));
-      return;
-    }
-    if (m.right().IsFloat32Mul() && CanCover(node, m.right().node())) {
-      // For Add.S(x, Mul.S(y, z)):
-      Float32BinopMatcher mright(m.right().node());
-      Emit(kMips64MaddS, g.DefineAsRegister(node),
-           g.UseRegister(m.left().node()), g.UseRegister(mright.left().node()),
-           g.UseRegister(mright.right().node()));
-      return;
-    }
-  }
+  // Optimization with Madd.S(z, x, y) is intentionally removed.
+  // See explanation for madd_s in assembler-mips64.cc.
   VisitRRR(this, kMips64AddS, node);
 }
 
 
 void InstructionSelector::VisitFloat64Add(Node* node) {
-  Mips64OperandGenerator g(this);
-  if (kArchVariant == kMips64r2) {  // Select Madd.S(z, x, y).
-    Float64BinopMatcher m(node);
-    if (m.left().IsFloat64Mul() && CanCover(node, m.left().node())) {
-      // For Add.D(Mul.D(x, y), z):
-      Float64BinopMatcher mleft(m.left().node());
-      Emit(kMips64MaddD, g.DefineAsRegister(node),
-           g.UseRegister(m.right().node()), g.UseRegister(mleft.left().node()),
-           g.UseRegister(mleft.right().node()));
-      return;
-    }
-    if (m.right().IsFloat64Mul() && CanCover(node, m.right().node())) {
-      // For Add.D(x, Mul.D(y, z)):
-      Float64BinopMatcher mright(m.right().node());
-      Emit(kMips64MaddD, g.DefineAsRegister(node),
-           g.UseRegister(m.left().node()), g.UseRegister(mright.left().node()),
-           g.UseRegister(mright.right().node()));
-      return;
-    }
-  }
+  // Optimization with Madd.D(z, x, y) is intentionally removed.
+  // See explanation for madd_d in assembler-mips64.cc.
   VisitRRR(this, kMips64AddD, node);
 }
 
 
 void InstructionSelector::VisitFloat32Sub(Node* node) {
-  Mips64OperandGenerator g(this);
-  if (kArchVariant == kMips64r2) {  // Select Madd.S(z, x, y).
-    Float32BinopMatcher m(node);
-    if (m.left().IsFloat32Mul() && CanCover(node, m.left().node())) {
-      // For Sub.S(Mul.S(x,y), z) select Msub.S(z, x, y).
-      Float32BinopMatcher mleft(m.left().node());
-      Emit(kMips64MsubS, g.DefineAsRegister(node),
-           g.UseRegister(m.right().node()), g.UseRegister(mleft.left().node()),
-           g.UseRegister(mleft.right().node()));
-      return;
-    }
-  }
+  // Optimization with Msub.S(z, x, y) is intentionally removed.
+  // See explanation for madd_s in assembler-mips64.cc.
   VisitRRR(this, kMips64SubS, node);
 }
 
 void InstructionSelector::VisitFloat64Sub(Node* node) {
-  Mips64OperandGenerator g(this);
-  if (kArchVariant == kMips64r2) {  // Select Madd.S(z, x, y).
-    Float64BinopMatcher m(node);
-    if (m.left().IsFloat64Mul() && CanCover(node, m.left().node())) {
-      // For Sub.D(Mul.S(x,y), z) select Msub.D(z, x, y).
-      Float64BinopMatcher mleft(m.left().node());
-      Emit(kMips64MsubD, g.DefineAsRegister(node),
-           g.UseRegister(m.right().node()), g.UseRegister(mleft.left().node()),
-           g.UseRegister(mleft.right().node()));
-      return;
-    }
-  }
+  // Optimization with Msub.D(z, x, y) is intentionally removed.
+  // See explanation for madd_d in assembler-mips64.cc.
   VisitRRR(this, kMips64SubD, node);
 }
 

src/mips64/assembler-mips64.cc

@@ -2877,26 +2877,30 @@ void Assembler::mul_d(FPURegister fd, FPURegister fs, FPURegister ft) {
 
 void Assembler::madd_s(FPURegister fd, FPURegister fr, FPURegister fs,
                        FPURegister ft) {
-  DCHECK(kArchVariant == kMips64r2);
-  GenInstrRegister(COP1X, fr, ft, fs, fd, MADD_S);
+  // On Loongson 3A (MIPS64R2), MADD.S instruction is actually fused MADD.S and
+  // this causes failure in some of the tests. Since this optimization is rarely
+  // used, and not used at all on MIPS64R6, this isntruction is removed.
+  UNREACHABLE();
 }
 
 void Assembler::madd_d(FPURegister fd, FPURegister fr, FPURegister fs,
     FPURegister ft) {
-  DCHECK(kArchVariant == kMips64r2);
-  GenInstrRegister(COP1X, fr, ft, fs, fd, MADD_D);
+  // On Loongson 3A (MIPS64R2), MADD.D instruction is actually fused MADD.D and
+  // this causes failure in some of the tests. Since this optimization is rarely
+  // used, and not used at all on MIPS64R6, this isntruction is removed.
+  UNREACHABLE();
 }
 
 void Assembler::msub_s(FPURegister fd, FPURegister fr, FPURegister fs,
                        FPURegister ft) {
-  DCHECK(kArchVariant == kMips64r2);
-  GenInstrRegister(COP1X, fr, ft, fs, fd, MSUB_S);
+  // See explanation for instruction madd_s.
+  UNREACHABLE();
 }
 
 void Assembler::msub_d(FPURegister fd, FPURegister fr, FPURegister fs,
                        FPURegister ft) {
-  DCHECK(kArchVariant == kMips64r2);
-  GenInstrRegister(COP1X, fr, ft, fs, fd, MSUB_D);
+  // See explanation for instruction madd_d.
+  UNREACHABLE();
 }
 
 void Assembler::maddf_s(FPURegister fd, FPURegister fs, FPURegister ft) {

src/mips64/macro-assembler-mips64.cc

@@ -2613,46 +2613,30 @@ void MacroAssembler::Trunc_ul_s(FPURegister fd, Register rs,
 
 void MacroAssembler::Madd_s(FPURegister fd, FPURegister fr, FPURegister fs,
                             FPURegister ft, FPURegister scratch) {
-  if (kArchVariant == kMips64r2) {
-    madd_s(fd, fr, fs, ft);
-  } else {
-    DCHECK(!fr.is(scratch) && !fs.is(scratch) && !ft.is(scratch));
-    mul_s(scratch, fs, ft);
-    add_s(fd, fr, scratch);
-  }
+  DCHECK(!fr.is(scratch) && !fs.is(scratch) && !ft.is(scratch));
+  mul_s(scratch, fs, ft);
+  add_s(fd, fr, scratch);
 }
 
 void MacroAssembler::Madd_d(FPURegister fd, FPURegister fr, FPURegister fs,
     FPURegister ft, FPURegister scratch) {
-  if (kArchVariant == kMips64r2) {
-    madd_d(fd, fr, fs, ft);
-  } else {
-    DCHECK(!fr.is(scratch) && !fs.is(scratch) && !ft.is(scratch));
-    mul_d(scratch, fs, ft);
-    add_d(fd, fr, scratch);
-  }
+  DCHECK(!fr.is(scratch) && !fs.is(scratch) && !ft.is(scratch));
+  mul_d(scratch, fs, ft);
+  add_d(fd, fr, scratch);
 }
 
 void MacroAssembler::Msub_s(FPURegister fd, FPURegister fr, FPURegister fs,
                             FPURegister ft, FPURegister scratch) {
-  if (kArchVariant == kMips64r2) {
-    msub_s(fd, fr, fs, ft);
-  } else {
-    DCHECK(!fr.is(scratch) && !fs.is(scratch) && !ft.is(scratch));
-    mul_s(scratch, fs, ft);
-    sub_s(fd, scratch, fr);
-  }
+  DCHECK(!fr.is(scratch) && !fs.is(scratch) && !ft.is(scratch));
+  mul_s(scratch, fs, ft);
+  sub_s(fd, scratch, fr);
 }
 
 void MacroAssembler::Msub_d(FPURegister fd, FPURegister fr, FPURegister fs,
                             FPURegister ft, FPURegister scratch) {
-  if (kArchVariant == kMips64r2) {
-    msub_d(fd, fr, fs, ft);
-  } else {
-    DCHECK(!fr.is(scratch) && !fs.is(scratch) && !ft.is(scratch));
-    mul_d(scratch, fs, ft);
-    sub_d(fd, scratch, fr);
-  }
+  DCHECK(!fr.is(scratch) && !fs.is(scratch) && !ft.is(scratch));
+  mul_d(scratch, fs, ft);
+  sub_d(fd, scratch, fr);
 }
 
 void MacroAssembler::BranchFCommon(SecondaryField sizeField, Label* target,

test/cctest/test-assembler-mips64.cc

@@ -316,7 +316,7 @@ TEST(MIPS3) {
   if (kArchVariant == kMips64r2) {
     __ Ldc1(f4, MemOperand(a0, offsetof(T, h)));
     __ Ldc1(f6, MemOperand(a0, offsetof(T, i)));
-    __ madd_d(f14, f6, f4, f6);
+    __ Madd_d(f14, f6, f4, f6, f8);
     __ Sdc1(f14, MemOperand(a0, offsetof(T, h)));
   }
 
@@ -6165,9 +6165,9 @@ void helper_madd_msub_maddf_msubf(F func) {
 TEST(madd_msub_s) {
   if (kArchVariant == kMips64r6) return;
   helper_madd_msub_maddf_msubf<float>([](MacroAssembler& assm) {
-    __ madd_s(f10, f4, f6, f8);
+    __ Madd_s(f10, f4, f6, f8, f12);
     __ Swc1(f10, MemOperand(a0, offsetof(TestCaseMaddMsub<float>, fd_add)));
-    __ msub_s(f16, f4, f6, f8);
+    __ Msub_s(f16, f4, f6, f8, f12);
     __ Swc1(f16, MemOperand(a0, offsetof(TestCaseMaddMsub<float>, fd_sub)));
   });
 }
@@ -6175,9 +6175,9 @@ TEST(madd_msub_s) {
 TEST(madd_msub_d) {
   if (kArchVariant == kMips64r6) return;
   helper_madd_msub_maddf_msubf<double>([](MacroAssembler& assm) {
-    __ madd_d(f10, f4, f6, f8);
+    __ Madd_d(f10, f4, f6, f8, f12);
     __ Sdc1(f10, MemOperand(a0, offsetof(TestCaseMaddMsub<double>, fd_add)));
-    __ msub_d(f16, f4, f6, f8);
+    __ Msub_d(f16, f4, f6, f8, f12);
     __ Sdc1(f16, MemOperand(a0, offsetof(TestCaseMaddMsub<double>, fd_sub)));
   });
 }

test/cctest/test-disasm-mips64.cc

@@ -1306,12 +1306,6 @@ TEST(ctc1_cfc1_disasm) {
 
 TEST(madd_msub_maddf_msubf) {
   SET_UP();
-  if (kArchVariant == kMips64r2) {
-    COMPARE(madd_s(f4, f6, f8, f10), "4cca4120       madd.s  f4, f6, f8, f10");
-    COMPARE(madd_d(f4, f6, f8, f10), "4cca4121       madd.d  f4, f6, f8, f10");
-    COMPARE(msub_s(f4, f6, f8, f10), "4cca4128       msub.s  f4, f6, f8, f10");
-    COMPARE(msub_d(f4, f6, f8, f10), "4cca4129       msub.d  f4, f6, f8, f10");
-  }
   if (kArchVariant == kMips64r6) {
     COMPARE(maddf_s(f4, f8, f10), "460a4118       maddf.s  f4, f8, f10");
     COMPARE(maddf_d(f4, f8, f10), "462a4118       maddf.d  f4, f8, f10");

test/unittests/compiler/mips64/instruction-selector-mips64-unittest.cc

@@ -1751,157 +1751,6 @@ TEST_F(InstructionSelectorTest, Float64Abs) {
   EXPECT_EQ(s.ToVreg(n), s.ToVreg(s[0]->Output()));
 }
 
-TEST_F(InstructionSelectorTest, Float32AddWithFloat32Mul) {
-  if (kArchVariant != kMips64r2) {
-    return;
-  }
-  {
-    StreamBuilder m(this, MachineType::Float32(), MachineType::Float32(),
-                    MachineType::Float32(), MachineType::Float32());
-    Node* const p0 = m.Parameter(0);
-    Node* const p1 = m.Parameter(1);
-    Node* const p2 = m.Parameter(2);
-    Node* const n = m.Float32Add(m.Float32Mul(p0, p1), p2);
-    m.Return(n);
-    Stream s = m.Build();
-    ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(kMips64MaddS, s[0]->arch_opcode());
-    ASSERT_EQ(3U, s[0]->InputCount());
-    EXPECT_EQ(s.ToVreg(p2), s.ToVreg(s[0]->InputAt(0)));
-    EXPECT_EQ(s.ToVreg(p0), s.ToVreg(s[0]->InputAt(1)));
-    EXPECT_EQ(s.ToVreg(p1), s.ToVreg(s[0]->InputAt(2)));
-    ASSERT_EQ(1U, s[0]->OutputCount());
-    EXPECT_FALSE(
-        UnallocatedOperand::cast(s[0]->Output())->HasSameAsInputPolicy());
-    EXPECT_EQ(s.ToVreg(n), s.ToVreg(s[0]->Output()));
-    EXPECT_EQ(kFlags_none, s[0]->flags_mode());
-  }
-  {
-    StreamBuilder m(this, MachineType::Float32(), MachineType::Float32(),
-                    MachineType::Float32(), MachineType::Float32());
-    Node* const p0 = m.Parameter(0);
-    Node* const p1 = m.Parameter(1);
-    Node* const p2 = m.Parameter(2);
-    Node* const n = m.Float32Add(p0, m.Float32Mul(p1, p2));
-    m.Return(n);
-    Stream s = m.Build();
-    ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(kMips64MaddS, s[0]->arch_opcode());
-    ASSERT_EQ(3U, s[0]->InputCount());
-    EXPECT_EQ(s.ToVreg(p0), s.ToVreg(s[0]->InputAt(0)));
-    EXPECT_EQ(s.ToVreg(p1), s.ToVreg(s[0]->InputAt(1)));
-    EXPECT_EQ(s.ToVreg(p2), s.ToVreg(s[0]->InputAt(2)));
-    ASSERT_EQ(1U, s[0]->OutputCount());
-    EXPECT_FALSE(
-        UnallocatedOperand::cast(s[0]->Output())->HasSameAsInputPolicy());
-    EXPECT_EQ(s.ToVreg(n), s.ToVreg(s[0]->Output()));
-    EXPECT_EQ(kFlags_none, s[0]->flags_mode());
-  }
-}
-
-TEST_F(InstructionSelectorTest, Float64AddWithFloat64Mul) {
-  if (kArchVariant != kMips64r2) {
-    return;
-  }
-  {
-    StreamBuilder m(this, MachineType::Float64(), MachineType::Float64(),
-                    MachineType::Float64(), MachineType::Float64());
-    Node* const p0 = m.Parameter(0);
-    Node* const p1 = m.Parameter(1);
-    Node* const p2 = m.Parameter(2);
-    Node* const n = m.Float64Add(m.Float64Mul(p0, p1), p2);
-    m.Return(n);
-    Stream s = m.Build();
-    ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(kMips64MaddD, s[0]->arch_opcode());
-    ASSERT_EQ(3U, s[0]->InputCount());
-    EXPECT_EQ(s.ToVreg(p2), s.ToVreg(s[0]->InputAt(0)));
-    EXPECT_EQ(s.ToVreg(p0), s.ToVreg(s[0]->InputAt(1)));
-    EXPECT_EQ(s.ToVreg(p1), s.ToVreg(s[0]->InputAt(2)));
-    ASSERT_EQ(1U, s[0]->OutputCount());
-    EXPECT_FALSE(
-        UnallocatedOperand::cast(s[0]->Output())->HasSameAsInputPolicy());
-    EXPECT_EQ(s.ToVreg(n), s.ToVreg(s[0]->Output()));
-    EXPECT_EQ(kFlags_none, s[0]->flags_mode());
-  }
-  {
-    StreamBuilder m(this, MachineType::Float64(), MachineType::Float64(),
-                    MachineType::Float64(), MachineType::Float64());
-    Node* const p0 = m.Parameter(0);
-    Node* const p1 = m.Parameter(1);
-    Node* const p2 = m.Parameter(2);
-    Node* const n = m.Float64Add(p0, m.Float64Mul(p1, p2));
-    m.Return(n);
-    Stream s = m.Build();
-    ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(kMips64MaddD, s[0]->arch_opcode());
-    ASSERT_EQ(3U, s[0]->InputCount());
-    EXPECT_EQ(s.ToVreg(p0), s.ToVreg(s[0]->InputAt(0)));
-    EXPECT_EQ(s.ToVreg(p1), s.ToVreg(s[0]->InputAt(1)));
-    EXPECT_EQ(s.ToVreg(p2), s.ToVreg(s[0]->InputAt(2)));
-    ASSERT_EQ(1U, s[0]->OutputCount());
-    EXPECT_FALSE(
-        UnallocatedOperand::cast(s[0]->Output())->HasSameAsInputPolicy());
-    EXPECT_EQ(s.ToVreg(n), s.ToVreg(s[0]->Output()));
-    EXPECT_EQ(kFlags_none, s[0]->flags_mode());
-  }
-}
-
-TEST_F(InstructionSelectorTest, Float32SubWithFloat32Mul) {
-  StreamBuilder m(this, MachineType::Float32(), MachineType::Float32(),
-                  MachineType::Float32(), MachineType::Float32());
-  if (kArchVariant != kMips64r2) {
-    return;
-  }
-  {
-    Node* const p0 = m.Parameter(0);
-    Node* const p1 = m.Parameter(1);
-    Node* const p2 = m.Parameter(2);
-    Node* n;
-    n = m.Float32Sub(m.Float32Mul(p1, p2), p0);
-    m.Return(n);
-    Stream s = m.Build();
-    ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(kMips64MsubS, s[0]->arch_opcode());
-    ASSERT_EQ(3U, s[0]->InputCount());
-    EXPECT_EQ(s.ToVreg(p0), s.ToVreg(s[0]->InputAt(0)));
-    EXPECT_EQ(s.ToVreg(p1), s.ToVreg(s[0]->InputAt(1)));
-    EXPECT_EQ(s.ToVreg(p2), s.ToVreg(s[0]->InputAt(2)));
-    ASSERT_EQ(1U, s[0]->OutputCount());
-    EXPECT_FALSE(
-        UnallocatedOperand::cast(s[0]->Output())->HasSameAsInputPolicy());
-    EXPECT_EQ(s.ToVreg(n), s.ToVreg(s[0]->Output()));
-    EXPECT_EQ(kFlags_none, s[0]->flags_mode());
-  }
-}
-
-TEST_F(InstructionSelectorTest, Float64SubWithFloat64Mul) {
-  StreamBuilder m(this, MachineType::Float64(), MachineType::Float64(),
-                  MachineType::Float64(), MachineType::Float64());
-  if (kArchVariant != kMips64r2) {
-    return;
-  }
-  {
-    Node* const p0 = m.Parameter(0);
-    Node* const p1 = m.Parameter(1);
-    Node* const p2 = m.Parameter(2);
-    Node* n;
-    n = m.Float64Sub(m.Float64Mul(p1, p2), p0);
-    m.Return(n);
-    Stream s = m.Build();
-    ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(kMips64MsubD, s[0]->arch_opcode());
-    ASSERT_EQ(3U, s[0]->InputCount());
-    EXPECT_EQ(s.ToVreg(p0), s.ToVreg(s[0]->InputAt(0)));
-    EXPECT_EQ(s.ToVreg(p1), s.ToVreg(s[0]->InputAt(1)));
-    EXPECT_EQ(s.ToVreg(p2), s.ToVreg(s[0]->InputAt(2)));
-    ASSERT_EQ(1U, s[0]->OutputCount());
-    EXPECT_FALSE(
-        UnallocatedOperand::cast(s[0]->Output())->HasSameAsInputPolicy());
-    EXPECT_EQ(s.ToVreg(n), s.ToVreg(s[0]->Output()));
-    EXPECT_EQ(kFlags_none, s[0]->flags_mode());
-  }
-}
 
 TEST_F(InstructionSelectorTest, Float64Max) {
   StreamBuilder m(this, MachineType::Float64(), MachineType::Float64(),