[PATCH] Move transforms from InstCombine to InstSimplify

[David Majnemer]

On Thu, Jul 11, 2013 at 12:49 AM, Duncan Sands <duncan.sands at gmail.com>wrote:

> Hi David,
>
>
> On 11/07/13 00:26, David Majnemer wrote:
>
>> InstCombine was pattern matching for (icmp (shl 1, X), Cst) to try to
>> transform
>> these icmps to true/false if we could prove that Cst must be in/out of
>> range.
>>
>> However, there are two problems:
>> 1. This sort of thing belongs in InstSimplify
>> 2. This can be generalized further
>>
>> Attached is a patch that moves over these transforms to
>> InstructionSimplify and
>> handles arbitrary constants on the left hand side of the shift and
>> constrains
>> the possible range further by analyzing the nsw/nuw flags on the shift.
>>
>
>  --- lib/Analysis/**InstructionSimplify.cpp        (revision 186043)
>> +++ lib/Analysis/**InstructionSimplify.cpp        (working copy)
>> @@ -1995,6 +1995,27 @@
>>          Lower = IntMin.sdiv(Val);
>>          Upper = IntMax.sdiv(Val) + 1;
>>        }
>> +    } else if (match(LHS, m_Shl(m_ConstantInt(CI2), m_Value()))) {
>> +      const APInt &CI2Value = CI2->getValue();
>> +      if (cast<**OverflowingBinaryOperator>(**LHS)->hasNoUnsignedWrap())
>> {
>> +        // 'shl nuw CI2, x' produces [CI2, CI2 << CLZ(CI2)]
>> +        Lower = CI2Value;
>> +        Upper = (CI2Value << CI2Value.countLeadingZeros()) + 1;
>> +      } else if (cast<**OverflowingBinaryOperator>(**LHS)->hasNoSignedWrap())
>> {
>> +        if (CI2->isNegative()) {
>> +          // 'shl nsw CI2, x' [CI2 << (CLO(CI2)-1), CI2]
>> +          Lower = (CI2Value << (CI2Value.countLeadingOnes() - 1));
>> +          Upper = CI2Value + 1;
>> +        } else {
>> +          // 'shl nsw CI2, x' [CI2, CI2 << (CLZ(CI2)-1)]
>> +          Lower = CI2Value;
>> +          Upper = (CI2Value << (CI2Value.countLeadingZeros() - 1)) + 1;
>> +        }
>>
>
> the cases above look OK, but
>
>  +      } else {
>> +        // 'shl CI2, x' produces [CI2, CI2 << (Width-1)]
>> +        Lower = CI2Value;
>> +        Upper = CI2Value.shl(Width - 1) + 1;
>>
>
> this one seems wrong to me.  Consider for example CI2 (in binary) equal to
>   10001000
> Then Lower=10001000, Upper=00000001, but a left-shift by 1 of CI2 is
> 00010000 which is not in the range Lower .. Upper.
>

Thanks, I've updated the patch so that we will set Lower to zero and Upper
to CI2 << CLZ(CI2) which is conservative but, AFAIK, correct.


>
> Ciao, Duncan.
> ______________________________**_________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/**mailman/listinfo/llvm-commits<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/20130711/ece58bde/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: shfcmpv2.diff
Type: application/octet-stream
Size: 5187 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130711/ece58bde/attachment.obj>