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

Summary:
See the two test cases.

; Can fold fcmp with undef on one side by choosing NaN for the undef

; Can fold fcmp with undef on both side
;   fcmp u_pred undef, undef -> true
;   fcmp o_pred undef, undef -> false
; because whatever you choose for the first undef
; you can choose NaN for the other undef

Reviewers: hfinkel, chandlerc, majnemer

Reviewed By: majnemer

Subscribers: majnemer, llvm-commits

Differential Revision: http://reviews.llvm.org/D7617

From: Mehdi Amini <mehdi.amini@apple.com>

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@231626 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/IR/Instructions.h b/include/llvm/IR/Instructions.h
index 1a7767523c443be351bc6caababeb10d94ebacf7..a737e5634076bf07d1b9fd7fe5fcae37963e5a94 100644 (file)
--- a/include/llvm/IR/Instructions.h
+++ b/include/llvm/IR/Instructions.h
@@ -1201,11 +1201,15 @@ public:
 
   /// @returns true if the predicate of this instruction is EQ or NE.
   /// \brief Determine if this is an equality predicate.
-  bool isEquality() const {
-    return getPredicate() == FCMP_OEQ || getPredicate() == FCMP_ONE ||
-           getPredicate() == FCMP_UEQ || getPredicate() == FCMP_UNE;
+  static bool isEquality(Predicate Pred) {
+    return Pred == FCMP_OEQ || Pred == FCMP_ONE || Pred == FCMP_UEQ ||
+           Pred == FCMP_UNE;
   }
 
+  /// @returns true if the predicate of this instruction is EQ or NE.
+  /// \brief Determine if this is an equality predicate.
+  bool isEquality() const { return isEquality(getPredicate()); }
+
   /// @returns true if the predicate of this instruction is commutative.
   /// \brief Determine if this is a commutative predicate.
   bool isCommutative() const {

diff --git a/lib/Analysis/InstructionSimplify.cpp b/lib/Analysis/InstructionSimplify.cpp
index 0cb0982d3114df872ef81d25dbd42012b11097ff..d90f14a13fa7c047d2c99ffc660507f74a02a276 100644 (file)
--- a/lib/Analysis/InstructionSimplify.cpp
+++ b/lib/Analysis/InstructionSimplify.cpp
@@ -3054,8 +3054,13 @@ static Value *SimplifyFCmpInst(unsigned Predicate, Value *LHS, Value *RHS,
   if (Pred == FCmpInst::FCMP_TRUE)
     return ConstantInt::get(GetCompareTy(LHS), 1);
 
-  if (isa<UndefValue>(RHS))                  // fcmp pred X, undef -> undef
-    return UndefValue::get(GetCompareTy(LHS));
+  // fcmp pred x, undef  and  fcmp pred undef, x
+  // fold to true if unordered, false if ordered
+  if (isa<UndefValue>(LHS) || isa<UndefValue>(RHS)) {
+    // Choosing NaN for the undef will always make unordered comparison succeed
+    // and ordered comparison fail.
+    return ConstantInt::get(GetCompareTy(LHS), CmpInst::isUnordered(Pred));
+  }
 
   // fcmp x,x -> true/false.  Not all compares are foldable.
   if (LHS == RHS) {

diff --git a/lib/IR/ConstantFold.cpp b/lib/IR/ConstantFold.cpp
index a915d285b14a1a743493d6c4dd1d6c602be5a727..cf7f3705fb777b7e1a9e7dad88dfea34aaa2e1b1 100644 (file)
--- a/lib/IR/ConstantFold.cpp
+++ b/lib/IR/ConstantFold.cpp
@@ -1327,7 +1327,7 @@ static FCmpInst::Predicate evaluateFCmpRelation(Constant *V1, Constant *V2) {
 
   if (!isa<ConstantExpr>(V1)) {
     if (!isa<ConstantExpr>(V2)) {
-      // We distilled thisUse the standard constant folder for a few cases
+      // Simple case, use the standard constant folder.
       ConstantInt *R = nullptr;
       R = dyn_cast<ConstantInt>(
                       ConstantExpr::getFCmp(FCmpInst::FCMP_OEQ, V1, V2));
@@ -1665,15 +1665,22 @@ Constant *llvm::ConstantFoldCompareInstruction(unsigned short pred,
 
   // Handle some degenerate cases first
   if (isa<UndefValue>(C1) || isa<UndefValue>(C2)) {
+    CmpInst::Predicate Predicate = CmpInst::Predicate(pred);
+    bool isIntegerPredicate = ICmpInst::isIntPredicate(Predicate);
     // For EQ and NE, we can always pick a value for the undef to make the
     // predicate pass or fail, so we can return undef.
-    // Also, if both operands are undef, we can return undef.
-    if (ICmpInst::isEquality(ICmpInst::Predicate(pred)) ||
-        (isa<UndefValue>(C1) && isa<UndefValue>(C2)))
+    // Also, if both operands are undef, we can return undef for int comparison.
+    if (ICmpInst::isEquality(Predicate) || (isIntegerPredicate && C1 == C2))
       return UndefValue::get(ResultTy);
-    // Otherwise, pick the same value as the non-undef operand, and fold
-    // it to true or false.
-    return ConstantInt::get(ResultTy, CmpInst::isTrueWhenEqual(pred));
+
+    // Otherwise, for integer compare, pick the same value as the non-undef
+    // operand, and fold it to true or false.
+    if (isIntegerPredicate)
+      return ConstantInt::get(ResultTy, CmpInst::isTrueWhenEqual(pred));
+
+    // Choosing NaN for the undef will always make unordered comparison succeed
+    // and ordered comparison fails.
+    return ConstantInt::get(ResultTy, CmpInst::isUnordered(Predicate));
   }
 
   // icmp eq/ne(null,GV) -> false/true
@@ -1789,7 +1796,10 @@ Constant *llvm::ConstantFoldCompareInstruction(unsigned short pred,
     return ConstantVector::get(ResElts);
   }
 
-  if (C1->getType()->isFloatingPointTy()) {
+  if (C1->getType()->isFloatingPointTy() &&
+      // Only call evaluateFCmpRelation if we have a constant expr to avoid
+      // infinite recursive loop
+      (isa<ConstantExpr>(C1) || isa<ConstantExpr>(C2))) {
     int Result = -1;  // -1 = unknown, 0 = known false, 1 = known true.
     switch (evaluateFCmpRelation(C1, C2)) {
     default: llvm_unreachable("Unknown relation!");

diff --git a/test/Transforms/InstCombine/fcmp.ll b/test/Transforms/InstCombine/fcmp.ll
index ee39d1084ebcbe616de2a8d037f2d770729d5445..7fd46f2281832ce13876c4fe7b6eabd4eea6d33e 100644 (file)
--- a/test/Transforms/InstCombine/fcmp.ll
+++ b/test/Transforms/InstCombine/fcmp.ll
@@ -240,3 +240,38 @@ define i32 @test17(double %a, double (double)* %p) nounwind {
   %conv = zext i1 %cmp to i32
   ret i32 %conv
 }
+
+; Can fold fcmp with undef on one side by choosing NaN for the undef
+define i32 @test18_undef_unordered(float %a) nounwind {
+; CHECK-LABEL: @test18_undef_unordered
+; CHECK: ret i32 1
+  %cmp = fcmp ueq float %a, undef
+  %conv = zext i1 %cmp to i32
+  ret i32 %conv
+}
+; Can fold fcmp with undef on one side by choosing NaN for the undef
+define i32 @test18_undef_ordered(float %a) nounwind {
+; CHECK-LABEL: @test18_undef_ordered
+; CHECK: ret i32 0
+  %cmp = fcmp oeq float %a, undef
+  %conv = zext i1 %cmp to i32
+  ret i32 %conv
+}
+
+; Can fold fcmp with undef on both side
+;   fcmp u_pred undef, undef -> true
+;   fcmp o_pred undef, undef -> false
+; because whatever you choose for the first undef
+; you can choose NaN for the other undef
+define i1 @test19_undef_unordered() nounwind {
+; CHECK-LABEL: @test19_undef
+; CHECK: ret i1 true
+  %cmp = fcmp ueq float undef, undef
+  ret i1 %cmp
+}
+define i1 @test19_undef_ordered() nounwind {
+; CHECK-LABEL: @test19_undef
+; CHECK: ret i1 false
+  %cmp = fcmp oeq float undef, undef
+  ret i1 %cmp
+}