Patchwork [PATCH:,PR,target/44999] Replace "and r0, r0, #255" with uxtb in thumb2

login
register
mail settings
Submitter Carrot Wei
Date Nov. 19, 2010, 2:40 a.m.
Message ID <AANLkTin8nW+gX4m39uWGWsn6yEqdVhvnCikAzGD9B9Gg@mail.gmail.com>
Download mbox | patch
Permalink /patch/72186/
State New
Headers show

Comments

Carrot Wei - Nov. 19, 2010, 2:40 a.m.
Hi

This patch reverts my last patch and implements the enhancement as a
new split rule. Additionally it modifies arm_class_likely_spilled_p so
that thumb2 registers are not likely spilled.

Testing has been run on qemu without regression.

thanks
Guozhi

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

        PR target/44999
        * config/arm/arm.md (andsi3): Revert it.
        * config/arm/thumb2.md (thumb2_zero_extendqisi2_v6): Revert it.
        (split andsi3): New split to convert and with 0xFF to uxtb.
        * config/arm/arm.c (arm_class_likely_spilled_p): Remove thumb2 from the
        likely spill cases.

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

        PR target/44999
        * gcc.target/arm/pr44999.c: Update it to more general.




On Mon, Nov 15, 2010 at 6:07 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 11/16/2010 02:29 AM, Carrot Wei wrote:
>>
>> The compiler always generates "and" instruction for both arm and thumb
>> modes. But the enhanced expander can still handle this case.
>
> If anything, a splitter seems preferrable to hacking the expander. Within an
> expander you cannot be sure that passes such as combine won't undo your
> change.
>
> Paolo
>
Richard Earnshaw - Dec. 2, 2010, 12:19 a.m.
Please don't try to mix patches for different things. Firstly it makes reviewing harder.  Secondly if there's a problem found after the patch is committed the all the changes may get revered rather than just the change that caused the problem. If one part of a patch is dependent on a separate change then please say so in your submission. 

Now on to the details. 

Uxtb and uxth may generate smaller code, but on cortex-a9 they take longer to execute than AND.  So we most likely only want to do this if either optimising for size or if we can't hoist a constant out of a loop.

What's the justification for the class_likely_spilled change?  I can't immediately see why this constraint is any less likely to be true on thumb2 than on thumb1, since the contents of the class is the same (getting this wrong can cause ICEs). 

R. 





On 19 Nov 2010, at 02:40, "Carrot Wei" <carrot@google.com> wrote:

> Hi
> 
> This patch reverts my last patch and implements the enhancement as a
> new split rule. Additionally it modifies arm_class_likely_spilled_p so
> that thumb2 registers are not likely spilled.
> 
> Testing has been run on qemu without regression.
> 
> thanks
> Guozhi
> 
> ChangeLog:
> 2010-11-18  Wei Guozhi  <carrot@google.com>
> 
>        PR target/44999
>        * config/arm/arm.md (andsi3): Revert it.
>        * config/arm/thumb2.md (thumb2_zero_extendqisi2_v6): Revert it.
>        (split andsi3): New split to convert and with 0xFF to uxtb.
>        * config/arm/arm.c (arm_class_likely_spilled_p): Remove thumb2 from the
>        likely spill cases.
> 
> ChangeLog:
> 2010-11-18  Wei Guozhi  <carrot@google.com>
> 
>        PR target/44999
>        * gcc.target/arm/pr44999.c: Update it to more general.
> 
> 
> Index: thumb2.md
> ===================================================================
> --- thumb2.md    (revision 165462)
> +++ thumb2.md    (working copy)
> @@ -585,7 +585,7 @@
>    (set_attr "neg_pool_range" "*,250")]
> )
> 
> -(define_insn "thumb2_zero_extendqisi2_v6"
> +(define_insn "*thumb2_zero_extendqisi2_v6"
>   [(set (match_operand:SI 0 "s_register_operand" "=r,r")
>    (zero_extend:SI (match_operand:QI 1 "nonimmediate_operand" "r,m")))]
>   "TARGET_THUMB2 && arm_arch6"
> @@ -1118,3 +1118,16 @@
>   "
>   operands[2] = GEN_INT (32 - INTVAL (operands[2]));
>   ")
> +
> +(define_split
> +  [(set (match_operand:SI 0 "s_register_operand" "")
> +    (and:SI (match_operand:SI 1 "s_register_operand" "")
> +        (match_operand:SI 2 "const_int_operand" "")))]
> +  "TARGET_THUMB2 && arm_arch6 && INTVAL (operands[2]) == 255"
> +  [(set (match_dup 0)
> +    (zero_extend:SI (match_dup 1)))]
> +  "
> +  operands[1] = convert_to_mode (QImode, operands[1], 1);
> +  "
> +)
> +
> Index: arm.c
> ===================================================================
> --- arm.c    (revision 165462)
> +++ arm.c    (working copy)
> @@ -22046,7 +22046,7 @@ arm_preferred_simd_mode (enum machine_mo
> static bool
> arm_class_likely_spilled_p (reg_class_t rclass)
> {
> -  if ((TARGET_THUMB && rclass == LO_REGS)
> +  if ((TARGET_THUMB1 && rclass == LO_REGS)
>       || rclass  == CC_REG)
>     return true;
> 
> Index: arm.md
> ===================================================================
> --- arm.md    (revision 165462)
> +++ arm.md    (working copy)
> @@ -2015,17 +2015,9 @@
>     {
>       if (GET_CODE (operands[2]) == CONST_INT)
>         {
> -      if (INTVAL (operands[2]) == 255 && arm_arch6)
> -        {
> -          operands[1] = convert_to_mode (QImode, operands[1], 1);
> -          emit_insn (gen_thumb2_zero_extendqisi2_v6 (operands[0],
> -                             operands[1]));
> -        }
> -      else
> -        arm_split_constant (AND, SImode, NULL_RTX,
> -                INTVAL (operands[2]), operands[0],
> -                operands[1],
> -                optimize && can_create_pseudo_p ());
> +      arm_split_constant (AND, SImode, NULL_RTX,
> +                  INTVAL (operands[2]), operands[0],
> +                  operands[1], optimize && can_create_pseudo_p ());
> 
>           DONE;
>         }
> 
> Index: pr44999.c
> ===================================================================
> --- pr44999.c    (revision 165462)
> +++ pr44999.c    (working copy)
> @@ -5,5 +5,5 @@
> 
> int tp(int x, int y)
> {
> -  return (x & 0xff) - (y & 0xffff);
> +  return ((x+3) & 0xff) - (y & 0xffff);
> }
> 
> 
> On Mon, Nov 15, 2010 at 6:07 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
>> On 11/16/2010 02:29 AM, Carrot Wei wrote:
>>> 
>>> The compiler always generates "and" instruction for both arm and thumb
>>> modes. But the enhanced expander can still handle this case.
>> 
>> If anything, a splitter seems preferrable to hacking the expander. Within an
>> expander you cannot be sure that passes such as combine won't undo your
>> change.
>> 
>> Paolo
>>

Patch

Index: thumb2.md
===================================================================
--- thumb2.md	(revision 165462)
+++ thumb2.md	(working copy)
@@ -585,7 +585,7 @@ 
    (set_attr "neg_pool_range" "*,250")]
 )

-(define_insn "thumb2_zero_extendqisi2_v6"
+(define_insn "*thumb2_zero_extendqisi2_v6"
   [(set (match_operand:SI 0 "s_register_operand" "=r,r")
 	(zero_extend:SI (match_operand:QI 1 "nonimmediate_operand" "r,m")))]
   "TARGET_THUMB2 && arm_arch6"
@@ -1118,3 +1118,16 @@ 
   "
   operands[2] = GEN_INT (32 - INTVAL (operands[2]));
   ")
+
+(define_split
+  [(set (match_operand:SI 0 "s_register_operand" "")
+	(and:SI (match_operand:SI 1 "s_register_operand" "")
+		(match_operand:SI 2 "const_int_operand" "")))]
+  "TARGET_THUMB2 && arm_arch6 && INTVAL (operands[2]) == 255"
+  [(set (match_dup 0)
+	(zero_extend:SI (match_dup 1)))]
+  "
+  operands[1] = convert_to_mode (QImode, operands[1], 1);
+  "
+)
+
Index: arm.c
===================================================================
--- arm.c	(revision 165462)
+++ arm.c	(working copy)
@@ -22046,7 +22046,7 @@  arm_preferred_simd_mode (enum machine_mo
 static bool
 arm_class_likely_spilled_p (reg_class_t rclass)
 {
-  if ((TARGET_THUMB && rclass == LO_REGS)
+  if ((TARGET_THUMB1 && rclass == LO_REGS)
       || rclass  == CC_REG)
     return true;

Index: arm.md
===================================================================
--- arm.md	(revision 165462)
+++ arm.md	(working copy)
@@ -2015,17 +2015,9 @@ 
     {
       if (GET_CODE (operands[2]) == CONST_INT)
         {
-	  if (INTVAL (operands[2]) == 255 && arm_arch6)
-	    {
-	      operands[1] = convert_to_mode (QImode, operands[1], 1);
-	      emit_insn (gen_thumb2_zero_extendqisi2_v6 (operands[0],
-							 operands[1]));
-	    }
-	  else
-	    arm_split_constant (AND, SImode, NULL_RTX,
-				INTVAL (operands[2]), operands[0],
-				operands[1],
-				optimize && can_create_pseudo_p ());
+	  arm_split_constant (AND, SImode, NULL_RTX,
+			      INTVAL (operands[2]), operands[0],
+			      operands[1], optimize && can_create_pseudo_p ());

           DONE;
         }

Index: pr44999.c
===================================================================
--- pr44999.c	(revision 165462)
+++ pr44999.c	(working copy)
@@ -5,5 +5,5 @@ 

 int tp(int x, int y)
 {
-  return (x & 0xff) - (y & 0xffff);
+  return ((x+3) & 0xff) - (y & 0xffff);
 }