Patchwork [ARM] Remove register constraints from push multiple patterns (was Re: [PATCH] Provide a hook for target to disable register renaming for some instructions)

login
register
mail settings
Submitter Jie Zhang
Date July 1, 2010, 1:19 p.m.
Message ID <4C2C95F5.9000000@codesourcery.com>
Download mbox | patch
Permalink /patch/57549/
State New
Headers show

Comments

Jie Zhang - July 1, 2010, 1:19 p.m.
On 07/01/2010 02:03 AM, Richard Henderson wrote:
> On 06/30/2010 10:53 AM, Jie Zhang wrote:
>> When compiler an application for ARM, the GAS issued a warning:
>>
>> Warning: register range not in ascending order
>>
>> for the instruction
>>
>> push    {ip, r3, r4, lr}
>>
>> Before regrename pass, this instruction looked like
>>
>> push    {r0, r3, r4, lr}
>
> Doesn't it work just as well to simply remove the register
> constraint from the push_multi instruction?  Without that
> the regrename pass won't get a register class for the
> operand and will leave it alone.
>
According to Richard's comment, a new patch is attached.  It removes 
register constraints from all three push multiple patterns.  Since there 
are no constraints for operand 1 of *push_multi_vfp, I use a new 
predicate to make it more stricter.

Tested on arm-none-eabi for NEON and non-NEON. No regressions found.

Is it OK?

Regards,
Richard Earnshaw - July 1, 2010, 1:37 p.m.
On Thu, 2010-07-01 at 21:19 +0800, Jie Zhang wrote:
> On 07/01/2010 02:03 AM, Richard Henderson wrote:
> > On 06/30/2010 10:53 AM, Jie Zhang wrote:
> >> When compiler an application for ARM, the GAS issued a warning:
> >>
> >> Warning: register range not in ascending order
> >>
> >> for the instruction
> >>
> >> push    {ip, r3, r4, lr}
> >>
> >> Before regrename pass, this instruction looked like
> >>
> >> push    {r0, r3, r4, lr}
> >
> > Doesn't it work just as well to simply remove the register
> > constraint from the push_multi instruction?  Without that
> > the regrename pass won't get a register class for the
> > operand and will leave it alone.
> >
> According to Richard's comment, a new patch is attached.  It removes 
> register constraints from all three push multiple patterns.  Since there 
> are no constraints for operand 1 of *push_multi_vfp, I use a new 
> predicate to make it more stricter.
> 
> Tested on arm-none-eabi for NEON and non-NEON. No regressions found.
> 
> Is it OK?
> 
> Regards,

OK

R.
Jie Zhang - July 3, 2010, 4:44 p.m.
On 07/01/2010 09:37 PM, Richard Earnshaw wrote:
>
> On Thu, 2010-07-01 at 21:19 +0800, Jie Zhang wrote:
>> On 07/01/2010 02:03 AM, Richard Henderson wrote:
>>> On 06/30/2010 10:53 AM, Jie Zhang wrote:
>>>> When compiler an application for ARM, the GAS issued a warning:
>>>>
>>>> Warning: register range not in ascending order
>>>>
>>>> for the instruction
>>>>
>>>> push    {ip, r3, r4, lr}
>>>>
>>>> Before regrename pass, this instruction looked like
>>>>
>>>> push    {r0, r3, r4, lr}
>>>
>>> Doesn't it work just as well to simply remove the register
>>> constraint from the push_multi instruction?  Without that
>>> the regrename pass won't get a register class for the
>>> operand and will leave it alone.
>>>
>> According to Richard's comment, a new patch is attached.  It removes
>> register constraints from all three push multiple patterns.  Since there
>> are no constraints for operand 1 of *push_multi_vfp, I use a new
>> predicate to make it more stricter.
>>
>> Tested on arm-none-eabi for NEON and non-NEON. No regressions found.
>>
>> Is it OK?
>>
>> Regards,
>
> OK
>
Thanks. Committed on trunk. OK for 4.5 branch, too?
Richard Earnshaw - July 5, 2010, 8:51 a.m.
On Sun, 2010-07-04 at 00:44 +0800, Jie Zhang wrote:
> On 07/01/2010 09:37 PM, Richard Earnshaw wrote:
> >
> > On Thu, 2010-07-01 at 21:19 +0800, Jie Zhang wrote:
> >> On 07/01/2010 02:03 AM, Richard Henderson wrote:
> >>> On 06/30/2010 10:53 AM, Jie Zhang wrote:
> >>>> When compiler an application for ARM, the GAS issued a warning:
> >>>>
> >>>> Warning: register range not in ascending order
> >>>>
> >>>> for the instruction
> >>>>
> >>>> push    {ip, r3, r4, lr}
> >>>>
> >>>> Before regrename pass, this instruction looked like
> >>>>
> >>>> push    {r0, r3, r4, lr}
> >>>
> >>> Doesn't it work just as well to simply remove the register
> >>> constraint from the push_multi instruction?  Without that
> >>> the regrename pass won't get a register class for the
> >>> operand and will leave it alone.
> >>>
> >> According to Richard's comment, a new patch is attached.  It removes
> >> register constraints from all three push multiple patterns.  Since there
> >> are no constraints for operand 1 of *push_multi_vfp, I use a new
> >> predicate to make it more stricter.
> >>
> >> Tested on arm-none-eabi for NEON and non-NEON. No regressions found.
> >>
> >> Is it OK?
> >>
> >> Regards,
> >
> > OK
> >
> Thanks. Committed on trunk. OK for 4.5 branch, too?
> 

Yes.

R.
Jie Zhang - July 5, 2010, 12:47 p.m.
On 07/05/2010 04:51 PM, Richard Earnshaw wrote:
>
> On Sun, 2010-07-04 at 00:44 +0800, Jie Zhang wrote:
>> On 07/01/2010 09:37 PM, Richard Earnshaw wrote:
>>>
>>> On Thu, 2010-07-01 at 21:19 +0800, Jie Zhang wrote:
>>>> On 07/01/2010 02:03 AM, Richard Henderson wrote:
>>>>> On 06/30/2010 10:53 AM, Jie Zhang wrote:
>>>>>> When compiler an application for ARM, the GAS issued a warning:
>>>>>>
>>>>>> Warning: register range not in ascending order
>>>>>>
>>>>>> for the instruction
>>>>>>
>>>>>> push    {ip, r3, r4, lr}
>>>>>>
>>>>>> Before regrename pass, this instruction looked like
>>>>>>
>>>>>> push    {r0, r3, r4, lr}
>>>>>
>>>>> Doesn't it work just as well to simply remove the register
>>>>> constraint from the push_multi instruction?  Without that
>>>>> the regrename pass won't get a register class for the
>>>>> operand and will leave it alone.
>>>>>
>>>> According to Richard's comment, a new patch is attached.  It removes
>>>> register constraints from all three push multiple patterns.  Since there
>>>> are no constraints for operand 1 of *push_multi_vfp, I use a new
>>>> predicate to make it more stricter.
>>>>
>>>> Tested on arm-none-eabi for NEON and non-NEON. No regressions found.
>>>>
>>>> Is it OK?
>>>>
>>>> Regards,
>>>
>>> OK
>>>
>> Thanks. Committed on trunk. OK for 4.5 branch, too?
>>
>
> Yes.
>
Thanks. Committed on 4.5 branch now.

Patch


	* config/arm/vfp.md (*push_multi_vfp): Use vfp_register_operand
	as predicate for operand 1 and remove its constraint.
	* config/arm/predicates.md (vfp_register_operand): New.
	* config/arm/arm.md (*push_multi): Remove the constraint of
	operand 1.
	(*push_fp_multi): Likewise.


Index: config/arm/vfp.md
===================================================================
--- config/arm/vfp.md	(revision 161654)
+++ config/arm/vfp.md	(working copy)
@@ -1132,7 +1132,7 @@ 
 (define_insn "*push_multi_vfp"
   [(match_parallel 2 "multi_register_push"
     [(set (match_operand:BLK 0 "memory_operand" "=m")
-	  (unspec:BLK [(match_operand:DF 1 "s_register_operand" "w")]
+	  (unspec:BLK [(match_operand:DF 1 "vfp_register_operand" "")]
 		      UNSPEC_PUSH_MULT))])]
   "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_VFP"
   "* return vfp_output_fstmd (operands);"
Index: config/arm/predicates.md
===================================================================
--- config/arm/predicates.md	(revision 161654)
+++ config/arm/predicates.md	(working copy)
@@ -73,6 +73,21 @@ 
 	      || REGNO_REG_CLASS (REGNO (op)) == FPA_REGS));
 })
 
+(define_predicate "vfp_register_operand"
+  (match_code "reg,subreg")
+{
+  if (GET_CODE (op) == SUBREG)
+    op = SUBREG_REG (op);
+
+  /* We don't consider registers whose class is NO_REGS
+     to be a register operand.  */
+  return (GET_CODE (op) == REG
+	  && (REGNO (op) >= FIRST_PSEUDO_REGISTER
+	      || REGNO_REG_CLASS (REGNO (op)) == VFP_LO_REGS
+	      || (TARGET_VFPD32
+		  && REGNO_REG_CLASS (REGNO (op)) == VFP_REGS)));
+})
+
 (define_special_predicate "subreg_lowpart_operator"
   (and (match_code "subreg")
        (match_test "subreg_lowpart_p (op)")))
Index: config/arm/arm.md
===================================================================
--- config/arm/arm.md	(revision 161654)
+++ config/arm/arm.md	(working copy)
@@ -10871,7 +10871,7 @@ 
 (define_insn "*push_multi"
   [(match_parallel 2 "multi_register_push"
     [(set (match_operand:BLK 0 "memory_operand" "=m")
-	  (unspec:BLK [(match_operand:SI 1 "s_register_operand" "r")]
+	  (unspec:BLK [(match_operand:SI 1 "s_register_operand" "")]
 		      UNSPEC_PUSH_MULT))])]
   "TARGET_32BIT"
   "*
@@ -10924,7 +10924,7 @@ 
 (define_insn "*push_fp_multi"
   [(match_parallel 2 "multi_register_push"
     [(set (match_operand:BLK 0 "memory_operand" "=m")
-	  (unspec:BLK [(match_operand:XF 1 "f_register_operand" "f")]
+	  (unspec:BLK [(match_operand:XF 1 "f_register_operand" "")]
 		      UNSPEC_PUSH_MULT))])]
   "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FPA"
   "*