[llvm] 34d35d4 - [ValueTracking] fix miscompile in maxnum case of cannotBeOrderedLessThanZeroImpl (PR46627)
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 14 05:08:30 PDT 2020
Author: Sanjay Patel
Date: 2020-07-14T08:08:09-04:00
New Revision: 34d35d4a42dc747345cea4a8b7066371a70cf7a8
URL: https://github.com/llvm/llvm-project/commit/34d35d4a42dc747345cea4a8b7066371a70cf7a8
DIFF: https://github.com/llvm/llvm-project/commit/34d35d4a42dc747345cea4a8b7066371a70cf7a8.diff
LOG: [ValueTracking] fix miscompile in maxnum case of cannotBeOrderedLessThanZeroImpl (PR46627)
A miscompile with -0.0 is shown in:
http://bugs.llvm.org/PR46627
This is because maxnum(-0.0, +0.0) does not specify a fixed result:
http://llvm.org/docs/LangRef.html#llvm-maxnum-intrinsic
So we need to tighten the constraints for when it is ok to say the
result of maxnum is positive (including +0.0).
Differential Revision: https://reviews.llvm.org/D83601
Added:
Modified:
llvm/lib/Analysis/ValueTracking.cpp
llvm/test/Transforms/InstCombine/copysign.ll
llvm/test/Transforms/InstSimplify/floating-point-arithmetic.ll
llvm/test/Transforms/InstSimplify/floating-point-compare.ll
Removed:
################################################################################
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 614c8bb2f1e6..ffa2037fa10b 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -3368,13 +3368,28 @@ static bool cannotBeOrderedLessThanZeroImpl(const Value *V,
switch (IID) {
default:
break;
- case Intrinsic::maxnum:
- return (isKnownNeverNaN(I->getOperand(0), TLI) &&
- cannotBeOrderedLessThanZeroImpl(I->getOperand(0), TLI,
- SignBitOnly, Depth + 1)) ||
- (isKnownNeverNaN(I->getOperand(1), TLI) &&
- cannotBeOrderedLessThanZeroImpl(I->getOperand(1), TLI,
- SignBitOnly, Depth + 1));
+ case Intrinsic::maxnum: {
+ Value *V0 = I->getOperand(0), *V1 = I->getOperand(1);
+ auto isPositiveNum = [&](Value *V) {
+ if (SignBitOnly) {
+ // With SignBitOnly, this is tricky because the result of
+ // maxnum(+0.0, -0.0) is unspecified. Just check if the operand is
+ // a constant strictly greater than 0.0.
+ const APFloat *C;
+ return match(V, m_APFloat(C)) &&
+ *C > APFloat::getZero(C->getSemantics());
+ }
+
+ // -0.0 compares equal to 0.0, so if this operand is at least -0.0,
+ // maxnum can't be ordered-less-than-zero.
+ return isKnownNeverNaN(V, TLI) &&
+ cannotBeOrderedLessThanZeroImpl(V, TLI, false, Depth + 1);
+ };
+
+ // TODO: This could be improved. We could also check that neither operand
+ // has its sign bit set (and at least 1 is not-NAN?).
+ return isPositiveNum(V0) || isPositiveNum(V1);
+ }
case Intrinsic::maximum:
return cannotBeOrderedLessThanZeroImpl(I->getOperand(0), TLI, SignBitOnly,
diff --git a/llvm/test/Transforms/InstCombine/copysign.ll b/llvm/test/Transforms/InstCombine/copysign.ll
index 2e0063e86356..2d87c7b27166 100644
--- a/llvm/test/Transforms/InstCombine/copysign.ll
+++ b/llvm/test/Transforms/InstCombine/copysign.ll
@@ -64,12 +64,13 @@ define <3 x double> @known_positive_sign_arg_vec(<3 x double> %x, <3 x i32> %y)
ret <3 x double> %r
}
-; FIXME: maxnum(-0.0, 0.0) can return -0.0.
+; maxnum(-0.0, 0.0) can return -0.0.
define float @not_known_positive_sign_arg(float %x, float %y) {
; CHECK-LABEL: @not_known_positive_sign_arg(
-; CHECK-NEXT: [[TMP1:%.*]] = call ninf float @llvm.fabs.f32(float [[Y:%.*]])
-; CHECK-NEXT: ret float [[TMP1]]
+; CHECK-NEXT: [[MAX:%.*]] = call float @llvm.maxnum.f32(float [[X:%.*]], float 0.000000e+00)
+; CHECK-NEXT: [[R:%.*]] = call ninf float @llvm.copysign.f32(float [[Y:%.*]], float [[MAX]])
+; CHECK-NEXT: ret float [[R]]
;
%max = call float @llvm.maxnum.f32(float %x, float 0.0)
%r = call ninf float @llvm.copysign.f32(float %y, float %max)
diff --git a/llvm/test/Transforms/InstSimplify/floating-point-arithmetic.ll b/llvm/test/Transforms/InstSimplify/floating-point-arithmetic.ll
index dca25a3791f5..8b606dca2e21 100644
--- a/llvm/test/Transforms/InstSimplify/floating-point-arithmetic.ll
+++ b/llvm/test/Transforms/InstSimplify/floating-point-arithmetic.ll
@@ -1334,10 +1334,13 @@ define float @fsub_fadd_common_op_wrong_commute_commute(float %x, float %y) {
ret float %r
}
+; PR46627 - https://bugs.llvm.org/show_bug.cgi?id=46627
+
define float @maxnum_with_poszero_op(float %a) {
; CHECK-LABEL: @maxnum_with_poszero_op(
; CHECK-NEXT: [[MAX:%.*]] = call float @llvm.maxnum.f32(float [[A:%.*]], float 0.000000e+00)
-; CHECK-NEXT: ret float [[MAX]]
+; CHECK-NEXT: [[FABS:%.*]] = call float @llvm.fabs.f32(float [[MAX]])
+; CHECK-NEXT: ret float [[FABS]]
;
%max = call float @llvm.maxnum.f32(float %a, float 0.0)
%fabs = call float @llvm.fabs.f32(float %max)
@@ -1348,7 +1351,8 @@ define float @maxnum_with_poszero_op_commute(float %a) {
; CHECK-LABEL: @maxnum_with_poszero_op_commute(
; CHECK-NEXT: [[SQRT:%.*]] = call float @llvm.sqrt.f32(float [[A:%.*]])
; CHECK-NEXT: [[MAX:%.*]] = call float @llvm.maxnum.f32(float 0.000000e+00, float [[SQRT]])
-; CHECK-NEXT: ret float [[MAX]]
+; CHECK-NEXT: [[FABS:%.*]] = call float @llvm.fabs.f32(float [[MAX]])
+; CHECK-NEXT: ret float [[FABS]]
;
%sqrt = call float @llvm.sqrt.f32(float %a)
%max = call float @llvm.maxnum.f32(float 0.0, float %sqrt)
@@ -1361,7 +1365,8 @@ define float @maxnum_with_negzero_op(float %a) {
; CHECK-NEXT: [[NNAN:%.*]] = call nnan float @llvm.sqrt.f32(float [[A:%.*]])
; CHECK-NEXT: [[FABSA:%.*]] = call float @llvm.fabs.f32(float [[NNAN]])
; CHECK-NEXT: [[MAX:%.*]] = call float @llvm.maxnum.f32(float -0.000000e+00, float [[FABSA]])
-; CHECK-NEXT: ret float [[MAX]]
+; CHECK-NEXT: [[FABS:%.*]] = call float @llvm.fabs.f32(float [[MAX]])
+; CHECK-NEXT: ret float [[FABS]]
;
%nnan = call nnan float @llvm.sqrt.f32(float %a)
%fabsa = call float @llvm.fabs.f32(float %nnan)
@@ -1375,7 +1380,8 @@ define float @maxnum_with_negzero_op_commute(float %a) {
; CHECK-NEXT: [[NNAN:%.*]] = call nnan float @llvm.sqrt.f32(float [[A:%.*]])
; CHECK-NEXT: [[FABSA:%.*]] = call float @llvm.fabs.f32(float [[NNAN]])
; CHECK-NEXT: [[MAX:%.*]] = call float @llvm.maxnum.f32(float [[FABSA]], float -0.000000e+00)
-; CHECK-NEXT: ret float [[MAX]]
+; CHECK-NEXT: [[FABS:%.*]] = call float @llvm.fabs.f32(float [[MAX]])
+; CHECK-NEXT: ret float [[FABS]]
;
%nnan = call nnan float @llvm.sqrt.f32(float %a)
%fabsa = call float @llvm.fabs.f32(float %nnan)
@@ -1384,6 +1390,8 @@ define float @maxnum_with_negzero_op_commute(float %a) {
ret float %fabs
}
+; If an operand is strictly greater than 0.0, we know the sign of the result of maxnum.
+
define float @maxnum_with_pos_one_op(float %a) {
; CHECK-LABEL: @maxnum_with_pos_one_op(
; CHECK-NEXT: [[MAX:%.*]] = call float @llvm.maxnum.f32(float [[A:%.*]], float 1.000000e+00)
diff --git a/llvm/test/Transforms/InstSimplify/floating-point-compare.ll b/llvm/test/Transforms/InstSimplify/floating-point-compare.ll
index 6f3f738b6d3e..0cee7b3ce20f 100644
--- a/llvm/test/Transforms/InstSimplify/floating-point-compare.ll
+++ b/llvm/test/Transforms/InstSimplify/floating-point-compare.ll
@@ -212,6 +212,8 @@ define i1 @orderedLessZero_fdiv(float %x) {
ret i1 %uge
}
+; If x == -0.0, maxnum can return -0.0, but that still compares equal to 0.0.
+
define i1 @orderedLessZero_maxnum(float %x) {
; CHECK-LABEL: @orderedLessZero_maxnum(
; CHECK-NEXT: ret i1 true
More information about the llvm-commits
mailing list