[mips] Generate NMADD and NMSUB instructions when fneg node is present

[stanislav.ocovaj]

Summary

This patch enables generation of NMADD and NMSUB instructions when fneg node is present. These instructions are currently only generated if fsub node is present.

[sdardis]

Please upload patches with full context next time. The arc tool generates the correct context, or use git diff -U9999. Also, the line endings for .td are windows/dos endings, they should be unix line endings.

Two major changes, I've briefly outline them in line. The optimization pattern should be like the multiclass BrcondPats in MipsInstrInfo.td, so that it can be instantiated in MicroMipsInstrInfo.td as well.

The second is an increase in test coverage. The nmadd & nmsub instructions were added to MIPS-IV, so they are present in mips4, mips64{r2, r3, r5}, mips32r{2, 3, 5} and micromips. So they need to be tested at least for mips4, mips64, misp64r2, mips32r2 and micromips. We need negative testing for mips3, mips32r1 and (micromips)mips{32,64}r6.

Can you also test the output of -asm-show-inst as well. We need to ensure that _MM versions don't occur for standard mips encodings and _MM versions only occur for micromips.

lib/Target/Mips/MipsInstrFPU.td

845 ↗

(On Diff #103557)

Nit: Space after the //.

847–863 ↗

(On Diff #103557)

This should be written as multiclass, taking the instruction and register operand as parameters. You can then instantiate it three times for the different instructions.

This also requires NotInMicroMips in the AdditionalPredicates. You'll need to duplicate this in the micromips fpu td file with the appropriate micromips instructions and InMicroMips for the AdditionalPredicate.

test/CodeGen/Mips/nmadd.ll

1–2 ↗

(On Diff #103557)

Test that this works also for micromips, mips4, mips32r2 with and without -mattr=+fp64.

Also, test that n{madd/msub} does not appear for mips3, mips32 and mips32r6 + micromips32r6.

1–2 ↗

(On Diff #103557)

You can reuse the same check prefix, also consider sorting to just CHECK-NM.

4 ↗

(On Diff #103557)

This test also needs a comment describing what it's testing.

5–6 ↗

(On Diff #103557)

Drop the "; Function Attrs: norecurse nounwind readnone" and " local_unnamed_addr #0 ". they're just visual clutter / metadata that provides no purpose for this test.

8 ↗

(On Diff #103557)

Space after the ';', drop the @ and a colon after the name to match the label properly.

[sdardis]

You should also add a test with -asm-show-inst for micromips and show that we only generate instructions with _MM suffixes.

Some more comments inlined.

lib/Target/Mips/MicroMipsInstrFPU.td

274 ↗

(On Diff #108899)

These predicates don't match the ones for standard mips.

lib/Target/Mips/MipsInstrFPU.td

863 ↗

(On Diff #108899)

Spurious blank line.

test/CodeGen/Mips/nmadd.ll

9 ↗

(On Diff #108899)

That should be -mattr=micromips. You can skip testing for micromips64r6 as that backend is going to be removed.

[stanislav.ocovaj]

The combination -mcpu=mips32r6 -mattr=micromips cannot be tested at the moment due to the following error: "LLVM ERROR: ran out of registers during register allocation". This issue has been reported.

[sdardis]

LGTM with the inline nits addressed.

For microMIPS, we need to change the predicates guarding various instructions.

lib/Target/Mips/MicroMipsInstrFPU.td

273 ↗

(On Diff #111660)

This also requires NotMips32r6, as the nmadd, nmsub are missing their relevant predicates.

test/CodeGen/Mips/nmadd.ll

11 ↗

(On Diff #111660)

The check prefixes here are incorrect, they should match the mips32r6 ones.

llvm/trunk/lib/Target/Mips/MicroMipsInstrFPU.td

defm D32_MM : C_COND_ALIASES<"d", AFGR64Opnd>, HARDFLOAT,
                 ISA_MIPS1_NOT_32R6_64R6, FGR_32;
   defm D64_MM : C_COND_ALIASES<"d", FGR64Opnd>, HARDFLOAT,
                 ISA_MIPS1_NOT_32R6_64R6, FGR_64;
 
   defm : BC1_ALIASES<BC1T_MM, "bc1t", BC1F_MM, "bc1f">,
          ISA_MIPS1_NOT_32R6_64R6, HARDFLOAT;
 }
 
+// To generate NMADD and NMSUB instructions when fneg node is present
+let AdditionalPredicates = [NoNaNsFPMath, HasMadd4, InMicroMips, NotMips32r6] in {
+  defm : NMADD_NMSUB<NMADD_S_MM, NMSUB_S_MM, FGR32Opnd>;
+  defm : NMADD_NMSUB<NMADD_D32_MM, NMSUB_D32_MM, AFGR64Opnd>;
+}
+
 //===----------------------------------------------------------------------===//
 // Floating Point Patterns
 //===----------------------------------------------------------------------===//
 let AdditionalPredicates = [InMicroMips] in {
   // Patterns for loads/stores with a reg+imm operand.
   let AddedComplexity = 40 in {
     def : LoadRegImmPat<LDC1_MM, f64, load>, FGR_32;
     def : StoreRegImmPat<SDC1_MM, f64>, FGR_32;
     def : LoadRegImmPat<LWC1_MM, f32, load>;
     def : StoreRegImmPat<SWC1_MM, f32>;
   }
 }

llvm/trunk/lib/Target/Mips/MipsInstrFPU.td

 def : MipsPat<(MipsTruncIntFP FGR64Opnd:$src),
               (TRUNC_L_D64 FGR64Opnd:$src)>, FGR_64;
 
 def : MipsPat<(f32 (fpround FGR64Opnd:$src)),
               (CVT_S_D64 FGR64Opnd:$src)>, FGR_64;
 def : MipsPat<(f64 (fpextend FGR32Opnd:$src)),
               (CVT_D64_S FGR32Opnd:$src)>, FGR_64;
 
+// To generate NMADD and NMSUB instructions when fneg node is present
+multiclass NMADD_NMSUB<Instruction Nmadd, Instruction Nmsub, RegisterOperand RC> {
+  def : MipsPat<(fneg (fadd (fmul RC:$fs, RC:$ft), RC:$fr)),
+                (Nmadd RC:$fr, RC:$fs, RC:$ft)>;
+  def : MipsPat<(fneg (fsub (fmul RC:$fs, RC:$ft), RC:$fr)),
+                (Nmsub RC:$fr, RC:$fs, RC:$ft)>;
+}
+
+let AdditionalPredicates = [NoNaNsFPMath, HasMadd4, NotInMicroMips] in {
+  defm : NMADD_NMSUB<NMADD_S, NMSUB_S, FGR32Opnd>, INSN_MIPS4_32R2_NOT_32R6_64R6;
+  defm : NMADD_NMSUB<NMADD_D32, NMSUB_D32, AFGR64Opnd>, FGR_32, INSN_MIPS4_32R2_NOT_32R6_64R6;
+  defm : NMADD_NMSUB<NMADD_D64, NMSUB_D64, FGR64Opnd>, FGR_64, INSN_MIPS4_32R2_NOT_32R6_64R6;
+}
+
 // Patterns for loads/stores with a reg+imm operand.
 let AdditionalPredicates = [NotInMicroMips] in {
   let AddedComplexity = 40 in {
     def : LoadRegImmPat<LWC1, f32, load>;
     def : StoreRegImmPat<SWC1, f32>;
 
     def : LoadRegImmPat<LDC164, f64, load>, FGR_64;
     def : StoreRegImmPat<SDC164, f64>, FGR_64;
 
     def : LoadRegImmPat<LDC1, f64, load>, FGR_32;
     def : StoreRegImmPat<SDC1, f64>, FGR_32;
   }
 }

llvm/trunk/test/CodeGen/Mips/nmadd.ll

+; Check whether nmadd/nmsub instructions are properly generated
+; RUN: llc < %s -march=mipsel   -mcpu=mips32r2 -enable-no-nans-fp-math | FileCheck %s -check-prefixes=ALL,CHECK-NM
+; RUN: llc < %s -march=mipsel   -mcpu=mips32r2 -mattr=+fp64 -enable-no-nans-fp-math | FileCheck %s -check-prefixes=ALL,CHECK-NM
+; RUN: llc < %s -march=mipsel   -mcpu=mips32r2 -mattr=micromips -enable-no-nans-fp-math -asm-show-inst | FileCheck %s -check-prefixes=ALL,CHECK-NM,CHECK-MM
+; RUN: llc < %s -march=mips64el -mcpu=mips64   -target-abi=n64 -enable-no-nans-fp-math | FileCheck %s -check-prefixes=ALL,CHECK-NM-64
+; RUN: llc < %s -march=mips64el -mcpu=mips64r2 -target-abi=n64 -enable-no-nans-fp-math | FileCheck %s -check-prefixes=ALL,CHECK-NM-64
+; RUN: llc < %s -march=mips64el -mcpu=mips4    -target-abi=n64 -enable-no-nans-fp-math | FileCheck %s -check-prefixes=ALL,CHECK-NM-64
+; RUN: llc < %s -march=mipsel   -mcpu=mips32   -enable-no-nans-fp-math | FileCheck %s -check-prefixes=ALL,CHECK-NOT-NM
+; RUN: llc < %s -march=mipsel   -mcpu=mips32r6 -enable-no-nans-fp-math | FileCheck %s -check-prefixes=ALL,CHECK-NOT-NM
+; RUN: llc < %s -march=mips64el -mcpu=mips3    -target-abi=n64 -enable-no-nans-fp-math | FileCheck %s -check-prefixes=ALL,CHECK-NOT-NM-64
+; RUN-TODO: llc < %s -march=mipsel   -mcpu=mips32r6 -mattr=micromips -enable-no-nans-fp-math | FileCheck %s -check-prefixes=ALL,CHECK-NOT-NM
+
+define float @add1(float %f, float %g, float %h) local_unnamed_addr #0 {
+entry:
+; ALL-LABEL: add1
+
+; CHECK-NM-64:             nmadd.s $f0, $f14, $f12, $f13
+; CHECK-NM:                nmadd.s $f0, $f0, $f12, $f14
+; CHECK-MM:                NMADD_S_MM
+; CHECK-NOT-NM-64          mul.s $f0, $f12, $f13
+; CHECK-NOT-NM-64:         neg.s $f0, $f0
+; CHECK-NOT-NM:            mul.s $f0, $f12, $f14
+; CHECK-NOT-NM:            neg.s $f0, $f0
+
+  %mul = fmul nnan float %f, %g
+  %add = fadd nnan float %mul, %h
+  %sub = fsub nnan float -0.000000e+00, %add
+  ret float %sub
+}
+
+define double @add2(double %f, double %g, double %h) local_unnamed_addr #0 {
+entry:
+; ALL-LABEL: add2
+
+; CHECK-NM-64:             nmadd.d $f0, $f14, $f12, $f13
+; CHECK-NM:                nmadd.d $f0, $f0, $f12, $f14
+; CHECK-MM:                NMADD_D32_MM
+; CHECK-NOT-NM-64          mul.d $f0, $f12, $f13
+; CHECK-NOT-NM-64:         neg.d $f0, $f0
+; CHECK-NOT-NM:            mul.d $f0, $f12, $f14
+; CHECK-NOT-NM:            neg.d $f0, $f0
+
+  %mul = fmul nnan double %f, %g
+  %add = fadd nnan double %mul, %h
+  %sub = fsub nnan double -0.000000e+00, %add
+  ret double %sub
+}
+
+define float @sub1(float %f, float %g, float %h) local_unnamed_addr #0 {
+entry:
+; ALL-LABEL: sub1
+
+; CHECK-NM-64:             nmsub.s $f0, $f14, $f12, $f13
+; CHECK-NM:                nmsub.s $f0, $f0, $f12, $f14
+; CHECK-MM:                NMSUB_S_MM
+; CHECK-NOT-NM-64          mul.s $f0, $f12, $f13
+; CHECK-NOT-NM-64:         neg.s $f0, $f0
+; CHECK-NOT-NM:            mul.s $f0, $f12, $f14
+; CHECK-NOT-NM:            neg.s $f0, $f0
+
+  %mul = fmul nnan float %f, %g
+  %sub = fsub nnan float %mul, %h
+  %sub1 = fsub nnan float -0.000000e+00, %sub
+  ret float %sub1
+}
+
+define double @sub2(double %f, double %g, double %h) local_unnamed_addr #0 {
+entry:
+; ALL-LABEL: sub2
+
+; CHECK-NM-64:             nmsub.d $f0, $f14, $f12, $f13
+; CHECK-NM:                nmsub.d $f0, $f0, $f12, $f14
+; CHECK-MM:                NMSUB_D32_MM
+; CHECK-NOT-NM-64          mul.d $f0, $f12, $f13
+; CHECK-NOT-NM-64:         neg.d $f0, $f0
+; CHECK-NOT-NM:            mul.d $f0, $f12, $f14
+; CHECK-NOT-NM:            neg.d $f0, $f0
+
+  %mul = fmul nnan double %f, %g
+  %sub = fsub nnan double %mul, %h
+  %sub1 = fsub nnan double -0.000000e+00, %sub
+  ret double %sub1
+}