[llvm] r212981 - InstSimplify: Correct sdiv x / -1

[David Majnemer]

I forgot to give thanks to Richard Smith for finding this bug!

On Mon, Jul 14, 2014 at 1:38 PM, David Majnemer
<david.majnemer at gmail.com> wrote:
> Author: majnemer
> Date: Mon Jul 14 15:38:45 2014
> New Revision: 212981
>
> URL: http://llvm.org/viewvc/llvm-project?rev=212981&view=rev
> Log:
> InstSimplify: Correct sdiv x / -1
>
> Determining the bounds of x/ -1 would start off with us dividing it by
> INT_MIN.  Suffice to say, this would not work very well.
>
> Instead, handle it upfront by checking for -1 and mapping it to the
> range: [INT_MIN + 1, INT_MAX.  This means that the result of our
> division can be any value other than INT_MIN.
>
> Modified:
>     llvm/trunk/lib/Analysis/InstructionSimplify.cpp
>     llvm/trunk/test/Transforms/InstSimplify/compare.ll
>
> Modified: llvm/trunk/lib/Analysis/InstructionSimplify.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InstructionSimplify.cpp?rev=212981&r1=212980&r2=212981&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/InstructionSimplify.cpp (original)
> +++ llvm/trunk/lib/Analysis/InstructionSimplify.cpp Mon Jul 14 15:38:45 2014
> @@ -1958,20 +1958,22 @@ static Value *SimplifyICmpInst(unsigned
>          Lower = (-Upper) + 1;
>        }
>      } else if (match(LHS, m_SDiv(m_Value(), m_ConstantInt(CI2)))) {
> -      // 'sdiv x, CI2' produces [INT_MIN / CI2, INT_MAX / CI2].
>        APInt IntMin = APInt::getSignedMinValue(Width);
>        APInt IntMax = APInt::getSignedMaxValue(Width);
> -      APInt Val = CI2->getValue().abs();
> -      if (!Val.isMinValue()) {
> +      APInt Val = CI2->getValue();
> +      if (Val.isAllOnesValue()) {
> +        // 'sdiv x, -1' produces [INT_MIN + 1, INT_MAX]
> +        //    where CI2 != -1 and CI2 != 0 and CI2 != 1
> +        Lower = IntMin + 1;
> +        Upper = IntMax + 1;
> +      } else if (Val.countLeadingZeros() < Width - 1) {
> +        // 'sdiv x, CI2' produces [INT_MIN / CI2, INT_MAX / CI2]
> +        //    where CI2 != -1 and CI2 != 0 and CI2 != 1
>          Lower = IntMin.sdiv(Val);
> -        APInt Rem;
> -        APInt::sdivrem(IntMax, Val, Upper, Rem);
> -        // We may need to round the result of the INT_MAX / CI2 calculation up
> -        // if we see that the division was not exact.
> -        if (Rem.isMinValue())
> -          Upper = Upper + 1;
> -        else
> -          Upper = Upper + 2;
> +        Upper = IntMax.sdiv(Val);
> +        if (Lower.sgt(Upper))
> +          std::swap(Lower, Upper);
> +        Upper = Upper + 1;
>          assert(Upper != Lower && "Upper part of range has wrapped!");
>        }
>      } else if (match(LHS, m_LShr(m_Value(), m_ConstantInt(CI2)))) {
>
> Modified: llvm/trunk/test/Transforms/InstSimplify/compare.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstSimplify/compare.ll?rev=212981&r1=212980&r2=212981&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstSimplify/compare.ll (original)
> +++ llvm/trunk/test/Transforms/InstSimplify/compare.ll Mon Jul 14 15:38:45 2014
> @@ -924,3 +924,14 @@ define i1 @icmp_sdiv_pr20288(i64 %a) {
>  ; CHECK-NEXT: [[CMP:%.*]] = icmp ne i64 [[DIV]], 1073741824
>  ; CHECK-NEXT: ret i1 [[CMP]]
>  }
> +
> +define i1 @icmp_sdiv_neg1(i64 %a) {
> + %div = sdiv i64 %call, -1
> + %cmp = icmp ne i64 %div, 1073741824
> + ret i1 %cmp
> +
> +; CHECK-LABEL: @icmp_sdiv_neg1
> +; CHECK-NEXT: [[DIV:%.*]] = sdiv i64 %a, -1
> +; CHECK-NEXT: [[CMP:%.*]] = icmp ne i64 [[DIV]], 1073741824
> +; CHECK-NEXT: ret i1 [[CMP]]
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits