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

[Renlin Li]

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.