From 4c8f5afd99a18ab81efe12b1fb26b150728a332d Mon Sep 17 00:00:00 2001 From: Mehdi Amini Date: Mon, 9 Mar 2015 03:20:25 +0000 Subject: [PATCH] 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 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@231626 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/Instructions.h | 10 +++++--- lib/Analysis/InstructionSimplify.cpp | 9 +++++-- lib/IR/ConstantFold.cpp | 26 ++++++++++++++------- test/Transforms/InstCombine/fcmp.ll | 35 ++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 13 deletions(-) diff --git a/include/llvm/IR/Instructions.h b/include/llvm/IR/Instructions.h index 1a7767523c4..a737e563407 100644 --- 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 0cb0982d311..d90f14a13fa 100644 --- 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(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(LHS) || isa(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 a915d285b14..cf7f3705fb7 100644 --- a/lib/IR/ConstantFold.cpp +++ b/lib/IR/ConstantFold.cpp @@ -1327,7 +1327,7 @@ static FCmpInst::Predicate evaluateFCmpRelation(Constant *V1, Constant *V2) { if (!isa(V1)) { if (!isa(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( ConstantExpr::getFCmp(FCmpInst::FCMP_OEQ, V1, V2)); @@ -1665,15 +1665,22 @@ Constant *llvm::ConstantFoldCompareInstruction(unsigned short pred, // Handle some degenerate cases first if (isa(C1) || isa(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(C1) && isa(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(C1) || isa(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 ee39d1084eb..7fd46f22818 100644 --- 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 +} -- 2.34.1