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

[Duncan Sands]

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.

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

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
>