[PATCH] InstCombine: fix fold "fcmp x, undef" to account for NaN

[David Majnemer]

================
Comment at: lib/Analysis/InstructionSimplify.cpp:3044
@@ +3043,3 @@
+  // fcmp pred x, undef
+  if (isa<UndefValue>(RHS) || isa<UndefValue>(LHS)) {
+    CmpInst::Predicate Predicate = CmpInst::Predicate(Pred);
----------------
It would be nice to have `LHS` on the left and `RHS` on the right.

I think the comment would be clearer if it mentioned that this simplification will effect `fcmp pred undef, x` as well.

================
Comment at: lib/Analysis/InstructionSimplify.cpp:3045
@@ +3044,3 @@
+  if (isa<UndefValue>(RHS) || isa<UndefValue>(LHS)) {
+    CmpInst::Predicate Predicate = CmpInst::Predicate(Pred);
+    // Choosing NaN for the undef will always make unordered comparison succeed
----------------
Isn't `Pred` already a `CmpInst::Predicate`? What does this line achieve?

================
Comment at: lib/Analysis/InstructionSimplify.cpp:3047
@@ +3046,3 @@
+    // Choosing NaN for the undef will always make unordered comparison succeed
+    // and ordered comparison fails.
+    return ConstantInt::get(GetCompareTy(LHS), CmpInst::isUnordered(Predicate));
----------------
s/fails/fail/

================
Comment at: lib/IR/ConstantFold.cpp:1330
@@ -1329,3 +1329,3 @@
     if (!isa<ConstantExpr>(V2)) {
-      // We distilled thisUse the standard constant folder for a few cases
+      // We distilled this down to a simple case, use standard constant folder.
       ConstantInt *R = nullptr;
----------------
perhaps "use the standard constant folder."

================
Comment at: lib/IR/ConstantFold.cpp:1673-1675
@@ -1673,1 +1672,5 @@
+    if (ICmpInst::isEquality(Predicate) ||
+        (ICmpInst::isIntPredicate(Predicate) && isa<UndefValue>(C1) &&
+         isa<UndefValue>(C2)))
       return UndefValue::get(ResultTy);
+
----------------
Either move the `ICmpInst::isIntPredicate(Predicate) && isa<UndefValue>(C1) && isa<UndefValue>(C2)` check out of this block or simplify it with `ICmpInst::isIntPredicate(Predicate) && C1 == C2`, the line is getting a bit too long.

http://reviews.llvm.org/D7617

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