[PATCH] D13003: [DAGCombiner] Improve FMA support for interpolation patterns

[Matt Arsenault via llvm-commits]

arsenm added a comment.

This mostly LGTM.

There aren't any tests stressing the FMAD path. AMDGPU seems to be only target using it still, and the one test change is in the expansion of an intrinsic which should be removed. If you can add some of those that would be good, otherwise I can try to do it after you commit


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7937
@@ +7936,3 @@
+  const TargetOptions &Options = DAG.getTarget().Options;
+  bool UnsafeFPMath =
+      (Options.AllowFPOpFusion == FPOpFusion::Fast || Options.UnsafeFPMath);
----------------
A better name would be AllowFusion or something like that

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7944-7946
@@ +7943,5 @@
+  // Floating-point multiply-add without intermediate rounding.
+  bool HasFMA =
+      ((!LegalOperations || TLI.isOperationLegalOrCustom(ISD::FMA, VT)) &&
+       TLI.isFMAFasterThanFMulAndFAdd(VT) && UnsafeFPMath);
+
----------------
I think the AllowFusion/UnsafeFPMath check should be first

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7953
@@ +7952,3 @@
+  // Always prefer FMAD to FMA for precision.
+  unsigned int PreferredFusedOpcode = HasFMAD ? ISD::FMAD : ISD::FMA;
+  bool Aggressive = TLI.enableAggressiveFMAFusion(VT);
----------------
Usually the int is omitted

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7954
@@ +7953,3 @@
+  unsigned int PreferredFusedOpcode = HasFMAD ? ISD::FMAD : ISD::FMA;
+  bool Aggressive = TLI.enableAggressiveFMAFusion(VT);
+
----------------
It seems wrong to use this in the FMAD case, although AMDGPU happens to not care because enableAggressiveFMAFusion always reports true and it seems to be what is used already.


Repository:
  rL LLVM

http://reviews.llvm.org/D13003