[PATCH] InstSimplify: Improve handling of ashr/lshr

[David Majnemer]

On Thu, May 15, 2014 at 8:46 PM, Nick Lewycky <nlewycky at google.com> wrote:

> On 14 May 2014 21:47, David Majnemer <david.majnemer at gmail.com> wrote:
>
>> Hi nicholas,
>>
>> Analyze the range of values produced by ashr/lshr cst, %V when it is
>> being used in an icmp.
>>
>> http://reviews.llvm.org/D3774
>>
>> Files:
>>   lib/Analysis/InstructionSimplify.cpp
>>   test/Transforms/InstSimplify/compare.ll
>>
>> Index: lib/Analysis/InstructionSimplify.cpp
>> ===================================================================
>> --- lib/Analysis/InstructionSimplify.cpp
>> +++ lib/Analysis/InstructionSimplify.cpp
>> @@ -2001,7 +2001,7 @@
>>
>>      // Many binary operators with constant RHS have easy to compute
>> constant
>>      // range.  Use them to check whether the comparison is a tautology.
>> -    uint32_t Width = CI->getBitWidth();
>> +    unsigned Width = CI->getBitWidth();
>>      APInt Lower = APInt(Width, 0);
>>      APInt Upper = APInt(Width, 0);
>>      ConstantInt *CI2;
>> @@ -2034,14 +2034,34 @@
>>        APInt NegOne = APInt::getAllOnesValue(Width);
>>        if (CI2->getValue().ult(Width))
>>          Upper = NegOne.lshr(CI2->getValue()) + 1;
>> +    } else if (match(LHS, m_LShr(m_ConstantInt(CI2), m_Value()))) {
>> +      // 'lshr CI2, x' produces [CI2 >> (Width-1), CI2].
>>
>
> Isn't this [0, CI2]? For a given CI2, I can pick an x such that CI2 >> x =
> 0, right?
>

Zero is not always possible. If the MSB is set, a single right shift will
not be able to shift it out.


>
>  +      unsigned ShiftAmount = Width - 1;
>> +      if (!CI2->isZero() && cast<BinaryOperator>(LHS)->isExact())
>> +        ShiftAmount = CI2->getValue().countTrailingZeros();
>> +      Lower = CI2->getValue().lshr(ShiftAmount);
>> +      Upper = CI2->getValue() + 1;
>>
>
> So if I'm thinking correctly, this should be a simple:
>   Upper = CI2->getValue() + 1;
> except for the handling of 'exact' where 'lshr exact CI2, x' produces
> [CI2>>ctz(CI2), CI2]?
>
> I feel like I'm missing something, and the same thing happens in the ashr
> case too.
>
> Nick
>
>
>>      } else if (match(LHS, m_AShr(m_Value(), m_ConstantInt(CI2)))) {
>>        // 'ashr x, CI2' produces [INT_MIN >> CI2, INT_MAX >> CI2].
>>        APInt IntMin = APInt::getSignedMinValue(Width);
>>        APInt IntMax = APInt::getSignedMaxValue(Width);
>>        if (CI2->getValue().ult(Width)) {
>>          Lower = IntMin.ashr(CI2->getValue());
>>          Upper = IntMax.ashr(CI2->getValue()) + 1;
>>        }
>> +    } else if (match(LHS, m_AShr(m_ConstantInt(CI2), m_Value()))) {
>> +      unsigned ShiftAmount = Width - 1;
>> +      if (!CI2->isZero() && cast<BinaryOperator>(LHS)->isExact())
>> +        ShiftAmount = CI2->getValue().countTrailingZeros();
>> +      if (CI2->isNegative()) {
>> +        // 'ashr CI2, x' produces [CI2, CI2 >> (Width-1)]
>> +        Lower = CI2->getValue();
>> +        Upper = CI2->getValue().ashr(ShiftAmount) + 1;
>> +      } else {
>> +        // 'ashr CI2, x' produces [CI2 >> (Width-1), CI2]
>> +        Lower = CI2->getValue().ashr(ShiftAmount);
>> +        Upper = CI2->getValue() + 1;
>> +      }
>>      } else if (match(LHS, m_Or(m_Value(), m_ConstantInt(CI2)))) {
>>        // 'or x, CI2' produces [CI2, UINT_MAX].
>>        Lower = CI2->getValue();
>> Index: test/Transforms/InstSimplify/compare.ll
>> ===================================================================
>> --- test/Transforms/InstSimplify/compare.ll
>> +++ test/Transforms/InstSimplify/compare.ll
>> @@ -817,3 +817,43 @@
>>  ; CHECK-LABEL: @compare_always_false_ne
>>  ; CHECK-NEXT: ret i1 true
>>  }
>> +
>> +define i1 @lshr_ugt_false(i32 %a) {
>> +  %shr = lshr i32 1, %a
>> +  %cmp = icmp ugt i32 %shr, 1
>> +  ret i1 %cmp
>> +; CHECK-LABEL: @lshr_ugt_false
>> +; CHECK-NEXT: ret i1 false
>> +}
>> +
>> +define i1 @exact_lshr_ugt_false(i32 %a) {
>> +  %shr = lshr exact i32 30, %a
>> +  %cmp = icmp ult i32 %shr, 15
>> +  ret i1 %cmp
>> +; CHECK-LABEL: @exact_lshr_ugt_false
>> +; CHECK-NEXT: ret i1 false
>> +}
>> +
>> +define i1 @lshr_sgt_false(i32 %a) {
>> +  %shr = lshr i32 1, %a
>> +  %cmp = icmp sgt i32 %shr, 1
>> +  ret i1 %cmp
>> +; CHECK-LABEL: @lshr_sgt_false
>> +; CHECK-NEXT: ret i1 false
>> +}
>> +
>> +define i1 @ashr_sgt_false(i32 %a) {
>> +  %shr = ashr i32 -30, %a
>> +  %cmp = icmp sgt i32 %shr, -1
>> +  ret i1 %cmp
>> +; CHECK-LABEL: @ashr_sgt_false
>> +; CHECK-NEXT: ret i1 false
>> +}
>> +
>> +define i1 @exact_ashr_sgt_false(i32 %a) {
>> +  %shr = ashr exact i32 -30, %a
>> +  %cmp = icmp sgt i32 %shr, -15
>> +  ret i1 %cmp
>> +; CHECK-LABEL: @exact_ashr_sgt_false
>> +; CHECK-NEXT: ret i1 false
>> +}
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140516/fe5baec3/attachment.html>