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...@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.sandif...@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);
+}
