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

[Mehdi AMINI]

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4627
@@ +4626,3 @@
+                                   SelectionDAG &DAG) {
+  if (!(LHS == True && RHS == False) && !(LHS == False && RHS == True))
+    return SDValue();
----------------
What about renaming True TrueValue and False FalseValue or something like that. I was confused with c++ true/false keyword when reading this in the first place.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4656
@@ +4655,3 @@
+  }
+}
+
----------------
Can't you move the code out of the switch and having the switch only initializing the OpCode? 

================
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();
----------------
Isn't the transformation valid for SETGT, SETGE, SETLT, SETLE even without Options.NoNaNsFPMath?

================
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();
----------------
joker.eph wrote:
> Isn't the transformation valid for SETGT, SETGE, SETLT, SETLE even without Options.NoNaNsFPMath?
Why is the limit N0.hasOneUse()?


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4750
@@ +4749,3 @@
+        N1, N2,
+        CC, TLI, DAG);
+      if (FMinMax)
----------------
It is the correct way of indenting the arg list here?

http://reviews.llvm.org/D6744

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