[PR83370,AARCH64] Use tighter register constraints for sibcall patterns.

Message ID 4fd9e978-bd24-a461-54d8-8d4635f063a7@foss.arm.com
State New
Headers show
Series
  • [PR83370,AARCH64] Use tighter register constraints for sibcall patterns.
Related show

Commit Message

Renlin Li Dec. 11, 2017, 3:27 p.m.
Hi all,

In aarch64 backend, ip0/ip1 register will be used in the prologue/epilogue as
temporary register.

When the compiler is performing sibcall optimization. It has the chance to use
ip0/ip1 register for indirect function call to hold the address. However, those two register might
be clobbered by the epilogue code which makes the last sibcall instruction
invalid.

The following is an extreme example:
When built with -O2 -ffixed-x0 -ffixed-x1 -ffixed-x2 -ffixed-x3 -ffixed-x4 -ffixed-x5 -ffixed-x6 -ffixed-x7
-ffixed-x8 -ffixed-x9 -ffixed-x10 -ffixed-x11 -ffixed-x12 -ffixed-x13 -ffixed-x14 -ffixed-x15 -ffixed-x17 -ffixed-x18

void (*f)();
int xx;

void tailcall (int i)

{
    int arr[5000];
    xx = arr[i];
    f();
}


tailcall:
	mov	x16, 20016
	sub	sp, sp, x16
	adrp	x16, .LANCHOR0
	stp	x19, x30, [sp]
	add	x19, sp, 16
	ldr	s0, [x19, w0, sxtw 2]
	ldp	x19, x30, [sp]
	str	s0, [x16, #:lo12:.LANCHOR0]
	mov	x16, 20016
	add	sp, sp, x16
	br	x16   // oops


As we can see, x16 is used in the indirect sibcall instruction. It is used as
a temporary in the epilogue code as well. The register allocation is invalid.

With the change, the register allocator is only allowed to use r0-r15, r18 for
indirect sibcall instruction.

For this particular case above, the compiler will ICE as there is not register
could be used for this sibcall instruction.
And I think it is better to fail instead of wrong code-generation.

test.c:10:1: error: unable to generate reloads for:
  }
  ^
(call_insn/j 16 12 17 2 (parallel [
             (call (mem:DI (reg/f:DI 84 [ f ]) [0 *f.0_2 S8 A8])
                 (const_int 0 [0]))
             (return)
         ]) "test.c":9 42 {*sibcall_insn}
      (expr_list:REG_DEAD (reg/f:DI 84 [ f ])
         (expr_list:REG_CALL_DECL (nil)
             (nil)))
     (expr_list (clobber (reg:DI 17 x17))
         (expr_list (clobber (reg:DI 16 x16))
             (nil))))

aarch64-none-elf test without regressions. Okay to commit?
The same issue affects gcc-6, gcc-7 as well. Backport are needed for those branches.

Regards,
Renlin

gcc/ChangeLog:

2017-12-11  Renlin Li  <renlin.li@arm.com>

         PR target/83370
	* config/aarch64/aarch64.c (aarch64_class_max_nregs): Handle
	TAILCALL_ADDR_REGS.
	(aarch64_register_move_cost): Likewise.
	* config/aarch64/aarch64.h (reg_class): Rename CALLER_SAVE_REGS to TAILCALL_ADDR_REGS.
	* config/aarch64/constraints.md (Ucs): Update register constraint.

Comments

Renlin Li Dec. 20, 2017, 10:29 a.m. | #1
Ping ~

On 11/12/17 15:27, Renlin Li wrote:
> Hi all,

> 

> In aarch64 backend, ip0/ip1 register will be used in the prologue/epilogue as

> temporary register.

> 

> When the compiler is performing sibcall optimization. It has the chance to use

> ip0/ip1 register for indirect function call to hold the address. However, those two register might

> be clobbered by the epilogue code which makes the last sibcall instruction

> invalid.

> 

> The following is an extreme example:

> When built with -O2 -ffixed-x0 -ffixed-x1 -ffixed-x2 -ffixed-x3 -ffixed-x4 -ffixed-x5 -ffixed-x6 -ffixed-x7

> -ffixed-x8 -ffixed-x9 -ffixed-x10 -ffixed-x11 -ffixed-x12 -ffixed-x13 -ffixed-x14 -ffixed-x15 -ffixed-x17 -ffixed-x18

> 

> void (*f)();

> int xx;

> 

> void tailcall (int i)

> 

> {

>     int arr[5000];

>     xx = arr[i];

>     f();

> }

> 

> 

> tailcall:

>      mov    x16, 20016

>      sub    sp, sp, x16

>      adrp    x16, .LANCHOR0

>      stp    x19, x30, [sp]

>      add    x19, sp, 16

>      ldr    s0, [x19, w0, sxtw 2]

>      ldp    x19, x30, [sp]

>      str    s0, [x16, #:lo12:.LANCHOR0]

>      mov    x16, 20016

>      add    sp, sp, x16

>      br    x16   // oops

> 

> 

> As we can see, x16 is used in the indirect sibcall instruction. It is used as

> a temporary in the epilogue code as well. The register allocation is invalid.

> 

> With the change, the register allocator is only allowed to use r0-r15, r18 for

> indirect sibcall instruction.

> 

> For this particular case above, the compiler will ICE as there is not register

> could be used for this sibcall instruction.

> And I think it is better to fail instead of wrong code-generation.

> 

> test.c:10:1: error: unable to generate reloads for:

>   }

>   ^

> (call_insn/j 16 12 17 2 (parallel [

>              (call (mem:DI (reg/f:DI 84 [ f ]) [0 *f.0_2 S8 A8])

>                  (const_int 0 [0]))

>              (return)

>          ]) "test.c":9 42 {*sibcall_insn}

>       (expr_list:REG_DEAD (reg/f:DI 84 [ f ])

>          (expr_list:REG_CALL_DECL (nil)

>              (nil)))

>      (expr_list (clobber (reg:DI 17 x17))

>          (expr_list (clobber (reg:DI 16 x16))

>              (nil))))

> 

> aarch64-none-elf test without regressions. Okay to commit?

> The same issue affects gcc-6, gcc-7 as well. Backport are needed for those branches.

> 

> Regards,

> Renlin

> 

> gcc/ChangeLog:

> 

> 2017-12-11  Renlin Li  <renlin.li@arm.com>

> 

>          PR target/83370

>      * config/aarch64/aarch64.c (aarch64_class_max_nregs): Handle

>      TAILCALL_ADDR_REGS.

>      (aarch64_register_move_cost): Likewise.

>      * config/aarch64/aarch64.h (reg_class): Rename CALLER_SAVE_REGS to TAILCALL_ADDR_REGS.

>      * config/aarch64/constraints.md (Ucs): Update register constraint.
Richard Sandiford Jan. 29, 2018, 8:23 p.m. | #2
Renlin Li <renlin.li@foss.arm.com> writes:
> Hi all,

>

> In aarch64 backend, ip0/ip1 register will be used in the prologue/epilogue as

> temporary register.

>

> When the compiler is performing sibcall optimization. It has the chance to use

> ip0/ip1 register for indirect function call to hold the address. However, those two register might

> be clobbered by the epilogue code which makes the last sibcall instruction

> invalid.

>

> The following is an extreme example:

> When built with -O2 -ffixed-x0 -ffixed-x1 -ffixed-x2 -ffixed-x3 -ffixed-x4 -ffixed-x5 -ffixed-x6 -ffixed-x7

> -ffixed-x8 -ffixed-x9 -ffixed-x10 -ffixed-x11 -ffixed-x12 -ffixed-x13 -ffixed-x14 -ffixed-x15 -ffixed-x17 -ffixed-x18

>

> void (*f)();

> int xx;

>

> void tailcall (int i)

>

> {

>     int arr[5000];

>     xx = arr[i];

>     f();

> }

>

>

> tailcall:

> 	mov	x16, 20016

> 	sub	sp, sp, x16

> 	adrp	x16, .LANCHOR0

> 	stp	x19, x30, [sp]

> 	add	x19, sp, 16

> 	ldr	s0, [x19, w0, sxtw 2]

> 	ldp	x19, x30, [sp]

> 	str	s0, [x16, #:lo12:.LANCHOR0]

> 	mov	x16, 20016

> 	add	sp, sp, x16

> 	br	x16   // oops

>

>

> As we can see, x16 is used in the indirect sibcall instruction. It is used as

> a temporary in the epilogue code as well. The register allocation is invalid.

>

> With the change, the register allocator is only allowed to use r0-r15, r18 for

> indirect sibcall instruction.

>

> For this particular case above, the compiler will ICE as there is not register

> could be used for this sibcall instruction.

> And I think it is better to fail instead of wrong code-generation.

>

> test.c:10:1: error: unable to generate reloads for:

>   }

>   ^

> (call_insn/j 16 12 17 2 (parallel [

>              (call (mem:DI (reg/f:DI 84 [ f ]) [0 *f.0_2 S8 A8])

>                  (const_int 0 [0]))

>              (return)

>          ]) "test.c":9 42 {*sibcall_insn}

>       (expr_list:REG_DEAD (reg/f:DI 84 [ f ])

>          (expr_list:REG_CALL_DECL (nil)

>              (nil)))

>      (expr_list (clobber (reg:DI 17 x17))

>          (expr_list (clobber (reg:DI 16 x16))

>              (nil))))

>

> aarch64-none-elf test without regressions. Okay to commit?

> The same issue affects gcc-6, gcc-7 as well. Backport are needed for those branches.


The patch looks good to me FWIW.  How about adding something like
the following testcase?

------------------------------------
/* { dg-do run } */
/* { dg-options "-O2" } */

typedef void (*fun) (void);

void __attribute__ ((noipa))
f (fun x1)
{
  register fun x2 asm ("x16");
  int arr[5000];
  int *volatile ptr = arr;
  asm ("mov %0, %1" : "=r" (x2) : "r" (x1));
  x2 ();
}

void g (void) {}

int
main (void)
{
  f (g);
}
------------------------------------

> [...]

> diff --git a/gcc/config/aarch64/constraints.md b/gcc/config/aarch64/constraints.md

> index af4143ef756464afac29d17f124b436520f90451..c3791aa89562a5d5542098d2f7951afc57901150 100644

> --- a/gcc/config/aarch64/constraints.md

> +++ b/gcc/config/aarch64/constraints.md

> @@ -21,8 +21,8 @@

>  (define_register_constraint "k" "STACK_REG"

>    "@internal The stack register.")

>  

> -(define_register_constraint "Ucs" "CALLER_SAVE_REGS"

> -  "@internal The caller save registers.")

> +(define_register_constraint "Ucs" "TAILCALL_ADDR_REGS"

> +  "@internal The indirect tail call address registers")

>  

>  (define_register_constraint "w" "FP_REGS"

>    "Floating point and SIMD vector registers.")


Maybe "@internal Registers suitable for an indirect tail call"?
Unlike the caller-save registers, these aren't a predefined set.

Thanks,
Richard
Renlin Li Jan. 30, 2018, 3:45 p.m. | #3
Hi Richard,

Thanks for the review!

On 29/01/18 20:23, Richard Sandiford wrote:
> 

> The patch looks good to me FWIW.  How about adding something like

> the following testcase?

> 

> ------------------------------------

> /* { dg-do run } */

> /* { dg-options "-O2" } */

> 

> typedef void (*fun) (void);

> 

> void __attribute__ ((noipa))

> f (fun x1)

> {

>    register fun x2 asm ("x16");

>    int arr[5000];

>    int *volatile ptr = arr;

>    asm ("mov %0, %1" : "=r" (x2) : "r" (x1));

>    x2 ();

> }

> 

> void g (void) {}

> 

> int

> main (void)

> {

>    f (g);

> }

> ------------------------------------


It was hard for me to construct an test case at that time.
Your example here exactly reflect the problem. The code-gen before the change is:

f:
	mov	x16, 20016
	sub	sp, sp, x16
	add	x0, sp, 16
	mov	x16, 20016
	str	x0, [sp, 8]
	add	sp, sp, x16
	br	x16

After the change to the register constraint:

f:
	mov	x16, 20016
	sub	sp, sp, x16
	// Start of user assembly
// 9 "indirect_tail_call_reg.c" 1
	mov x16, x0
// 0 "" 2
	// End of user assembly
	add	x0, sp, 16
	str	x0, [sp, 8]
	mov	x0, x16
	mov	x16, 20016
	add	sp, sp, x16
	br	x0

I updated the patch with the new test case,
the wording about the register constraint is also updated.

Thanks,
Renlin

gcc/ChangeLog:

2018-01-30  Renlin Li  <renlin.li@arm.com>

     * config/aarch64/aarch64.c (aarch64_class_max_nregs): Handle
     TAILCALL_ADDR_REGS.
     (aarch64_register_move_cost): Likewise.
     * config/aarch64/aarch64.h (reg_class): Rename CALLER_SAVE_REGS to
     TAILCALL_ADDR_REGS.
     (REG_CLASS_NAMES): Likewise.
     (REG_CLASS_CONTENTS): Rename CALLER_SAVE_REGS to
     TAILCALL_ADDR_REGS. Remove IP registers.
     * config/aarch64/aarch64.md (Ucs): Update register constraint.

gcc/testsuite/ChangeLog:

2018-01-30  Richard Sandiford  <richard.sandiford@linaro.org>

     * gcc.target/aarch64/indirect_tail_call_reg.c: New.

> 

>> [...]

>> diff --git a/gcc/config/aarch64/constraints.md b/gcc/config/aarch64/constraints.md

>> index af4143ef756464afac29d17f124b436520f90451..c3791aa89562a5d5542098d2f7951afc57901150 100644

>> --- a/gcc/config/aarch64/constraints.md

>> +++ b/gcc/config/aarch64/constraints.md

>> @@ -21,8 +21,8 @@

>>   (define_register_constraint "k" "STACK_REG"

>>     "@internal The stack register.")

>>   

>> -(define_register_constraint "Ucs" "CALLER_SAVE_REGS"

>> -  "@internal The caller save registers.")

>> +(define_register_constraint "Ucs" "TAILCALL_ADDR_REGS"

>> +  "@internal The indirect tail call address registers")

>>   

>>   (define_register_constraint "w" "FP_REGS"

>>     "Floating point and SIMD vector registers.")

> 

> Maybe "@internal Registers suitable for an indirect tail call"?

> Unlike the caller-save registers, these aren't a predefined set.

> 

> Thanks,

> Richard

>
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 93d29b84d47b7017661a2129d61e7d740bbf7c93..322b7f4628aa69cf331c12ff2c8df351890da9ef 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -446,7 +446,7 @@ extern unsigned aarch64_architecture_version;
 enum reg_class
 {
   NO_REGS,
-  CALLER_SAVE_REGS,
+  TAILCALL_ADDR_REGS,
   GENERAL_REGS,
   STACK_REG,
   POINTER_REGS,
@@ -462,7 +462,7 @@ enum reg_class
 #define REG_CLASS_NAMES				\
 {						\
   "NO_REGS",					\
-  "CALLER_SAVE_REGS",				\
+  "TAILCALL_ADDR_REGS",				\
   "GENERAL_REGS",				\
   "STACK_REG",					\
   "POINTER_REGS",				\
@@ -475,7 +475,7 @@ enum reg_class
 #define REG_CLASS_CONTENTS						\
 {									\
   { 0x00000000, 0x00000000, 0x00000000 },	/* NO_REGS */		\
-  { 0x0007ffff, 0x00000000, 0x00000000 },	/* CALLER_SAVE_REGS */	\
+  { 0x0004ffff, 0x00000000, 0x00000000 },	/* TAILCALL_ADDR_REGS */\
   { 0x7fffffff, 0x00000000, 0x00000003 },	/* GENERAL_REGS */	\
   { 0x80000000, 0x00000000, 0x00000000 },	/* STACK_REG */		\
   { 0xffffffff, 0x00000000, 0x00000003 },	/* POINTER_REGS */	\
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 1da313f57e0eed4df36dbd15aecbae9fd73f7388..59ca95019ddf0491c382e7ee2b99966694d0b36d 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -6062,7 +6062,7 @@ aarch64_class_max_nregs (reg_class_t regclass, machine_mode mode)
 {
   switch (regclass)
     {
-    case CALLER_SAVE_REGS:
+    case TAILCALL_ADDR_REGS:
     case POINTER_REGS:
     case GENERAL_REGS:
     case ALL_REGS:
@@ -8228,10 +8228,10 @@ aarch64_register_move_cost (machine_mode mode,
     = aarch64_tune_params.regmove_cost;
 
   /* Caller save and pointer regs are equivalent to GENERAL_REGS.  */
-  if (to == CALLER_SAVE_REGS || to == POINTER_REGS)
+  if (to == TAILCALL_ADDR_REGS || to == POINTER_REGS)
     to = GENERAL_REGS;
 
-  if (from == CALLER_SAVE_REGS || from == POINTER_REGS)
+  if (from == TAILCALL_ADDR_REGS || from == POINTER_REGS)
     from = GENERAL_REGS;
 
   /* Moving between GPR and stack cost is the same as GP2GP.  */
diff --git a/gcc/config/aarch64/constraints.md b/gcc/config/aarch64/constraints.md
index 70ea3cde686914db4e0fc93819774b0b7ff17ee5..911d00c67b83c3d9d10bfe8283d4f762d8f447da 100644
--- a/gcc/config/aarch64/constraints.md
+++ b/gcc/config/aarch64/constraints.md
@@ -21,8 +21,8 @@
 (define_register_constraint "k" "STACK_REG"
   "@internal The stack register.")
 
-(define_register_constraint "Ucs" "CALLER_SAVE_REGS"
-  "@internal The caller save registers.")
+(define_register_constraint "Ucs" "TAILCALL_ADDR_REGS"
+  "@internal Registers suitable for an indirect tail call")
 
 (define_register_constraint "w" "FP_REGS"
   "Floating point and SIMD vector registers.")
diff --git a/gcc/testsuite/gcc.target/aarch64/indirect_tail_call_reg.c b/gcc/testsuite/gcc.target/aarch64/indirect_tail_call_reg.c
new file mode 100644
index 0000000000000000000000000000000000000000..cde8876279f63c56e1a9ccc925195c136e4de7f1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/indirect_tail_call_reg.c
@@ -0,0 +1,22 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+typedef void (*fun) (void);
+
+void __attribute__ ((noipa))
+f (fun x1)
+{
+  register fun x2 asm ("x16");
+  int arr[5000];
+  int *volatile ptr = arr;
+  asm ("mov %0, %1" : "=r" (x2) : "r" (x1));
+  x2 ();
+}
+
+void g (void) {}
+
+int
+main (void)
+{
+  f (g);
+}
James Greenhalgh Jan. 31, 2018, 5:56 p.m. | #4
On Tue, Jan 30, 2018 at 03:45:17PM +0000, Renlin Li wrote:
> Hi Richard,

> 

> Thanks for the review!

> 

> On 29/01/18 20:23, Richard Sandiford wrote:

> > 

> > The patch looks good to me FWIW.  How about adding something like

> > the following testcase?

> > 

> > ------------------------------------

> > /* { dg-do run } */

> > /* { dg-options "-O2" } */

> > 

> > typedef void (*fun) (void);

> > 

> > void __attribute__ ((noipa))

> > f (fun x1)

> > {

> >    register fun x2 asm ("x16");

> >    int arr[5000];

> >    int *volatile ptr = arr;

> >    asm ("mov %0, %1" : "=r" (x2) : "r" (x1));

> >    x2 ();

> > }

> > 

> > void g (void) {}

> > 

> > int

> > main (void)

> > {

> >    f (g);

> > }

> > ------------------------------------

> 

> It was hard for me to construct an test case at that time.

> Your example here exactly reflect the problem. The code-gen before the change is:

> 

> f:

> 	mov	x16, 20016

> 	sub	sp, sp, x16

> 	add	x0, sp, 16

> 	mov	x16, 20016

> 	str	x0, [sp, 8]

> 	add	sp, sp, x16

> 	br	x16

> 

> After the change to the register constraint:

> 

> f:

> 	mov	x16, 20016

> 	sub	sp, sp, x16

> 	// Start of user assembly

> // 9 "indirect_tail_call_reg.c" 1

> 	mov x16, x0

> // 0 "" 2

> 	// End of user assembly

> 	add	x0, sp, 16

> 	str	x0, [sp, 8]

> 	mov	x0, x16

> 	mov	x16, 20016

> 	add	sp, sp, x16

> 	br	x0

> 

> I updated the patch with the new test case,

> the wording about the register constraint is also updated.


This patch is OK.

Thanks,
James

> gcc/ChangeLog:

> 

> 2018-01-30  Renlin Li  <renlin.li@arm.com>

> 

>      * config/aarch64/aarch64.c (aarch64_class_max_nregs): Handle

>      TAILCALL_ADDR_REGS.

>      (aarch64_register_move_cost): Likewise.

>      * config/aarch64/aarch64.h (reg_class): Rename CALLER_SAVE_REGS to

>      TAILCALL_ADDR_REGS.

>      (REG_CLASS_NAMES): Likewise.

>      (REG_CLASS_CONTENTS): Rename CALLER_SAVE_REGS to

>      TAILCALL_ADDR_REGS. Remove IP registers.

>      * config/aarch64/aarch64.md (Ucs): Update register constraint.

> 

> gcc/testsuite/ChangeLog:

> 

> 2018-01-30  Richard Sandiford  <richard.sandiford@linaro.org>

> 

>      * gcc.target/aarch64/indirect_tail_call_reg.c: New.

>
Renlin Li Feb. 1, 2018, 2:43 p.m. | #5
Hi James,

Thanks for the review! I committed it on trunk.

Is it Okay to backport this patch to release branch 5, 6,7?
It applies cleanly without any logic changes.

Regards,
Renlin

On 31/01/18 17:56, James Greenhalgh wrote:
> On Tue, Jan 30, 2018 at 03:45:17PM +0000, Renlin Li wrote:

>> Hi Richard,

>>

>> Thanks for the review!

>>

>> On 29/01/18 20:23, Richard Sandiford wrote:

>>>

>>> The patch looks good to me FWIW.  How about adding something like

>>> the following testcase?

>>>

>>> ------------------------------------

>>> /* { dg-do run } */

>>> /* { dg-options "-O2" } */

>>>

>>> typedef void (*fun) (void);

>>>

>>> void __attribute__ ((noipa))

>>> f (fun x1)

>>> {

>>>     register fun x2 asm ("x16");

>>>     int arr[5000];

>>>     int *volatile ptr = arr;

>>>     asm ("mov %0, %1" : "=r" (x2) : "r" (x1));

>>>     x2 ();

>>> }

>>>

>>> void g (void) {}

>>>

>>> int

>>> main (void)

>>> {

>>>     f (g);

>>> }

>>> ------------------------------------

>>

>> It was hard for me to construct an test case at that time.

>> Your example here exactly reflect the problem. The code-gen before the change is:

>>

>> f:

>> 	mov	x16, 20016

>> 	sub	sp, sp, x16

>> 	add	x0, sp, 16

>> 	mov	x16, 20016

>> 	str	x0, [sp, 8]

>> 	add	sp, sp, x16

>> 	br	x16

>>

>> After the change to the register constraint:

>>

>> f:

>> 	mov	x16, 20016

>> 	sub	sp, sp, x16

>> 	// Start of user assembly

>> // 9 "indirect_tail_call_reg.c" 1

>> 	mov x16, x0

>> // 0 "" 2

>> 	// End of user assembly

>> 	add	x0, sp, 16

>> 	str	x0, [sp, 8]

>> 	mov	x0, x16

>> 	mov	x16, 20016

>> 	add	sp, sp, x16

>> 	br	x0

>>

>> I updated the patch with the new test case,

>> the wording about the register constraint is also updated.

> 

> This patch is OK.

> 

> Thanks,

> James

> 

>> gcc/ChangeLog:

>>

>> 2018-01-30  Renlin Li  <renlin.li@arm.com>

>>

>>       * config/aarch64/aarch64.c (aarch64_class_max_nregs): Handle

>>       TAILCALL_ADDR_REGS.

>>       (aarch64_register_move_cost): Likewise.

>>       * config/aarch64/aarch64.h (reg_class): Rename CALLER_SAVE_REGS to

>>       TAILCALL_ADDR_REGS.

>>       (REG_CLASS_NAMES): Likewise.

>>       (REG_CLASS_CONTENTS): Rename CALLER_SAVE_REGS to

>>       TAILCALL_ADDR_REGS. Remove IP registers.

>>       * config/aarch64/aarch64.md (Ucs): Update register constraint.

>>

>> gcc/testsuite/ChangeLog:

>>

>> 2018-01-30  Richard Sandiford  <richard.sandiford@linaro.org>

>>

>>       * gcc.target/aarch64/indirect_tail_call_reg.c: New.

>>

>
James Greenhalgh Feb. 1, 2018, 2:47 p.m. | #6
On Thu, Feb 01, 2018 at 02:43:17PM +0000, Renlin Li wrote:
> Hi James,

> 

> Thanks for the review! I committed it on trunk.

> 

> Is it Okay to backport this patch to release branch 5, 6,7?

> It applies cleanly without any logic changes.


6 and 7 yes, OK.

5 is no longer supported so may be a waste of your time :-)

James

Patch

diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 93d29b84d47b7017661a2129d61e7d740bbf7c93..322b7f4628aa69cf331c12ff2c8df351890da9ef 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -446,7 +446,7 @@  extern unsigned aarch64_architecture_version;
 enum reg_class
 {
   NO_REGS,
-  CALLER_SAVE_REGS,
+  TAILCALL_ADDR_REGS,
   GENERAL_REGS,
   STACK_REG,
   POINTER_REGS,
@@ -462,7 +462,7 @@  enum reg_class
 #define REG_CLASS_NAMES				\
 {						\
   "NO_REGS",					\
-  "CALLER_SAVE_REGS",				\
+  "TAILCALL_ADDR_REGS",				\
   "GENERAL_REGS",				\
   "STACK_REG",					\
   "POINTER_REGS",				\
@@ -475,7 +475,7 @@  enum reg_class
 #define REG_CLASS_CONTENTS						\
 {									\
   { 0x00000000, 0x00000000, 0x00000000 },	/* NO_REGS */		\
-  { 0x0007ffff, 0x00000000, 0x00000000 },	/* CALLER_SAVE_REGS */	\
+  { 0x0004ffff, 0x00000000, 0x00000000 },	/* TAILCALL_ADDR_REGS */\
   { 0x7fffffff, 0x00000000, 0x00000003 },	/* GENERAL_REGS */	\
   { 0x80000000, 0x00000000, 0x00000000 },	/* STACK_REG */		\
   { 0xffffffff, 0x00000000, 0x00000003 },	/* POINTER_REGS */	\
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 75a6c0d0421354d7c0759292947eb5d407f5b703..66d503ac6edf59a1ea2fa3675fbbe03d70769833 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -6060,7 +6060,7 @@  aarch64_class_max_nregs (reg_class_t regclass, machine_mode mode)
 {
   switch (regclass)
     {
-    case CALLER_SAVE_REGS:
+    case TAILCALL_ADDR_REGS:
     case POINTER_REGS:
     case GENERAL_REGS:
     case ALL_REGS:
@@ -8226,10 +8226,10 @@  aarch64_register_move_cost (machine_mode mode,
     = aarch64_tune_params.regmove_cost;
 
   /* Caller save and pointer regs are equivalent to GENERAL_REGS.  */
-  if (to == CALLER_SAVE_REGS || to == POINTER_REGS)
+  if (to == TAILCALL_ADDR_REGS || to == POINTER_REGS)
     to = GENERAL_REGS;
 
-  if (from == CALLER_SAVE_REGS || from == POINTER_REGS)
+  if (from == TAILCALL_ADDR_REGS || from == POINTER_REGS)
     from = GENERAL_REGS;
 
   /* Moving between GPR and stack cost is the same as GP2GP.  */
diff --git a/gcc/config/aarch64/constraints.md b/gcc/config/aarch64/constraints.md
index af4143ef756464afac29d17f124b436520f90451..c3791aa89562a5d5542098d2f7951afc57901150 100644
--- a/gcc/config/aarch64/constraints.md
+++ b/gcc/config/aarch64/constraints.md
@@ -21,8 +21,8 @@ 
 (define_register_constraint "k" "STACK_REG"
   "@internal The stack register.")
 
-(define_register_constraint "Ucs" "CALLER_SAVE_REGS"
-  "@internal The caller save registers.")
+(define_register_constraint "Ucs" "TAILCALL_ADDR_REGS"
+  "@internal The indirect tail call address registers")
 
 (define_register_constraint "w" "FP_REGS"
   "Floating point and SIMD vector registers.")