* [PATCH] AArch64: Cleanup aarch64_classify_symbol
@ 2021-04-28 14:36 Wilco Dijkstra
2021-04-28 15:34 ` Richard Sandiford
2021-04-28 18:04 ` Andrew Pinski
0 siblings, 2 replies; 8+ messages in thread
From: Wilco Dijkstra @ 2021-04-28 14:36 UTC (permalink / raw)
To: GCC Patches; +Cc: Kyrylo Tkachov, Richard Sandiford
Use a GOT indirection for extern weak symbols instead of a literal - this is the same as
PIC/PIE and mirrors LLVM behaviour. 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 <wdijkstr@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. */
-
- 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;
+
/* 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] AArch64: Cleanup aarch64_classify_symbol
2021-04-28 14:36 [PATCH] AArch64: Cleanup aarch64_classify_symbol Wilco Dijkstra
@ 2021-04-28 15:34 ` Richard Sandiford
2021-04-28 17:48 ` Wilco Dijkstra
2021-04-28 18:04 ` Andrew Pinski
1 sibling, 1 reply; 8+ messages in thread
From: Richard Sandiford @ 2021-04-28 15:34 UTC (permalink / raw)
To: Wilco Dijkstra via Gcc-patches; +Cc: Wilco Dijkstra
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 <wdijkstr@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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] AArch64: Cleanup aarch64_classify_symbol
2021-04-28 15:34 ` Richard Sandiford
@ 2021-04-28 17:48 ` Wilco Dijkstra
2021-04-30 8:46 ` Richard Sandiford
0 siblings, 1 reply; 8+ messages in thread
From: Wilco Dijkstra @ 2021-04-28 17:48 UTC (permalink / raw)
To: Richard Sandiford; +Cc: Wilco Dijkstra via Gcc-patches
Hi Richard,
> 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:
It's just to make the code simpler and more orthogonal - the size of the
sequence and sharing of GOT/literal is identical in both cases.
Here is v2 with comments/spaces fixed up:
Use a GOT indirection for extern weak symbols instead of a literal - this is the same as
PIC/PIE and mirrors LLVM behaviour. 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 <wdijkstr@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..641c83b479e76cbcc75b299eb7ae5f634d9db7cd 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -17854,7 +17854,14 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
switch (aarch64_cmodel)
{
+ case AARCH64_CMODEL_TINY_PIC:
case AARCH64_CMODEL_TINY:
+ /* With -fPIC non-local symbols use the GOT. For orthogonality
+ always use the GOT for extern weak symbols. */
+ 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
@@ -17862,42 +17869,30 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
symbol + offset is outside the addressible range of +/-1MB in the
TINY code model. So we limit the maximum offset to +/-64KB and
assume the offset to the symbol is not larger than +/-(1MB - 64KB).
- 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. */
-
- if (SYMBOL_REF_WEAK (x) && !aarch64_symbol_binds_local_p (x))
- return SYMBOL_FORCE_TO_MEM;
+ If offset_within_block_p is true we allow larger offsets. */
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;
+
/* 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] AArch64: Cleanup aarch64_classify_symbol
2021-04-28 17:48 ` Wilco Dijkstra
@ 2021-04-30 8:46 ` Richard Sandiford
2021-04-30 12:11 ` Wilco Dijkstra
0 siblings, 1 reply; 8+ messages in thread
From: Richard Sandiford @ 2021-04-30 8:46 UTC (permalink / raw)
To: Wilco Dijkstra via Gcc-patches; +Cc: Wilco Dijkstra
Wilco Dijkstra via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Hi Richard,
>
>> 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:
>
> It's just to make the code simpler and more orthogonal - the size of the
> sequence and sharing of GOT/literal is identical in both cases.
Hmm, OK. I guess it makes things more consistent in that sense
(PIC vs. non-PIC). But on the other side it's making things less
internally consistent for non-PIC, since we don't use the GOT for
anything else there. I guess in principle there's a danger that a
custom *-elf linker script might not bother to set up the .got properly,
on the assumption that it shouldn't be needed.
But there's not much in it either way, and I agree consistency with
LLVM is a factor too.
> Here is v2 with comments/spaces fixed up:
>
>
> Use a GOT indirection for extern weak symbols instead of a literal - this is the same as
> PIC/PIE and mirrors LLVM behaviour. 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 <wdijkstr@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.
OK, thanks.
Richard
>
> ---
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 82957dddbe42a7f907b2960294ac7f8abf7be2ff..641c83b479e76cbcc75b299eb7ae5f634d9db7cd 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -17854,7 +17854,14 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
>
> switch (aarch64_cmodel)
> {
> + case AARCH64_CMODEL_TINY_PIC:
> case AARCH64_CMODEL_TINY:
> + /* With -fPIC non-local symbols use the GOT. For orthogonality
> + always use the GOT for extern weak symbols. */
> + 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
> @@ -17862,42 +17869,30 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
> symbol + offset is outside the addressible range of +/-1MB in the
> TINY code model. So we limit the maximum offset to +/-64KB and
> assume the offset to the symbol is not larger than +/-(1MB - 64KB).
> - 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. */
> -
> - if (SYMBOL_REF_WEAK (x) && !aarch64_symbol_binds_local_p (x))
> - return SYMBOL_FORCE_TO_MEM;
> + If offset_within_block_p is true we allow larger offsets. */
> 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;
> +
> /* 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] AArch64: Cleanup aarch64_classify_symbol
2021-04-30 8:46 ` Richard Sandiford
@ 2021-04-30 12:11 ` Wilco Dijkstra
2021-04-30 12:50 ` Richard Sandiford
0 siblings, 1 reply; 8+ messages in thread
From: Wilco Dijkstra @ 2021-04-30 12:11 UTC (permalink / raw)
To: Richard Sandiford; +Cc: Wilco Dijkstra via Gcc-patches
Hi Richard,
> Hmm, OK. I guess it makes things more consistent in that sense
> (PIC vs. non-PIC). But on the other side it's making things less
> internally consistent for non-PIC, since we don't use the GOT for
> anything else there. I guess in principle there's a danger that a
> custom *-elf linker script might not bother to set up the .got properly,
> on the assumption that it shouldn't be needed.
The GOT is always used in executables (even an empty main function has
a non-zero .got and .got.plt section). When you need an indirection,
using an existing mechanism is better than doing something special.
For example if the weak reference is resolved to a local symbol, the
linker could remove the indirection.
Cheers,
Wilco
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] AArch64: Cleanup aarch64_classify_symbol
2021-04-30 12:11 ` Wilco Dijkstra
@ 2021-04-30 12:50 ` Richard Sandiford
0 siblings, 0 replies; 8+ messages in thread
From: Richard Sandiford @ 2021-04-30 12:50 UTC (permalink / raw)
To: Wilco Dijkstra via Gcc-patches; +Cc: Wilco Dijkstra
Wilco Dijkstra via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Hi Richard,
>> Hmm, OK. I guess it makes things more consistent in that sense
>> (PIC vs. non-PIC). But on the other side it's making things less
>> internally consistent for non-PIC, since we don't use the GOT for
>> anything else there. I guess in principle there's a danger that a
>> custom *-elf linker script might not bother to set up the .got properly,
>> on the assumption that it shouldn't be needed.
>
> The GOT is always used in executables (even an empty main function has
> a non-zero .got and .got.plt section).
The comment above was talking about *-elf scripts though. When I try
the following locally, no .got is used:
$ echo 'int main() { return 0; }' > foo.c
$ aarch64-none-elf-gcc foo.c -mcmodel=small --specs=aem-validation.specs
$ aarch64-none-elf-readelf --sections ./a.out
Thanks,
Richard
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] AArch64: Cleanup aarch64_classify_symbol
2021-04-28 14:36 [PATCH] AArch64: Cleanup aarch64_classify_symbol Wilco Dijkstra
2021-04-28 15:34 ` Richard Sandiford
@ 2021-04-28 18:04 ` Andrew Pinski
2021-04-28 20:34 ` Wilco Dijkstra
1 sibling, 1 reply; 8+ messages in thread
From: Andrew Pinski @ 2021-04-28 18:04 UTC (permalink / raw)
To: Wilco Dijkstra; +Cc: GCC Patches, Richard Sandiford
On Wed, Apr 28, 2021 at 9:50 AM Wilco Dijkstra via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
> Use a GOT indirection for extern weak symbols instead of a literal - this is the same as
> PIC/PIE and mirrors LLVM behaviour. Ensure PIC/PIE use the same offset limits for symbols
> that don't use the GOT.
I thought that was changed not to use the GOT on purpose.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63874
That is if the symbol is not declared in the TU, then using the GOT is
correct thing to do.
Is the testcase gcc.target/aarch64/pr63874.c still working or is not
testing the correct thing?
Thanks,
Andrew Pinski
>
> Passes bootstrap and regress. OK for commit?
>
> ChangeLog:
> 2021-04-27 Wilco Dijkstra <wdijkstr@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. */
> -
> - 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;
> +
> /* 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] AArch64: Cleanup aarch64_classify_symbol
2021-04-28 18:04 ` Andrew Pinski
@ 2021-04-28 20:34 ` Wilco Dijkstra
0 siblings, 0 replies; 8+ messages in thread
From: Wilco Dijkstra @ 2021-04-28 20:34 UTC (permalink / raw)
To: Andrew Pinski; +Cc: GCC Patches, Richard Sandiford
Hi Andrew,
> I thought that was changed not to use the GOT on purpose.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63874
>
> That is if the symbol is not declared in the TU, then using the GOT is
> correct thing to do.
> Is the testcase gcc.target/aarch64/pr63874.c still working or is not
> testing the correct thing?
That PR was about weak definitions, not references. The exact same
sequence is emitted whether we use the GOT or literal (ADRP+LDR),
hence there is no point in treating the non-PIC case differently.
Cheers,
Wilco
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-04-30 12:51 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-28 14:36 [PATCH] AArch64: Cleanup aarch64_classify_symbol Wilco Dijkstra
2021-04-28 15:34 ` Richard Sandiford
2021-04-28 17:48 ` Wilco Dijkstra
2021-04-30 8:46 ` Richard Sandiford
2021-04-30 12:11 ` Wilco Dijkstra
2021-04-30 12:50 ` Richard Sandiford
2021-04-28 18:04 ` Andrew Pinski
2021-04-28 20:34 ` Wilco Dijkstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).