| Message ID | 4fd9e978-bd24-a461-54d8-8d4635f063a7@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PR83370,AARCH64] Use tighter register constraints for sibcall patterns. | expand |
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.
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
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); +}
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. >
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. >> >
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
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.")
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.