[AArch64][MachineCombine] Fold FNMUL+FSUB -> FNMADD

[mcrosier]

Summary

This patch extends support in the machine combiner for forming more FNMADD instructions.

I saw a minor, but measurable above noise, improvement in spec2000/eon on Kryo.

PTAL,
Chad

[gberry]

lib/Target/AArch64/AArch64InstrInfo.cpp

3170

Can you just rename canCombineWithFMUL instead of having two identical copies?

test/CodeGen/AArch64/arm64-fml-combines.ll

1–3

It might be a good idea to add a second run line with -fp-contract=fast instead of -enable-unsafe-fp-math and get rid of the 'contract' bits on the instructions in the new test.

[mcrosier]

-Address Geoff's comments.

[mcrosier]

-Drop contract bits from tests, per Geoff's comments.

[Gerolf]

LGTM.

[mcrosier]

Approved, per Gerolf.

[mcrosier]

r302822.

include/llvm/CodeGen/MachineCombinerPattern.h

enum class MachineCombinerPattern {
   FMULADDS_OP1,
   FMULADDS_OP2,
   FMULSUBS_OP1,
   FMULSUBS_OP2,
   FMULADDD_OP1,
   FMULADDD_OP2,
   FMULSUBD_OP1,
   FMULSUBD_OP2,
+  FNMULSUBS_OP1,
+  FNMULSUBD_OP1,
   FMLAv1i32_indexed_OP1,
   FMLAv1i32_indexed_OP2,
   FMLAv1i64_indexed_OP1,
   FMLAv1i64_indexed_OP2,
   FMLAv2f32_OP2,
   FMLAv2f32_OP1,
   FMLAv2f64_OP1,
   FMLAv2f64_OP2,

lib/Target/AArch64/AArch64InstrInfo.cpp

 //
 // Is \param MO defined by a floating-point multiply and can be combined?
 static bool canCombineWithFMUL(MachineBasicBlock &MBB, MachineOperand &MO,
                                unsigned MulOpc) {
   return canCombine(MBB, MO, MulOpc);
 }
 
 // TODO: There are many more machine instruction opcodes to match:
 //       1. Other data types (integer, vectors)
 //       2. Other math / logic operations (xor, or)
 //       3. Other forms of the same operation (intrinsics and other variants)
 bool AArch64InstrInfo::isAssociativeAndCommutative(const MachineInstr &Inst) const {
   switch (Inst.getOpcode()) {
   case AArch64::FADDDrr:
   case AArch64::FADDSrr:
   case AArch64::FADDv2f32:
   case AArch64::FADDv2f64:
case AArch64::FSUBSrr:
     if (canCombineWithFMUL(MBB, Root.getOperand(2), AArch64::FMULSrr)) {
       Patterns.push_back(MachineCombinerPattern::FMULSUBS_OP2);
       Found = true;
     } else if (canCombineWithFMUL(MBB, Root.getOperand(2),
                                   AArch64::FMULv1i32_indexed)) {
       Patterns.push_back(MachineCombinerPattern::FMLSv1i32_indexed_OP2);
       Found = true;
     }
+    if (canCombineWithFMUL(MBB, Root.getOperand(1), AArch64::FNMULSrr)) {
+      Patterns.push_back(MachineCombinerPattern::FNMULSUBS_OP1);
+      Found = true;
+    }
     break;
   case AArch64::FSUBDrr:
     if (canCombineWithFMUL(MBB, Root.getOperand(1), AArch64::FMULDrr)) {
       Patterns.push_back(MachineCombinerPattern::FMULSUBD_OP1);
       Found = true;
     }
     if (canCombineWithFMUL(MBB, Root.getOperand(2), AArch64::FMULDrr)) {
       Patterns.push_back(MachineCombinerPattern::FMULSUBD_OP2);
       Found = true;
     } else if (canCombineWithFMUL(MBB, Root.getOperand(2),
                                   AArch64::FMULv1i64_indexed)) {
       Patterns.push_back(MachineCombinerPattern::FMLSv1i64_indexed_OP2);
       Found = true;
     }
+    if (canCombineWithFMUL(MBB, Root.getOperand(1), AArch64::FNMULDrr)) {
+      Patterns.push_back(MachineCombinerPattern::FNMULSUBD_OP1);
+      Found = true;
+    }
     break;
   case AArch64::FSUBv2f32:
     if (canCombineWithFMUL(MBB, Root.getOperand(2),
                            AArch64::FMULv2i32_indexed)) {
       Patterns.push_back(MachineCombinerPattern::FMLSv2i32_indexed_OP2);
       Found = true;
     } else if (canCombineWithFMUL(MBB, Root.getOperand(2),
                                   AArch64::FMULv2f32)) {
AArch64InstrInfo::isThroughputPattern(MachineCombinerPattern Pattern) const {
   case MachineCombinerPattern::FMULADDS_OP1:
   case MachineCombinerPattern::FMULADDS_OP2:
   case MachineCombinerPattern::FMULSUBS_OP1:
   case MachineCombinerPattern::FMULSUBS_OP2:
   case MachineCombinerPattern::FMULADDD_OP1:
   case MachineCombinerPattern::FMULADDD_OP2:
   case MachineCombinerPattern::FMULSUBD_OP1:
   case MachineCombinerPattern::FMULSUBD_OP2:
+  case MachineCombinerPattern::FNMULSUBS_OP1:
+  case MachineCombinerPattern::FNMULSUBD_OP1:
   case MachineCombinerPattern::FMLAv1i32_indexed_OP1:
   case MachineCombinerPattern::FMLAv1i32_indexed_OP2:
   case MachineCombinerPattern::FMLAv1i64_indexed_OP1:
   case MachineCombinerPattern::FMLAv1i64_indexed_OP2:
   case MachineCombinerPattern::FMLAv2f32_OP2:
   case MachineCombinerPattern::FMLAv2f32_OP1:
   case MachineCombinerPattern::FMLAv2f64_OP1:
   case MachineCombinerPattern::FMLAv2f64_OP2:
if (Pattern == MachineCombinerPattern::FMULSUBS_OP1) {
       RC = &AArch64::FPR32RegClass;
     } else {
       Opc = AArch64::FNMSUBDrrr;
       RC = &AArch64::FPR64RegClass;
     }
     MUL = genFusedMultiply(MF, MRI, TII, Root, InsInstrs, 1, Opc, RC);
     break;
   }
+
+  case MachineCombinerPattern::FNMULSUBS_OP1:
+  case MachineCombinerPattern::FNMULSUBD_OP1: {
+    // FNMUL I=A,B,0
+    // FSUB R,I,C
+    // ==> FNMADD R,A,B,C // = -A*B - C
+    // --- Create(FNMADD);
+    if (Pattern == MachineCombinerPattern::FNMULSUBS_OP1) {
+      Opc = AArch64::FNMADDSrrr;
+      RC = &AArch64::FPR32RegClass;
+    } else {
+      Opc = AArch64::FNMADDDrrr;
+      RC = &AArch64::FPR64RegClass;
+    }
+    MUL = genFusedMultiply(MF, MRI, TII, Root, InsInstrs, 1, Opc, RC);
+    break;
+  }
+
   case MachineCombinerPattern::FMULSUBS_OP2:
   case MachineCombinerPattern::FMULSUBD_OP2: {
     // FMUL I=A,B,0
     // FSUB R,C,I
     // ==> FMSUB R,A,B,C (computes C - A*B)
     // --- Create(FMSUB);
     if (Pattern == MachineCombinerPattern::FMULSUBS_OP2) {
       Opc = AArch64::FMSUBSrrr;

test/CodeGen/AArch64/arm64-fml-combines.ll

-; RUN: llc < %s -O=3 -mtriple=arm64-apple-ios  -mcpu=cyclone -enable-unsafe-fp-math | FileCheck %s
+; RUN: llc < %s -O3 -mtriple=arm64-apple-ios -enable-unsafe-fp-math | FileCheck %s
+; RUN: llc < %s -O3 -mtriple=arm64-apple-ios -fp-contract=fast | FileCheck %s
+
 define void @foo_2d(double* %src) {
 entry:
   %arrayidx1 = getelementptr inbounds double, double* %src, i64 5
   %arrayidx2 = getelementptr inbounds double, double* %src, i64 11
   %tmp = bitcast double* %arrayidx1 to <2 x double>*
   br label %for.body
 
 ; CHECK-LABEL: %for.body
for.body:                                         ; preds = %for.body, %entry
   %mul3 = fmul fast float %mul, %e10
   store float %mul3, float* %arrayidx2, align 8
   %exitcond = icmp eq i64 %indvars.iv.next, 25
   br i1 %exitcond, label %for.end, label %for.body
 
 for.end:                                          ; preds = %for.body
   ret void
 }
+
+; CHECK-LABEL: test1:
+; CHECK: fnmadd s0, s0, s1, s2
+define float @test1(float %a, float %b, float %c) {
+entry:
+  %0 = fmul float %a, %b
+  %mul = fsub float -0.000000e+00, %0
+  %sub1 = fsub float %mul, %c
+  ret float %sub1
+}
+
+; CHECK-LABEL: test2:
+; CHECK: fnmadd d0, d0, d1, d2
+define double @test2(double %a, double %b, double %c) {
+entry:
+  %0 = fmul double %a, %b
+  %mul = fsub double -0.000000e+00, %0
+  %sub1 = fsub double %mul, %c
+  ret double %sub1
+}