[PATCH] D11146: [ValueTracking] Add support for floating-point minnum and maxnum

[hfinkel at anl.gov]

hfinkel added inline comments.

================
Comment at: lib/Analysis/ValueTracking.cpp:3345
@@ +3344,3 @@
+  //   (0.0 <= -0.0) ? 0.0 : -0.0 // Returns 0.0
+  //   minNum(0.0, -0.0)          // May return -0.0 or 0.0 (IEEE 754-2008 5.3.1)
+  // Therefore we behave conservatively and only proceed if at least one of the
----------------
I feel compelled to say that this is sad. The relevant text for minNum/maxNum even notes, "this means results might differ among implementations", and this is the only place I can find in the standard that says that.


================
Comment at: lib/Analysis/ValueTracking.cpp:3409
@@ +3408,3 @@
+
+  // (icmp X, Y) ? X : Y
+  if (TrueVal == CmpLHS && FalseVal == CmpRHS) {
----------------
This comment should now say icmp/fcmp (or similar).

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2277
@@ -2276,3 +2276,3 @@
     Value *LHS, *RHS;
-    SelectPatternFlavor SPF = matchSelectPattern(const_cast<User*>(&I), LHS, RHS);
+    SelectPatternFlavor SPF = matchSelectPattern(const_cast<User*>(&I), LHS, RHS).Flavor;
     ISD::NodeType Opc = ISD::DELETED_NODE;
----------------
Line is too long.

================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1312
@@ +1311,3 @@
+  //  - but only if this isn't part of a min/max operation, else we'll
+  // ruin min/max canonical form.
+  Value *LHS, *RHS;
----------------
What is the canonical form? I think it would be helpful to say (or refer to somewhere else that says).


Repository:
  rL LLVM

http://reviews.llvm.org/D11146