[PATCH] Combine fcmp + select to fminnum / fmaxnum if no nans and legal

[Ahmed Bougacha]

The NaN part LGTM, but what happens on 0?

Say:

  select +0.0, -0.0, (fcmp lt +0.0, -0.0)

turns into:

  fminnum +0.0, -0.0

My understanding is, the first returns -0.0 (because they compare equal, and not "lt").
But it's unspecified which the second returns, so if the implementation returns the first operand when both are zero, it would return a "different" result here, +0.0.
Does that make sense?

-Ahmed


REPOSITORY
  rL LLVM

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4626
@@ +4625,3 @@
+                                   const TargetLowering &TLI,
+                                   SelectionDAG &DAG) {
+  if (!(LHS == True && RHS == False) && !(LHS == False && RHS == True))
----------------
Nit: clang-format? (or at least the same formatting as the call, with e.g. LHS/RHS on the same line.)

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4743
@@ +4742,3 @@
+    const TargetOptions &Options = DAG.getTarget().Options;
+    if (Options.NoNaNsFPMath && VT.isFloatingPoint() && N0.hasOneUse()) {
+      ISD::CondCode CC = cast<CondCodeSDNode>(N0.getOperand(2))->get();
----------------
Optionally, how about checking isKnownNeverNaN for both operands?
Also, the non-U/-O CondCodes don't care about NaNs, so it would be fine to bypass NoNaNsFPMath in that case as well, no?  Though I'm not sure that combination ever happens anyway.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4750
@@ +4749,3 @@
+        N1, N2,
+        CC, TLI, DAG);
+      if (FMinMax)
----------------
Nit: same, format?

http://reviews.llvm.org/D6744

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/