From fd0862123da4aba99851e66045c44869ecbcf53d Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Thu, 2 Aug 2018 13:46:20 +0000 Subject: [PATCH] [ValueTracking] fix maxnum miscompile for cannotBeOrderedLessThanZero (PR37776) This adds the NAN checks suggested in PR37776: https://bugs.llvm.org/show_bug.cgi?id=37776 If both operands to maxnum are NAN, that should get constant folded, so we don't have to handle that case. This is the same assumption as other FP ops in this function. Returning 'false' is always conservatively correct. Copying from the bug report: Currently, we have this for "when is cannotBeOrderedLessThanZero (mustBePositiveOrNaN) true for maxnum": L ------------------- | Pos | Neg | NaN | ------------------------ |Pos | x | x | x | ------------------------ R |Neg | x | | x | ------------------------ |NaN | x | x | x | ------------------------ The cases with (Neg & NaN) are wrong. We should have: L ------------------- | Pos | Neg | NaN | ------------------------ |Pos | x | x | x | ------------------------ R |Neg | x | | | ------------------------ |NaN | x | | x | ------------------------ Differential Revision: https://reviews.llvm.org/D50081 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@338716 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/ValueTracking.cpp | 11 +++++++---- test/Transforms/InstSimplify/floating-point-compare.ll | 6 ++++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/Analysis/ValueTracking.cpp b/lib/Analysis/ValueTracking.cpp index 0ef39163bda..edd46c5fe36 100644 --- a/lib/Analysis/ValueTracking.cpp +++ b/lib/Analysis/ValueTracking.cpp @@ -2817,10 +2817,13 @@ static bool cannotBeOrderedLessThanZeroImpl(const Value *V, default: break; case Intrinsic::maxnum: - return cannotBeOrderedLessThanZeroImpl(I->getOperand(0), TLI, SignBitOnly, - Depth + 1) || - cannotBeOrderedLessThanZeroImpl(I->getOperand(1), TLI, SignBitOnly, - Depth + 1); + return (isKnownNeverNaN(I->getOperand(0)) && + cannotBeOrderedLessThanZeroImpl(I->getOperand(0), TLI, + SignBitOnly, Depth + 1)) || + (isKnownNeverNaN(I->getOperand(1)) && + cannotBeOrderedLessThanZeroImpl(I->getOperand(1), TLI, + SignBitOnly, Depth + 1)); + case Intrinsic::minnum: return cannotBeOrderedLessThanZeroImpl(I->getOperand(0), TLI, SignBitOnly, Depth + 1) && diff --git a/test/Transforms/InstSimplify/floating-point-compare.ll b/test/Transforms/InstSimplify/floating-point-compare.ll index b1444bb5a1e..bc5c58a698e 100644 --- a/test/Transforms/InstSimplify/floating-point-compare.ll +++ b/test/Transforms/InstSimplify/floating-point-compare.ll @@ -266,13 +266,15 @@ define i1 @orderedLessZeroMinNum(float, float) { ret i1 %uge } -; FIXME: This is wrong. ; PR37776: https://bugs.llvm.org/show_bug.cgi?id=37776 ; exp() may return nan, leaving %1 as the unknown result, so we can't simplify. define i1 @orderedLessZeroMaxNum(float, float) { ; CHECK-LABEL: @orderedLessZeroMaxNum( -; CHECK-NEXT: ret i1 true +; CHECK-NEXT: [[A:%.*]] = call float @llvm.exp.f32(float [[TMP0:%.*]]) +; CHECK-NEXT: [[B:%.*]] = call float @llvm.maxnum.f32(float [[A]], float [[TMP1:%.*]]) +; CHECK-NEXT: [[UGE:%.*]] = fcmp uge float [[B]], 0.000000e+00 +; CHECK-NEXT: ret i1 [[UGE]] ; %a = call float @llvm.exp.f32(float %0) %b = call float @llvm.maxnum.f32(float %a, float %1) -- 2.11.0