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

[Arnaud A. de Grandmaison]

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: 4222 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130214/93c14466/attachment.bin>