[PATCH] D23313: X86: FMA intrinsic + FNEG - sequence optimization

[Sanjay Patel via llvm-commits]

spatel added inline comments.

================
Comment at: ../lib/Target/X86/X86ISelLowering.cpp:4869
@@ -4868,3 +4868,3 @@
 
-static const Constant *getTargetShuffleMaskConstant(SDValue MaskNode) {
+static const Constant *getTargetConstantFromNode(SDValue MaskNode) {
   MaskNode = peekThroughBitcasts(MaskNode);
----------------
It's good to generalize this, but you should also change the variable names from Mask* to something generic since it's not about shuffles anymore. Please make this change ahead of the fneg changes to reduce the size of the patch.



================
Comment at: ../lib/Target/X86/X86ISelLowering.cpp:30361-30373
@@ -30360,3 +30360,15 @@
                          Arg.getOperand(1), Arg.getOperand(2));
+    case X86ISD::FMADD_RND:
+      return DAG.getNode(X86ISD::FNMSUB_RND, DL, VT, Arg.getOperand(0),
+                         Arg.getOperand(1), Arg.getOperand(2), Arg.getOperand(3));
+    case X86ISD::FMSUB_RND:
+      return DAG.getNode(X86ISD::FNMADD_RND, DL, VT, Arg.getOperand(0),
+                         Arg.getOperand(1), Arg.getOperand(2), Arg.getOperand(3));
+    case X86ISD::FNMADD_RND:
+      return DAG.getNode(X86ISD::FMSUB_RND, DL, VT, Arg.getOperand(0),
+                         Arg.getOperand(1), Arg.getOperand(2), Arg.getOperand(3));
+    case X86ISD::FNMSUB_RND:
+      return DAG.getNode(X86ISD::FMADD_RND, DL, VT, Arg.getOperand(0),
+                         Arg.getOperand(1), Arg.getOperand(2), Arg.getOperand(3));
     }
   }
----------------
Are the *RND changes independent of the fneg bug fix? If yes, can you separate them from this patch?

================
Comment at: ../lib/Target/X86/X86ISelLowering.cpp:30402
@@ +30401,3 @@
+
+/// returns true if the node \p N is FNEG(x) or FXOR (x, 0x80000000)
+bool isFNEG(const SDNode *N) {
----------------
returns -> Returns
Add period to end of sentence.

================
Comment at: ../lib/Target/X86/X86ISelLowering.cpp:30411
@@ +30410,3 @@
+
+    auto isSingBitValue = [&](const ConstantFP *C) {
+      return C->getValueAPF().bitcastToAPInt() == APInt::getSignBit(EltBits);
----------------
Sing -> Sign


Repository:
  rL LLVM

https://reviews.llvm.org/D23313