Patchwork [ARM] Fix PR19599 tail

login
register
mail settings
Submitter Ramana Radhakrishnan
Date July 25, 2013, 9:42 a.m.
Message ID <51F0F300.7030604@arm.com>
Download mbox | patch
Permalink /patch/261655/
State New
Headers show

Comments

Ramana Radhakrishnan - July 25, 2013, 9:42 a.m.
Hi,

	This fixes up the issues with PR target/19599 and the issues we've had 
around it. This fixes up some of the current issues we have around this. 
I'll follow up with a separate patch around longjmps.

This puts in predicates for bx where needed - missed out CCFSM (sigh :( 
), replaces uses of "Ss" with another two letter constraint in the U 
space rather than inventing a new one and gets rid of the issues where 
we missed handling HFAs in tail calls with values (sigh :( ). Ofcourse I 
considered using 's' and 'i' but they are unusable in general because of 
this in reload:

                 case 's':
                     if (CONST_SCALAR_INT_P (operand))
                       break;
                   case 'i':
                     if (CONSTANT_P (operand)
                         && (! flag_pic || LEGITIMATE_PIC_OPERAND_P 
(operand)))
                       win = 1;
                     break;


and our definition of LEGITIMATE_PIC_OPERAND_P doesn't allow a symbol 
ref. So be it.

Regression tested on arm-none-linux-gnueabi arm / thumb multilibs v7-a , 
bootstrapped and checked it all works fine. Applied.

regards
Ramana


2013-07-25  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

         PR target/19599
         PR target/57731
         PR target/57748
         * config/arm/arm.md ("*sibcall_value_insn): Replace use of
         Ss with US. Adjust output for v5 and v4t.
         (*sibcall_value_insn): Likewise and loosen predicate on
         operand0.
         * config/arm/constraints.md ("Ss"): Rename to US.
Mikael Pettersson - July 29, 2013, 10:05 a.m.
Ramana Radhakrishnan writes:
 > Hi,
 > 
 > 	This fixes up the issues with PR target/19599 and the issues we've had
 > around it.

...

 > 2013-07-25  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>
 > 
 >          PR target/19599
 >          PR target/57731
 >          PR target/57748

Are you sure about the 57748 reference?  Looks to me like it should
reference 57837 instead.

/Mikael

 >          * config/arm/arm.md ("*sibcall_value_insn): Replace use of
 >          Ss with US. Adjust output for v5 and v4t.
 >          (*sibcall_value_insn): Likewise and loosen predicate on
 >          operand0.
 >          * config/arm/constraints.md ("Ss"): Rename to US.
Ramana Radhakrishnan - Aug. 1, 2013, 10:33 a.m.
On 07/29/13 11:05, Mikael Pettersson wrote:
> Ramana Radhakrishnan writes:
>   > Hi,
>   >
>   > 	This fixes up the issues with PR target/19599 and the issues we've had
>   > around it.
>
> ...
>
>   > 2013-07-25  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>
>   >
>   >          PR target/19599
>   >          PR target/57731
>   >          PR target/57748
>
> Are you sure about the 57748 reference?  Looks to me like it should
> reference 57837 instead.

Thanks and smashes head on the wall.

Fixed in the Changelog and log messages.

regards
Ramana

>
> /Mikael
>
>   >          * config/arm/arm.md ("*sibcall_value_insn): Replace use of
>   >          Ss with US. Adjust output for v5 and v4t.
>   >          (*sibcall_value_insn): Likewise and loosen predicate on
>   >          operand0.
>   >          * config/arm/constraints.md ("Ss"): Rename to US.
>

Patch

Index: gcc/config/arm/arm.md
===================================================================
--- gcc/config/arm/arm.md	(revision 201239)
+++ gcc/config/arm/arm.md	(working copy)
@@ -9619,7 +9619,7 @@ 
 )
 
 (define_insn "*sibcall_insn"
- [(call (mem:SI (match_operand:SI 0 "call_insn_operand" "Cs,Ss"))
+ [(call (mem:SI (match_operand:SI 0 "call_insn_operand" "Cs, US"))
 	(match_operand 1 "" ""))
   (return)
   (use (match_operand 2 "" ""))]
@@ -9630,7 +9630,7 @@ 
   else
     {
       if (arm_arch5 || arm_arch4t)
-	return \" bx\\t%0\\t%@ indirect register sibling call\";
+	return \" bx%?\\t%0\\t%@ indirect register sibling call\";
       else
 	return \"mov%?\\t%|pc, %0\\t%@ indirect register sibling call\";
     }
@@ -9639,8 +9639,8 @@ 
 )
 
 (define_insn "*sibcall_value_insn"
- [(set (match_operand 0 "s_register_operand" "")
-       (call (mem:SI (match_operand:SI 1 "call_insn_operand" "Cs,Ss"))
+ [(set (match_operand 0 "" "")
+       (call (mem:SI (match_operand:SI 1 "call_insn_operand" "Cs,US"))
 	     (match_operand 2 "" "")))
   (return)
   (use (match_operand 3 "" ""))]
@@ -9651,7 +9651,7 @@ 
   else
     {
       if (arm_arch5 || arm_arch4t)
-	return \"bx\\t%1\";
+	return \"bx%?\\t%1\";
       else
 	return \"mov%?\\t%|pc, %1\\t@ indirect sibling call \";
     }
Index: gcc/config/arm/constraints.md
===================================================================
--- gcc/config/arm/constraints.md	(revision 201239)
+++ gcc/config/arm/constraints.md	(working copy)
@@ -21,7 +21,7 @@ 
 ;; The following register constraints have been used:
 ;; - in ARM/Thumb-2 state: t, w, x, y, z
 ;; - in Thumb state: h, b
-;; - in both states: l, c, k, q
+;; - in both states: l, c, k, q, US
 ;; In ARM state, 'l' is an alias for 'r'
 ;; 'f' and 'v' were previously used for FPA and MAVERICK registers.
 
@@ -417,6 +417,12 @@ 
 						   0)
 		   && GET_CODE (XEXP (op, 0)) != POST_INC")))
 
+(define_constraint "US"
+ "@internal
+  US is a symbol reference."
+ (match_code "symbol_ref")
+)
+
 ;; We used to have constraint letters for S and R in ARM state, but
 ;; all uses of these now appear to have been removed.
 
@@ -424,8 +430,3 @@ 
 ;; this wasn't really a valid memory constraint.  Again, all uses of
 ;; this now seem to have been removed.
 
-(define_constraint "Ss"
- "@internal
-  Ss is a symbol reference."
- (match_code "symbol_ref")
-)