On 20/06/17 15:56, Wilco Dijkstra wrote:
> Richard Earnshaw wrote:
>
>> What testing has this had with -fpic? I'm not convinced that this
>> assertion is true in that case?
>
> I ran the GLIBC tests which pass. -fpic works since it does also form a
> constant address, ie. instead of:
>
> adrp x1, global
> add x1, x1, :lo12:global
>
> we do:
>
> adrp x1, :got:global
> ldr x1, [x1, :got_lo12:global]
>
> CSEing or rematerializing either sequence works in the same way.
>
> With TLS the resulting addresses are also constant, however this could
> cause rather complex TLS sequences to be rematerialized. It seems
> best to block that. Updated patch below:
>
>
> Aarch64_legitimate_constant_p currently returns false for symbols,
> eventhough they are always valid constants. This means LOSYM isn't
> CSEd correctly. If we return true CSE works better, resulting in
> smaller/faster code (0.3% smaller code on SPEC2006). Avoid this
> for TLS symbols since their sequence is complex.
>
> int x0 = 1, x1 = 2, x2 = 3;
>
> int
> f (int x, int y)
> {
> x += x1;
> if (x > 100)
> y += x2;
> x += x0;
> return x + y;
> }
>
> Before:
> adrp x3, .LANCHOR0
> add x4, x3, :lo12:.LANCHOR0
> ldr w2, [x3, #:lo12:.LANCHOR0]
> add w0, w0, w2
> cmp w0, 100
> ble .L5
> ldr w2, [x4, 8]
> add w1, w1, w2
> .L5:
> add x3, x3, :lo12:.LANCHOR0
> ldr w2, [x3, 4]
> add w0, w0, w2
> add w0, w0, w1
> ret
>
> After:
> adrp x2, .LANCHOR0
> add x3, x2, :lo12:.LANCHOR0
> ldr w2, [x2, #:lo12:.LANCHOR0]
> add w0, w0, w2
> cmp w0, 100
> ble .L5
> ldr w2, [x3, 8]
> add w1, w1, w2
> .L5:
> ldr w2, [x3, 4]
> add w0, w0, w2
> add w0, w0, w1
> ret
>
> Bootstrap OK, OK for commit?
>
> ChangeLog:
> 2017-06-20 Wilco Dijkstra <wdijk...@arm.com>
>
> * config/aarch64/aarch64.c (aarch64_legitimate_constant_p):
> Return true for non-tls symbols.
> --
OK.
R.
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index
> 5ec6bbfcf484baa4005b8a88cb98d0d04f710877..060cd8476d2954119daac495ecb059c9be73edbe
> 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -10111,6 +10111,11 @@ aarch64_legitimate_constant_p (machine_mode mode,
> rtx x)
> && aarch64_valid_symref (XEXP (x, 0), GET_MODE (XEXP (x, 0))))
> return true;
>
> + /* Treat symbols as constants. Avoid TLS symbols as they are complex,
> + so spilling them is better than rematerialization. */
> + if (SYMBOL_REF_P (x) && !SYMBOL_REF_TLS_MODEL (x))
> + return true;
> +
> return aarch64_constant_address_p (x);
> }
>
>
