[PATCH] InstSimplify: Improve handling of ashr/lshr

[Nick Lewycky]

On 16 May 2014 01:42, David Majnemer <david.majnemer at gmail.com> wrote:

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

Aha! That's what I was missing.

 +      unsigned ShiftAmount = Width - 1;
>>> +      if (!CI2->isZero() && cast<BinaryOperator>(LHS)->isExact())
>>> +        ShiftAmount = CI2->getValue().countTrailingZeros();
>>> +      Lower = CI2->getValue().lshr(ShiftAmount);
>>>
>>
Just to check my understanding, Lower = 0 is CI2 is positive, 1 if CI2 is
negative. Might be worth a comment.

 +      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.
>>
>
LGTM

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/5806d101/attachment.html>