[PATCH:,PR,target/46631] Change operands order so we can use 16bit AND instead of 32bit in thumb2

Hi

An instruction of the following style is 32 bit in thumb2.
    and    r2, r3, r2

If we change the source operands order, it will be 16 bit.
    ands    r2, r2, r3

This patch contains a new peephole2 to detect the situation that the all 3
operands of AND are low registers, and the target is the same as the second
source, then replace it with another AND with its source operands exchanged.

This patch passed the regression test on arm qemu.

thanks
Guozhi


ChangeLog:
2010-11-30  Wei Guozhi  <carrot@google.com>

        PR target/46631
        * config/arm/thumb2.md (new peephole2): New peephole2.


2010-11-30  Wei Guozhi  <carrot@google.com>

        PR target/46631
        * gcc.target/arm/pr46631: New testcase.

On Tue, Nov 30, 2010 at 3:21 PM, Carrot Wei <carrot@google.com> wrote:
> Hi
>
> An instruction of the following style is 32 bit in thumb2.
>    and    r2, r3, r2
>
> If we change the source operands order, it will be 16 bit.
>    ands    r2, r2, r3
>
> This patch contains a new peephole2 to detect the situation that the all 3
> operands of AND are low registers, and the target is the same as the second
> source, then replace it with another AND with its source operands exchanged.


Looking at what thumb1 does:
      if (GET_CODE (operands[2]) != CONST_INT)
        {
          rtx tmp = force_reg (SImode, operands[2]);
          if (rtx_equal_p (operands[0], operands[1]))
            operands[2] = tmp;
          else
            {
              operands[2] = operands[1];
              operands[1] = tmp;
            }
        }
I almost think thumb2 should do the same (maybe it should be done
always in the arm back-end).

Thanks,
Andrew Pinski

On Tue, Nov 30, 2010 at 3:28 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Tue, Nov 30, 2010 at 3:21 PM, Carrot Wei <carrot@google.com> wrote:
>> Hi
>>
>> An instruction of the following style is 32 bit in thumb2.
>>    and    r2, r3, r2
>>
>> If we change the source operands order, it will be 16 bit.
>>    ands    r2, r2, r3
>>
>> This patch contains a new peephole2 to detect the situation that the all 3
>> operands of AND are low registers, and the target is the same as the second
>> source, then replace it with another AND with its source operands exchanged.
>
>
> Looking at what thumb1 does:
>      if (GET_CODE (operands[2]) != CONST_INT)
>        {
>          rtx tmp = force_reg (SImode, operands[2]);
>          if (rtx_equal_p (operands[0], operands[1]))
>            operands[2] = tmp;
>          else
>            {
>              operands[2] = operands[1];
>              operands[1] = tmp;
>            }
>        }
> I almost think thumb2 should do the same (maybe it should be done
> always in the arm back-end).
>
That's too early, all are in pseudo registers. I think it may miss some cases.

thanks
Guozhi

On Tue, Nov 30, 2010 at 4:02 PM, Carrot Wei <carrot@google.com> wrote:
> That's too early, all are in pseudo registers. I think it may miss some cases.

Oh I see it now, I think.  Though it might help in general and make
thumb less generic.

-- Pinski

On Tue, 2010-11-30 at 15:21 -0800, Carrot Wei wrote:
> Hi
> 
> An instruction of the following style is 32 bit in thumb2.
>     and    r2, r3, r2
> 
> If we change the source operands order, it will be 16 bit.
>     ands    r2, r2, r3
> 
> This patch contains a new peephole2 to detect the situation that the all 3
> operands of AND are low registers, and the target is the same as the second
> source, then replace it with another AND with its source operands exchanged.
> 
> This patch passed the regression test on arm qemu.
> 

So gas will already generate a 16-bit instruction from
	ands	r2, r3, r2

So it should be possible to just extend the existing peephole/split to
handle this case (and for all other commutative operations).

R.

> thanks
> Guozhi
> 
> 
> ChangeLog:
> 2010-11-30  Wei Guozhi  <carrot@google.com>
> 
>         PR target/46631
>         * config/arm/thumb2.md (new peephole2): New peephole2.
> 
> 
> 2010-11-30  Wei Guozhi  <carrot@google.com>
> 
>         PR target/46631
>         * gcc.target/arm/pr46631: New testcase.
> 
> 
> Index: thumb2.md
> ===================================================================
> --- thumb2.md	(revision 167257)
> +++ thumb2.md	(working copy)
> @@ -1118,3 +1118,17 @@
>    "
>    operands[2] = GEN_INT (32 - INTVAL (operands[2]));
>    ")
> +
> +(define_peephole2
> +  [(set (match_operand:SI 0 "low_register_operand" "")
> +	(and:SI (match_operand:SI 1 "low_register_operand" "")
> +		(match_dup 0)))]
> +  "TARGET_THUMB2 && peep2_regno_dead_p(0, CC_REGNUM)"
> +  [(parallel
> +    [(set (match_dup 0)
> +	  (and:SI (match_dup 0)
> +		  (match_dup 1)))
> +     (clobber (reg:CC CC_REGNUM))])]
> +  ""
> +)
> +
> 
> 
> Index: pr46631.c
> ===================================================================
> --- pr46631.c	(revision 0)
> +++ pr46631.c	(revision 0)
> @@ -0,0 +1,16 @@
> +/* { dg-options "-mthumb -Os" } */
> +/* { dg-require-effective-target arm_thumb2_ok } */
> +/* { dg-final { scan-assembler "ands" } } */
> +
> +struct S {
> +      int bi_buf;
> +      int bi_valid;
> +};
> +
> +int tz (struct S* p, int bits, int value)
> +{
> +     if (p == 0) return 1;
> +      p->bi_valid = bits;
> +      p->bi_buf = value & ((1 << bits) - 1);
> +      return 0;
> +}