Skip to content

Commit 34d35d4

Browse files
committed
[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
1 parent 9cc669d commit 34d35d4

File tree

4 files changed

+40
-14
lines changed

4 files changed

+40
-14
lines changed

llvm/lib/Analysis/ValueTracking.cpp

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3368,13 +3368,28 @@ static bool cannotBeOrderedLessThanZeroImpl(const Value *V,
33683368
switch (IID) {
33693369
default:
33703370
break;
3371-
case Intrinsic::maxnum:
3372-
return (isKnownNeverNaN(I->getOperand(0), TLI) &&
3373-
cannotBeOrderedLessThanZeroImpl(I->getOperand(0), TLI,
3374-
SignBitOnly, Depth + 1)) ||
3375-
(isKnownNeverNaN(I->getOperand(1), TLI) &&
3376-
cannotBeOrderedLessThanZeroImpl(I->getOperand(1), TLI,
3377-
SignBitOnly, Depth + 1));
3371+
case Intrinsic::maxnum: {
3372+
Value *V0 = I->getOperand(0), *V1 = I->getOperand(1);
3373+
auto isPositiveNum = [&](Value *V) {
3374+
if (SignBitOnly) {
3375+
// With SignBitOnly, this is tricky because the result of
3376+
// maxnum(+0.0, -0.0) is unspecified. Just check if the operand is
3377+
// a constant strictly greater than 0.0.
3378+
const APFloat *C;
3379+
return match(V, m_APFloat(C)) &&
3380+
*C > APFloat::getZero(C->getSemantics());
3381+
}
3382+
3383+
// -0.0 compares equal to 0.0, so if this operand is at least -0.0,
3384+
// maxnum can't be ordered-less-than-zero.
3385+
return isKnownNeverNaN(V, TLI) &&
3386+
cannotBeOrderedLessThanZeroImpl(V, TLI, false, Depth + 1);
3387+
};
3388+
3389+
// TODO: This could be improved. We could also check that neither operand
3390+
// has its sign bit set (and at least 1 is not-NAN?).
3391+
return isPositiveNum(V0) || isPositiveNum(V1);
3392+
}
33783393

33793394
case Intrinsic::maximum:
33803395
return cannotBeOrderedLessThanZeroImpl(I->getOperand(0), TLI, SignBitOnly,

llvm/test/Transforms/InstCombine/copysign.ll

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,13 @@ define <3 x double> @known_positive_sign_arg_vec(<3 x double> %x, <3 x i32> %y)
6464
ret <3 x double> %r
6565
}
6666

67-
; FIXME: maxnum(-0.0, 0.0) can return -0.0.
67+
; maxnum(-0.0, 0.0) can return -0.0.
6868

6969
define float @not_known_positive_sign_arg(float %x, float %y) {
7070
; CHECK-LABEL: @not_known_positive_sign_arg(
71-
; CHECK-NEXT: [[TMP1:%.*]] = call ninf float @llvm.fabs.f32(float [[Y:%.*]])
72-
; CHECK-NEXT: ret float [[TMP1]]
71+
; CHECK-NEXT: [[MAX:%.*]] = call float @llvm.maxnum.f32(float [[X:%.*]], float 0.000000e+00)
72+
; CHECK-NEXT: [[R:%.*]] = call ninf float @llvm.copysign.f32(float [[Y:%.*]], float [[MAX]])
73+
; CHECK-NEXT: ret float [[R]]
7374
;
7475
%max = call float @llvm.maxnum.f32(float %x, float 0.0)
7576
%r = call ninf float @llvm.copysign.f32(float %y, float %max)

llvm/test/Transforms/InstSimplify/floating-point-arithmetic.ll

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1334,10 +1334,13 @@ define float @fsub_fadd_common_op_wrong_commute_commute(float %x, float %y) {
13341334
ret float %r
13351335
}
13361336

1337+
; PR46627 - https://bugs.llvm.org/show_bug.cgi?id=46627
1338+
13371339
define float @maxnum_with_poszero_op(float %a) {
13381340
; CHECK-LABEL: @maxnum_with_poszero_op(
13391341
; CHECK-NEXT: [[MAX:%.*]] = call float @llvm.maxnum.f32(float [[A:%.*]], float 0.000000e+00)
1340-
; CHECK-NEXT: ret float [[MAX]]
1342+
; CHECK-NEXT: [[FABS:%.*]] = call float @llvm.fabs.f32(float [[MAX]])
1343+
; CHECK-NEXT: ret float [[FABS]]
13411344
;
13421345
%max = call float @llvm.maxnum.f32(float %a, float 0.0)
13431346
%fabs = call float @llvm.fabs.f32(float %max)
@@ -1348,7 +1351,8 @@ define float @maxnum_with_poszero_op_commute(float %a) {
13481351
; CHECK-LABEL: @maxnum_with_poszero_op_commute(
13491352
; CHECK-NEXT: [[SQRT:%.*]] = call float @llvm.sqrt.f32(float [[A:%.*]])
13501353
; CHECK-NEXT: [[MAX:%.*]] = call float @llvm.maxnum.f32(float 0.000000e+00, float [[SQRT]])
1351-
; CHECK-NEXT: ret float [[MAX]]
1354+
; CHECK-NEXT: [[FABS:%.*]] = call float @llvm.fabs.f32(float [[MAX]])
1355+
; CHECK-NEXT: ret float [[FABS]]
13521356
;
13531357
%sqrt = call float @llvm.sqrt.f32(float %a)
13541358
%max = call float @llvm.maxnum.f32(float 0.0, float %sqrt)
@@ -1361,7 +1365,8 @@ define float @maxnum_with_negzero_op(float %a) {
13611365
; CHECK-NEXT: [[NNAN:%.*]] = call nnan float @llvm.sqrt.f32(float [[A:%.*]])
13621366
; CHECK-NEXT: [[FABSA:%.*]] = call float @llvm.fabs.f32(float [[NNAN]])
13631367
; CHECK-NEXT: [[MAX:%.*]] = call float @llvm.maxnum.f32(float -0.000000e+00, float [[FABSA]])
1364-
; CHECK-NEXT: ret float [[MAX]]
1368+
; CHECK-NEXT: [[FABS:%.*]] = call float @llvm.fabs.f32(float [[MAX]])
1369+
; CHECK-NEXT: ret float [[FABS]]
13651370
;
13661371
%nnan = call nnan float @llvm.sqrt.f32(float %a)
13671372
%fabsa = call float @llvm.fabs.f32(float %nnan)
@@ -1375,7 +1380,8 @@ define float @maxnum_with_negzero_op_commute(float %a) {
13751380
; CHECK-NEXT: [[NNAN:%.*]] = call nnan float @llvm.sqrt.f32(float [[A:%.*]])
13761381
; CHECK-NEXT: [[FABSA:%.*]] = call float @llvm.fabs.f32(float [[NNAN]])
13771382
; CHECK-NEXT: [[MAX:%.*]] = call float @llvm.maxnum.f32(float [[FABSA]], float -0.000000e+00)
1378-
; CHECK-NEXT: ret float [[MAX]]
1383+
; CHECK-NEXT: [[FABS:%.*]] = call float @llvm.fabs.f32(float [[MAX]])
1384+
; CHECK-NEXT: ret float [[FABS]]
13791385
;
13801386
%nnan = call nnan float @llvm.sqrt.f32(float %a)
13811387
%fabsa = call float @llvm.fabs.f32(float %nnan)
@@ -1384,6 +1390,8 @@ define float @maxnum_with_negzero_op_commute(float %a) {
13841390
ret float %fabs
13851391
}
13861392

1393+
; If an operand is strictly greater than 0.0, we know the sign of the result of maxnum.
1394+
13871395
define float @maxnum_with_pos_one_op(float %a) {
13881396
; CHECK-LABEL: @maxnum_with_pos_one_op(
13891397
; CHECK-NEXT: [[MAX:%.*]] = call float @llvm.maxnum.f32(float [[A:%.*]], float 1.000000e+00)

llvm/test/Transforms/InstSimplify/floating-point-compare.ll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,8 @@ define i1 @orderedLessZero_fdiv(float %x) {
212212
ret i1 %uge
213213
}
214214

215+
; If x == -0.0, maxnum can return -0.0, but that still compares equal to 0.0.
216+
215217
define i1 @orderedLessZero_maxnum(float %x) {
216218
; CHECK-LABEL: @orderedLessZero_maxnum(
217219
; CHECK-NEXT: ret i1 true

0 commit comments

Comments
 (0)