Do not recombine FMA when that is not needed.

[deadalnix]

Summary

As per title. This creates useless recombines.

[deadalnix]

Do it in all cases.

[zvi]

lib/Target/X86/X86ISelLowering.cpp

34158

In addition to this change, would it be better to refactor the if-else chain into a switch statement?

[deadalnix]

lib/Target/X86/X86ISelLowering.cpp

34158

You tell me :) I'm not such a siwhc with another switch in each 'case' would be clearer TBH.

[deadalnix]

Ping ?

[RKSimon]

LGTM - add comments if you can.

lib/Target/X86/X86ISelLowering.cpp

34532

comment

34563

comment

[deadalnix]

Add comments

lib/Target/X86/X86ISelLowering.cpp

 
   unsigned NewOpcode;
   if (!NegMul)
     NewOpcode = (!NegC) ? X86ISD::FMADD : X86ISD::FMSUB;
   else
     NewOpcode = (!NegC) ? X86ISD::FNMADD : X86ISD::FNMSUB;
 
 
   if (N->getOpcode() == X86ISD::FMADD_RND) {
     switch (NewOpcode) {
     case X86ISD::FMADD:  NewOpcode = X86ISD::FMADD_RND; break;
     case X86ISD::FMSUB:  NewOpcode = X86ISD::FMSUB_RND; break;
     case X86ISD::FNMADD: NewOpcode = X86ISD::FNMADD_RND; break;
     case X86ISD::FNMSUB: NewOpcode = X86ISD::FNMSUB_RND; break;
     }
   } else if (N->getOpcode() == X86ISD::FMADDS1_RND) {
     switch (NewOpcode) {
     case X86ISD::FMADD:  NewOpcode = X86ISD::FMADDS1_RND; break;
     case X86ISD::FMSUB:  NewOpcode = X86ISD::FMSUBS1_RND; break;
     case X86ISD::FNMADD: NewOpcode = X86ISD::FNMADDS1_RND; break;
     case X86ISD::FNMSUB: NewOpcode = X86ISD::FNMSUBS1_RND; break;
     }
   } else if (N->getOpcode() == X86ISD::FMADDS3_RND) {
     switch (NewOpcode) {
     case X86ISD::FMADD:  NewOpcode = X86ISD::FMADDS3_RND; break;
     case X86ISD::FMSUB:  NewOpcode = X86ISD::FMSUBS3_RND; break;
     case X86ISD::FNMADD: NewOpcode = X86ISD::FNMADDS3_RND; break;
     case X86ISD::FNMSUB: NewOpcode = X86ISD::FNMSUBS3_RND; break;
     }
   } else {
     assert((N->getOpcode() == X86ISD::FMADD || N->getOpcode() == ISD::FMA) &&
            "Unexpected opcode!");
-    return DAG.getNode(NewOpcode, dl, VT, A, B, C);
+    if (NewOpcode != N->getOpcode())
+      return DAG.getNode(NewOpcode, dl, VT, A, B, C);
+    return SDValue();
   }
 
   return DAG.getNode(NewOpcode, dl, VT, A, B, C, N->getOperand(3));
 }
 
 static SDValue combineZext(SDNode *N, SelectionDAG &DAG,
                            TargetLowering::DAGCombinerInfo &DCI,
                            const X86Subtarget &Subtarget) {

 
   // Transform (SINT_TO_FP (i64 ...)) into an x87 operation if we have
   // a 32-bit target where SSE doesn't support i64->FP operations.
   if (!Subtarget.useSoftFloat() && Op0.getOpcode() == ISD::LOAD) {
     LoadSDNode *Ld = cast<LoadSDNode>(Op0.getNode());
     EVT LdVT = Ld->getValueType(0);
 
     // This transformation is not supported if the result type is f16 or f128.
     if (VT == MVT::f16 || VT == MVT::f128)
       return SDValue();
 
     if (!Ld->isVolatile() && !VT.isVector() &&
         ISD::isNON_EXTLoad(Op0.getNode()) && Op0.hasOneUse() &&
         !Subtarget.is64Bit() && LdVT == MVT::i64) {
       SDValue FILDChain = Subtarget.getTargetLowering()->BuildFILD(
           SDValue(N, 0), LdVT, Ld->getChain(), Op0, DAG);
       DAG.ReplaceAllUsesOfValueWith(Op0.getValue(1), FILDChain.getValue(1));

     while (Carry.getOpcode() == ISD::TRUNCATE ||
            Carry.getOpcode() == ISD::ZERO_EXTEND ||
            Carry.getOpcode() == ISD::SIGN_EXTEND ||
            Carry.getOpcode() == ISD::ANY_EXTEND ||
            (Carry.getOpcode() == ISD::AND &&
             isOneConstant(Carry.getOperand(1))))
       Carry = Carry.getOperand(0);
 
     if (Carry.getOpcode() == X86ISD::SETCC ||
         Carry.getOpcode() == X86ISD::SETCC_CARRY) {
       if (Carry.getConstantOperandVal(0) == X86::COND_B)
         return DCI.CombineTo(N, SDValue(N, 0), Carry.getOperand(1));
     }
   }
 
   return SDValue();
 }