[PATCH] D40335: [WIP] X86-specific path: Implemented the fusing of MUL+SUBADD to FMSUBADD

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:7620
+  SDValue Opnd2;
+  if (isFMAddSub(Subtarget, DAG, Opnd0, Opnd1, Opnd2))
+    return DAG.getNode(X86ISD::FMSUBADD, DL, VT, Opnd0, Opnd1, Opnd2);
----------------
Rename to isFMAddSubOrFMSubAdd?


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:7628
+  // recognition.
+  if (VT.is512BitVector())
+    return SDValue();
----------------
Should just return SDValue() here no matter the type. We can't create a X86ISD::SUBADD, there's no such instruction.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:29708
+/// the fact that they're unused.
+static bool isSubAdd(SDNode *N, const X86Subtarget &Subtarget,
+                     SDValue &Opnd0, SDValue &Opnd1) {
----------------
Maybe merge this with isAddSub by passing a bool value to indicate which one you're looking for. Then you just need to factor that bool into the code that detects the FSUB/FADD


================
Comment at: lib/Target/X86/X86ISelLowering.h:204
       ADDSUB,
+      SUBADD,
 
----------------
There's no such instruction as SUBADD that doesn't include the multiply. So we shouldn't be adding this opcode.


https://reviews.llvm.org/D40335