Wilco Dijkstra via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Use a GOT indirection for extern weak symbols instead of a literal - this is
> the same as
> PIC/PIE and mirrors LLVM behaviour.
Just to check: I guess this part is an optimisation, because it
means that we can share the GOT entry with other TUs. Is that right?
I think it would be worth having a comment either way, whatever the
rationale. A couple of other very minor things:
> Ensure PIC/PIE use the same offset limits for symbols
> that don't use the GOT.
>
> Passes bootstrap and regress. OK for commit?
>
> ChangeLog:
> 2021-04-27 Wilco Dijkstra <wdijk...@arm.com>
>
> * config/aarch64/aarch64.c (aarch64_classify_symbol): Use GOT for
> extern weak symbols.
> Limit symbol offsets for non-GOT symbols with PIC/PIE.
>
> ---
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index
> 82957dddbe42a7f907b2960294ac7f8abf7be2ff..08aaea70d952fd62147b7f06ba781eb55e697d3a
> 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -17854,7 +17854,12 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
>
> switch (aarch64_cmodel)
> {
> + case AARCH64_CMODEL_TINY_PIC:
> case AARCH64_CMODEL_TINY:
> + if ((flag_pic || SYMBOL_REF_WEAK (x))
> + && !aarch64_symbol_binds_local_p (x))
> + return SYMBOL_TINY_GOT;
> +
> /* When we retrieve symbol + offset address, we have to make sure
> the offset does not cause overflow of the final address. But
> we have no way of knowing the address of symbol at compile time
> @@ -17865,39 +17870,29 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT
> offset)
> If offset_within_block_p is true we allow larger offsets.
> Furthermore force to memory if the symbol is a weak reference to
> something that doesn't resolve to a symbol in this module. */
The “Furthermore…” part no longer applies.
> -
> - if (SYMBOL_REF_WEAK (x) && !aarch64_symbol_binds_local_p (x))
> - return SYMBOL_FORCE_TO_MEM;
> if (!(IN_RANGE (offset, -0x10000, 0x10000)
> || offset_within_block_p (x, offset)))
> return SYMBOL_FORCE_TO_MEM;
>
> return SYMBOL_TINY_ABSOLUTE;
>
> +
> + case AARCH64_CMODEL_SMALL_SPIC:
> + case AARCH64_CMODEL_SMALL_PIC:
> case AARCH64_CMODEL_SMALL:
> + if ((flag_pic || SYMBOL_REF_WEAK (x))
> + && !aarch64_symbol_binds_local_p (x))
> + return aarch64_cmodel == AARCH64_CMODEL_SMALL_SPIC
> + ? SYMBOL_SMALL_GOT_28K : SYMBOL_SMALL_GOT_4G;
> +
I realise you're just moving this code, but: should only be one space
after “?”.
Thanks,
Richard
> /* Same reasoning as the tiny code model, but the offset cap here is
> 1MB, allowing +/-3.9GB for the offset to the symbol. */
> -
> - if (SYMBOL_REF_WEAK (x) && !aarch64_symbol_binds_local_p (x))
> - return SYMBOL_FORCE_TO_MEM;
> if (!(IN_RANGE (offset, -0x100000, 0x100000)
> || offset_within_block_p (x, offset)))
> return SYMBOL_FORCE_TO_MEM;
>
> return SYMBOL_SMALL_ABSOLUTE;
>
> - case AARCH64_CMODEL_TINY_PIC:
> - if (!aarch64_symbol_binds_local_p (x))
> - return SYMBOL_TINY_GOT;
> - return SYMBOL_TINY_ABSOLUTE;
> -
> - case AARCH64_CMODEL_SMALL_SPIC:
> - case AARCH64_CMODEL_SMALL_PIC:
> - if (!aarch64_symbol_binds_local_p (x))
> - return (aarch64_cmodel == AARCH64_CMODEL_SMALL_SPIC
> - ? SYMBOL_SMALL_GOT_28K : SYMBOL_SMALL_GOT_4G);
> - return SYMBOL_SMALL_ABSOLUTE;
> -
> case AARCH64_CMODEL_LARGE:
> /* This is alright even in PIC code as the constant
> pool reference is always PC relative and within
