[PATCH] D50081: [ValueTracking] fix maxnum miscompile for cannotBeOrderedLessThanZero (PR37776)

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
spatel added reviewers: t.p.northover, arsenm, craig.topper, scanon.
Herald added subscribers: wdng, mcrosier.

The definition of maxnum may still be up for discussion on llvm-dev, but I think we need to fix this miscompile regardless of that.

This adds the NAN checks suggested by @t.p.northover in PR37776:
https://bugs.llvm.org/show_bug.cgi?id=37776

If both operands to maxnum are NAN, that should get constant folded, so I don't think we have to handle that case.

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  |
     ------------------------


https://reviews.llvm.org/D50081

Files:
  lib/Analysis/ValueTracking.cpp
  test/Transforms/InstSimplify/floating-point-compare.ll


Index: test/Transforms/InstSimplify/floating-point-compare.ll
===================================================================
--- test/Transforms/InstSimplify/floating-point-compare.ll
+++ test/Transforms/InstSimplify/floating-point-compare.ll
@@ -266,13 +266,15 @@
   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)
Index: lib/Analysis/ValueTracking.cpp
===================================================================
--- lib/Analysis/ValueTracking.cpp
+++ lib/Analysis/ValueTracking.cpp
@@ -2817,10 +2817,13 @@
     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) &&


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D50081.158316.patch
Type: text/x-patch
Size: 2046 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180731/c9c49d0b/attachment.bin>