[llvm] r338716 - [ValueTracking] fix maxnum miscompile for cannotBeOrderedLessThanZero (PR37776)

[Hans Wennborg via llvm-commits]

Merged to 7.0 in r339234.

On Thu, Aug 2, 2018 at 3:46 PM, Sanjay Patel via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: spatel
> Date: Thu Aug  2 06:46:20 2018
> New Revision: 338716
>
> URL: http://llvm.org/viewvc/llvm-project?rev=338716&view=rev
> Log:
> [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
>
>
> Modified:
>     llvm/trunk/lib/Analysis/ValueTracking.cpp
>     llvm/trunk/test/Transforms/InstSimplify/floating-point-compare.ll
>
> Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=338716&r1=338715&r2=338716&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)
> +++ llvm/trunk/lib/Analysis/ValueTracking.cpp Thu Aug  2 06:46:20 2018
> @@ -2817,10 +2817,13 @@ static bool cannotBeOrderedLessThanZeroI
>      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) &&
>
> Modified: llvm/trunk/test/Transforms/InstSimplify/floating-point-compare.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstSimplify/floating-point-compare.ll?rev=338716&r1=338715&r2=338716&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstSimplify/floating-point-compare.ll (original)
> +++ llvm/trunk/test/Transforms/InstSimplify/floating-point-compare.ll Thu Aug  2 06:46:20 2018
> @@ -266,13 +266,15 @@ define i1 @orderedLessZeroMinNum(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)
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits