[Patch] Teach InstCombine to work with smaller legal types in icmp (shl %v, C1), C2

[Arnaud A. de Grandmaison]

Patch update after discussing with Duncan Sand on irc.

Cheers,
--
Arnaud de Grandmaison
On 02/14/2013 04:05 PM, Arnaud A. de Grandmaison wrote:
> Hi Duncan,
>
> On 02/14/2013 01:58 PM, Duncan Sands wrote:
>> Hi Arnaud,
>>
>> On 14/02/13 11:02, Arnaud A. de Grandmaison wrote:
>>> This patch teaches InstCombine to work with smaller legal types in icmp
>>> (shl %v, C1), C2.
>> I think it best to avoid using target information in the generic IR
>> optimizers
>> if possible.  If you just drop this check, what happens if you
>> truncate to a
>> type that is too small for the target?  I suspect the type legalizer
>> will fix it
>> up in a reasonable way.
>>
> I dropped the target information, and this makes this patch kick in the
> InstCombine/exact.ll testcase : Instcombine will converge to the same
> result, but following different combining steps, so the intermediate
> names had to be changed. Beside, dropping the target information
> simplifies the patch.
>
>>> Using smaller constants is target friendly, at least for those which can
>>> compare to immediate. It also avoids inserting a shift in favor of a
>>> trunc, which can be free on those targets where we are using a legal
>>> type.
>>>
>>> This used to work up to LLVM-3.1, but regressed with the 3.2 release.
>>>
>>> This pattern can occur frequently because of the integer promotion rules
>>> in C/C++ code, which will generate "icmp (sext(trunc %v)), constant".
>>> With 3.2, the sext(trunc) is combined to shifts by visitSExt, before the
>>> icmp combining has a chance to kick in. When it kicks in, it faces a
>>> pattern which is not recognized.
>>>
>>> Any comments or ok to commit ?
>> At a glance it looks like it is only correct for signed comparisons
>> and may
>> give wrong results for unsigned comparisons.
>>
> I do not get this one. The constant I compare to is checked for bits'
> loss, and there is no information loss for the shifted variable ---
> unless I am mistaken.
>
> Thanks for the comments,
> --
> Arnaud
>
>> Ciao, Duncan.
>>
>>> I found it with the following testcase :
>>>
>>> $ cat testcase.c
>>> #include <stdint.h>
>>>
>>> uint16_t
>>> sw_modulo (uint16_t val)
>>> {
>>>    uint16_t start = 0;
>>>    uint16_t end = 349;
>>>    int16_t res = 0;
>>>
>>>    do {
>>>      uint16_t cpt_i = (start + end)/2;
>>>      res = val - cpt_i*36;
>>>
>>>      if (res < 0)
>>>        end = cpt_i - 1;
>>>      else
>>>        start = cpt_i + 1;
>>>
>>>    } while ( (res >= 36) || (res < 0) );
>>>
>>>    return res;
>>> }
>>>
>>>
>>> Without the patch, the X86_64 loop code looks like :
>>> ...
>>> .LBB0_2:                                # %.thread
>>>      addl    $65535, %edx            # imm = 0xFFFF
>>>      movzwl    %dx, %r8d
>>>      .align    16, 0x90
>>> .LBB0_1:                                # %.critedge
>>>      movzwl    %cx, %edx
>>>      addl    %r8d, %edx
>>>      shrl    %edx
>>>      movzwl    %dx, %eax
>>>      imull    $-36, %eax, %eax
>>>      addl    %edi, %eax
>>>      movl    %eax, %esi
>>>      shll    $16, %esi
>>>      testl    %esi, %esi
>>>      js    .LBB0_2
>>>      incl    %edx
>>>      cmpl    $2293760, %esi          # imm = 0x230000
>>>      movw    %dx, %cx
>>>      ja    .LBB0_1
>>> ...
>>>
>>> With the patch:
>>> ...
>>> .LBB0_2:                                # %.thread
>>>      addl    $65535, %edx            # imm = 0xFFFF
>>>      movzwl    %dx, %eax
>>>      .align    16, 0x90
>>> .LBB0_1:                                # %.critedge
>>>      movzwl    %cx, %edx
>>>      addl    %eax, %edx
>>>      shrl    %edx
>>>      imull    $-36, %edx, %esi
>>>      addl    %edi, %esi
>>>      testw    %si, %si
>>>      js    .LBB0_2
>>>      incl    %edx
>>>      cmpw    $35, %si
>>>      movw    %dx, %cx
>>>      ja    .LBB0_1
>>> ...
>>>
>>> Cheers,
>>>
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>


-- 
Arnaud A. de Grandmaison

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Teach-InstCombine-to-work-with-smaller-legal-types-i.patch
Type: text/x-patch
Size: 4322 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130214/3b870971/attachment.bin>