[PATCH] D87037: [DAGCombiner] Propagate FMF flags in FMA folding

[Qiu Chaofan via Phabricator via llvm-commits]

qiucf created this revision.
qiucf added reviewers: spatel, PowerPC, jsji, nemanjai, steven.zhang, aemerson, hfinkel.
Herald added subscribers: llvm-commits, ecnelises, hiraditya.
Herald added a project: LLVM.
qiucf requested review of this revision.

DAG combiner folds `(fma a 1.0 b)` into `(fadd a b)` but the flag isn't propagated into new fadd. This patch fixes that.

---

However, besides this, SDNodeFlags are easily missed/passed wrongly. (D86871 <https://reviews.llvm.org/D86871>) Is there any 'rule' about whether pass flags or not? Roughly speaking, there are (or more than?) three types of transformation in SDAG:

- The node is eliminated. Flags of it will never be passed to others.
- A node is expanded/simplified into another node(s). In this case, flags of the node should be passed to all newly created node(s).
- Some nodes are transformed to another group of nodes. Here new nodes should have intersection of original flags? Or decided by opcode type?

We can do some check against new nodes or pass flags automatically if the rule is clear. Any ideas?


https://reviews.llvm.org/D87037

Files:
  llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  llvm/test/CodeGen/PowerPC/fma-combine.ll


Index: llvm/test/CodeGen/PowerPC/fma-combine.ll
===================================================================
--- llvm/test/CodeGen/PowerPC/fma-combine.ll
+++ llvm/test/CodeGen/PowerPC/fma-combine.ll
@@ -243,17 +243,18 @@
 define double @fma_flag_propagation(double %a) {
 ; CHECK-FAST-LABEL: fma_flag_propagation:
 ; CHECK-FAST:       # %bb.0: # %entry
-; CHECK-FAST-NEXT:    xssubdp 1, 1, 1
+; CHECK-FAST-NEXT:    xxlxor 1, 1, 1
 ; CHECK-FAST-NEXT:    blr
 ;
 ; CHECK-FAST-NOVSX-LABEL: fma_flag_propagation:
 ; CHECK-FAST-NOVSX:       # %bb.0: # %entry
-; CHECK-FAST-NOVSX-NEXT:    fsub 1, 1, 1
+; CHECK-FAST-NOVSX-NEXT:    addis 3, 2, .LCPI6_0 at toc@ha
+; CHECK-FAST-NOVSX-NEXT:    lfs 1, .LCPI6_0 at toc@l(3)
 ; CHECK-FAST-NOVSX-NEXT:    blr
 ;
 ; CHECK-LABEL: fma_flag_propagation:
 ; CHECK:       # %bb.0: # %entry
-; CHECK-NEXT:    xssubdp 1, 1, 1
+; CHECK-NEXT:    xxlxor 1, 1, 1
 ; CHECK-NEXT:    blr
 entry:
   %0 = fneg double %a
Index: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -13152,11 +13152,11 @@
     if (N1CFP && N1CFP->isZero())
       return N2;
   }
-  // TODO: The FMA node should have flags that propagate to these nodes.
+
   if (N0CFP && N0CFP->isExactlyValue(1.0))
-    return DAG.getNode(ISD::FADD, SDLoc(N), VT, N1, N2);
+    return DAG.getNode(ISD::FADD, SDLoc(N), VT, N1, N2, Flags);
   if (N1CFP && N1CFP->isExactlyValue(1.0))
-    return DAG.getNode(ISD::FADD, SDLoc(N), VT, N0, N2);
+    return DAG.getNode(ISD::FADD, SDLoc(N), VT, N0, N2, Flags);
 
   // Canonicalize (fma c, x, y) -> (fma x, c, y)
   if (isConstantFPBuildVectorOrConstantFP(N0) &&
@@ -13185,19 +13185,13 @@
     }
   }
 
-  // (fma x, 1, y) -> (fadd x, y)
   // (fma x, -1, y) -> (fadd (fneg x), y)
   if (N1CFP) {
-    if (N1CFP->isExactlyValue(1.0))
-      // TODO: The FMA node should have flags that propagate to this node.
-      return DAG.getNode(ISD::FADD, DL, VT, N0, N2);
-
     if (N1CFP->isExactlyValue(-1.0) &&
         (!LegalOperations || TLI.isOperationLegal(ISD::FNEG, VT))) {
       SDValue RHSNeg = DAG.getNode(ISD::FNEG, DL, VT, N0);
       AddToWorklist(RHSNeg.getNode());
-      // TODO: The FMA node should have flags that propagate to this node.
-      return DAG.getNode(ISD::FADD, DL, VT, N2, RHSNeg);
+      return DAG.getNode(ISD::FADD, DL, VT, N2, RHSNeg, Flags);
     }
 
     // fma (fneg x), K, y -> fma x -K, y


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87037.289454.patch
Type: text/x-patch
Size: 2522 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200902/139d2248/attachment.bin>