* [PATCH][AArch64] Fix symbol offset limit
@ 2016-08-23 14:11 Wilco Dijkstra
2016-08-26 10:43 ` Richard Earnshaw (lists)
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2016-08-23 14:11 UTC (permalink / raw)
To: GCC Patches; +Cc: nd
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
Bootstrap OK, updated tests now pass rather than failing with symbol out of range.
OK for commit? As this is a latent bug, OK to backport to GCC6.x?
ChangeLog:
2016-08-23 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index ab609223db7c54f467183db39c4f1ae8b789bfb5..60cc61a095bc144d602597a35b51b9a426c76c69 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9367,24 +9367,22 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
return SYMBOL_SMALL_ABSOLUTE;
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d399a3637ed834ddc4bb429594c4ec229b5c2ea8 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..4b4ec8dab9321026d1fae96d336565a7883c8203 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2016-08-23 14:11 [PATCH][AArch64] Fix symbol offset limit Wilco Dijkstra
@ 2016-08-26 10:43 ` Richard Earnshaw (lists)
2016-08-26 19:07 ` Wilco Dijkstra
0 siblings, 1 reply; 48+ messages in thread
From: Richard Earnshaw (lists) @ 2016-08-26 10:43 UTC (permalink / raw)
To: Wilco Dijkstra, GCC Patches; +Cc: nd
On 23/08/16 15:10, Wilco Dijkstra wrote:
> In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
> This means the offset can use all of the +/-4GB offset, leaving no offset available
> for the symbol itself. This results in relocation overflow and link-time errors
> for simple expressions like &global_char + 0xffffff00.
> To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
> 3GB offset from its references. For the tiny code model use a 64KB offset, allowing
> most of the 1MB range for code/data between the symbol and its references.
>
> Bootstrap OK, updated tests now pass rather than failing with symbol out of range.
So isn't the real bug that we've permitted the user to create an object
that is too large for the data model?
Consider, for example:
char fixed_regs[0x200000000ULL];
char fixed_regs2[100];
int
main()
{
return fixed_regs[0] + fixed_regs2[0];
}
$ gcc -o test -mcmodel=small test2.c
/tmp/ccadJpSk.o: In function `main':
test2.c:(.text+0x10): relocation truncated to fit:
R_AARCH64_ADR_PREL_PG_HI21 against symbol `fixed_regs2' defined in
COMMON section in /tmp/ccadJpSk.o
collect2: error: ld returned 1 exit status
Neither offset is too large, but we still generate relocation errors
when trying to reference fixed_regs2.
R.
>
> OK for commit? As this is a latent bug, OK to backport to GCC6.x?
Not yet. And any back-port would need a PR first.
>
> ChangeLog:
> 2016-08-23 Wilco Dijkstra <wdijkstr@arm.com>
>
> gcc/
> * config/aarch64/aarch64.c (aarch64_classify_symbol):
> Apply reasonable limit to symbol offsets.
>
> testsuite/
> * gcc.target/aarch64/symbol-range.c (foo): Set new limit.
> * gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
>
> --
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index ab609223db7c54f467183db39c4f1ae8b789bfb5..60cc61a095bc144d602597a35b51b9a426c76c69 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -9367,24 +9367,22 @@ aarch64_classify_symbol (rtx x, rtx offset)
> we have no way of knowing the address of symbol at compile time
> so we can't accurately say if the distance between the PC and
> symbol + offset is outside the addressible range of +/-1M in the
> - TINY code model. So we rely on images not being greater than
> - 1M and cap the offset at 1M and anything beyond 1M will have to
> - be loaded using an alternative mechanism. Furthermore if the
> - symbol is a weak reference to something that isn't known to
> - resolve to a symbol in this module, then force to memory. */
> + TINY code model. So we limit the maximum offset to +/-64KB and
> + assume the offset to the symbol is not larger than +/-(1M - 64KB).
> + 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))
> - || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
> + || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
> return SYMBOL_FORCE_TO_MEM;
> return SYMBOL_TINY_ABSOLUTE;
>
> case AARCH64_CMODEL_SMALL:
> /* Same reasoning as the tiny code model, but the offset cap here is
> - 4G. */
> + 1G, allowing +/-3G for the offset to the symbol. */
> if ((SYMBOL_REF_WEAK (x)
> && !aarch64_symbol_binds_local_p (x))
> - || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
> - HOST_WIDE_INT_C (4294967264)))
> + || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
> return SYMBOL_FORCE_TO_MEM;
> return SYMBOL_SMALL_ABSOLUTE;
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
> index d7e46b059e41f2672b3a1da5506fa8944e752e01..d399a3637ed834ddc4bb429594c4ec229b5c2ea8 100644
> --- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
> +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
> @@ -1,12 +1,12 @@
> -/* { dg-do compile } */
> +/* { dg-do link } */
> /* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
>
> -int fixed_regs[0x00200000];
> +char fixed_regs[0x00200000];
>
> int
> -foo()
> +main()
> {
> - return fixed_regs[0x00080000];
> + return fixed_regs[0x000ff000];
> }
>
> /* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> index 6574cf4310430b847e77ea56bf8f20ef312d53e4..4b4ec8dab9321026d1fae96d336565a7883c8203 100644
> --- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> @@ -1,12 +1,12 @@
> -/* { dg-do compile } */
> +/* { dg-do link } */
> /* { dg-options "-O3 -save-temps -mcmodel=small" } */
>
> -int fixed_regs[0x200000000ULL];
> +char fixed_regs[0x200000000ULL];
>
> int
> -foo()
> +main()
> {
> - return fixed_regs[0x100000000ULL];
> + return fixed_regs[0xfffff000ULL];
> }
>
> /* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2016-08-26 10:43 ` Richard Earnshaw (lists)
@ 2016-08-26 19:07 ` Wilco Dijkstra
2016-09-12 15:30 ` [PATCH v2][AArch64] " Wilco Dijkstra
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2016-08-26 19:07 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: nd
Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote:
>
> So isn't the real bug that we've permitted the user to create an object
> that is too large for the data model?
No that's a different issue I'm not trying to address here. The key is that as long
as the start of the symbol is in range, we should be able to link. Due to optimization
the offset may be huge even when the object is tiny, so the offset must be limited.
> Consider, for example:
char fixed_regs[0x200000000ULL];
char fixed_regs2[100];
int
main()
{
return fixed_regs[0] + fixed_regs2[0];
}
> Neither offset is too large, but we still generate relocation errors
> when trying to reference fixed_regs2.
But so would creating a million objects of size 1. The linker could warn about
large objects as well as giving better error messages for relocations that are
out of range. But that's mostly QoI, what we have here is a case where legal
code fails to link due to optimization. The original example is from GCC itself,
the fixed_regs array is small but due to optimization we can end up with
&fixed_regs + 0xffffffff.
Wilco
> ChangeLog:
> 2016-08-23 Wilco Dijkstra <wdijkstr@arm.com>
>
> gcc/
> * config/aarch64/aarch64.c (aarch64_classify_symbol):
> Apply reasonable limit to symbol offsets.
>
> testsuite/
> * gcc.target/aarch64/symbol-range.c (foo): Set new limit.
> * gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
>
> --
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index ab609223db7c54f467183db39c4f1ae8b789bfb5..60cc61a095bc144d602597a35b51b9a426c76c69 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -9367,24 +9367,22 @@ aarch64_classify_symbol (rtx x, rtx offset)
> we have no way of knowing the address of symbol at compile time
> so we can't accurately say if the distance between the PC and
> symbol + offset is outside the addressible range of +/-1M in the
> - TINY code model. So we rely on images not being greater than
> - 1M and cap the offset at 1M and anything beyond 1M will have to
> - be loaded using an alternative mechanism. Furthermore if the
> - symbol is a weak reference to something that isn't known to
> - resolve to a symbol in this module, then force to memory. */
> + TINY code model. So we limit the maximum offset to +/-64KB and
> + assume the offset to the symbol is not larger than +/-(1M - 64KB).
> + 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))
> - || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
> + || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
> return SYMBOL_FORCE_TO_MEM;
> return SYMBOL_TINY_ABSOLUTE;
>
> case AARCH64_CMODEL_SMALL:
> /* Same reasoning as the tiny code model, but the offset cap here is
> - 4G. */
> + 1G, allowing +/-3G for the offset to the symbol. */
> if ((SYMBOL_REF_WEAK (x)
> && !aarch64_symbol_binds_local_p (x))
> - || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
> - HOST_WIDE_INT_C (4294967264)))
> + || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
> return SYMBOL_FORCE_TO_MEM;
> return SYMBOL_SMALL_ABSOLUTE;
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
> index d7e46b059e41f2672b3a1da5506fa8944e752e01..d399a3637ed834ddc4bb429594c4ec229b5c2ea8 100644
> --- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
> +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
> @@ -1,12 +1,12 @@
> -/* { dg-do compile } */
> +/* { dg-do link } */
> /* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
>
> -int fixed_regs[0x00200000];
> +char fixed_regs[0x00200000];
>
> int
> -foo()
> +main()
> {
> - return fixed_regs[0x00080000];
> + return fixed_regs[0x000ff000];
> }
>
> /* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> index 6574cf4310430b847e77ea56bf8f20ef312d53e4..4b4ec8dab9321026d1fae96d336565a7883c8203 100644
> --- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> @@ -1,12 +1,12 @@
> -/* { dg-do compile } */
> +/* { dg-do link } */
> /* { dg-options "-O3 -save-temps -mcmodel=small" } */
>
> -int fixed_regs[0x200000000ULL];
> +char fixed_regs[0x200000000ULL];
>
> int
> -foo()
> +main()
> {
> - return fixed_regs[0x100000000ULL];
> + return fixed_regs[0xfffff000ULL];
> }
>
> /* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2][AArch64] Fix symbol offset limit
2016-08-26 19:07 ` Wilco Dijkstra
@ 2016-09-12 15:30 ` Wilco Dijkstra
2016-09-21 14:48 ` Wilco Dijkstra
` (2 more replies)
0 siblings, 3 replies; 48+ messages in thread
From: Wilco Dijkstra @ 2016-09-12 15:30 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: nd
Wilco wrote:
> The original example is from GCC itself, the fixed_regs array is small but due to
> optimization we can end up with &fixed_regs + 0xffffffff.
We could also check the bounds of each symbol if they exist, like the patch below.
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2016-09-12 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 385bd560fb12cd5d404e6ddb2f01edf1fe72d729..275a828ac9e6e9b8187380c1b602ffb1b2bcfb21 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9351,6 +9351,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9359,25 +9361,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d399a3637ed834ddc4bb429594c4ec229b5c2ea8 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..4b4ec8dab9321026d1fae96d336565a7883c8203 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2][AArch64] Fix symbol offset limit
2016-09-12 15:30 ` [PATCH v2][AArch64] " Wilco Dijkstra
@ 2016-09-21 14:48 ` Wilco Dijkstra
2016-10-17 12:42 ` Wilco Dijkstra
2016-12-06 15:07 ` Wilco Dijkstra
2 siblings, 0 replies; 48+ messages in thread
From: Wilco Dijkstra @ 2016-09-21 14:48 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: nd
ping
Wilco wrote:
> The original example is from GCC itself, the fixed_regs array is small but due to
> optimization we can end up with &fixed_regs + 0xffffffff.
We could also check the bounds of each symbol if they exist, like the patch below.
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2016-09-12 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 385bd560fb12cd5d404e6ddb2f01edf1fe72d729..275a828ac9e6e9b8187380c1b602ffb1b2bcfb21 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9351,6 +9351,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9359,25 +9361,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d399a3637ed834ddc4bb429594c4ec229b5c2ea8 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..4b4ec8dab9321026d1fae96d336565a7883c8203 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2][AArch64] Fix symbol offset limit
2016-09-12 15:30 ` [PATCH v2][AArch64] " Wilco Dijkstra
2016-09-21 14:48 ` Wilco Dijkstra
@ 2016-10-17 12:42 ` Wilco Dijkstra
2016-10-25 9:47 ` Wilco Dijkstra
2016-12-06 15:07 ` Wilco Dijkstra
2 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2016-10-17 12:42 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 12 September 2016 15:50
To: Richard Earnshaw; GCC Patches
Cc: nd
Subject: Re: [PATCH v2][AArch64] Fix symbol offset limit
Wilco wrote:
> The original example is from GCC itself, the fixed_regs array is small but due to
> optimization we can end up with &fixed_regs + 0xffffffff.
We could also check the bounds of each symbol if they exist, like the patch below.
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2016-09-12 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 385bd560fb12cd5d404e6ddb2f01edf1fe72d729..275a828ac9e6e9b8187380c1b602ffb1b2bcfb21 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9351,6 +9351,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9359,25 +9361,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d399a3637ed834ddc4bb429594c4ec229b5c2ea8 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..4b4ec8dab9321026d1fae96d336565a7883c8203 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2][AArch64] Fix symbol offset limit
2016-10-17 12:42 ` Wilco Dijkstra
@ 2016-10-25 9:47 ` Wilco Dijkstra
2016-11-02 16:48 ` Wilco Dijkstra
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2016-10-25 9:47 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 12 September 2016 15:50
To: Richard Earnshaw; GCC Patches
Cc: nd
Subject: Re: [PATCH v2][AArch64] Fix symbol offset limit
Wilco wrote:
> The original example is from GCC itself, the fixed_regs array is small but due to
> optimization we can end up with &fixed_regs + 0xffffffff.
We could also check the bounds of each symbol if they exist, like the patch below.
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2016-09-12 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 385bd560fb12cd5d404e6ddb2f01edf1fe72d729..275a828ac9e6e9b8187380c1b602ffb1b2bcfb21 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9351,6 +9351,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9359,25 +9361,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d399a3637ed834ddc4bb429594c4ec229b5c2ea8 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..4b4ec8dab9321026d1fae96d336565a7883c8203 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2][AArch64] Fix symbol offset limit
2016-10-25 9:47 ` Wilco Dijkstra
@ 2016-11-02 16:48 ` Wilco Dijkstra
2016-11-14 13:07 ` Wilco Dijkstra
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2016-11-02 16:48 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 12 September 2016 15:50
To: Richard Earnshaw; GCC Patches
Cc: nd
Subject: Re: [PATCH v2][AArch64] Fix symbol offset limit
Wilco wrote:
> The original example is from GCC itself, the fixed_regs array is small but due to
> optimization we can end up with &fixed_regs + 0xffffffff.
We could also check the bounds of each symbol if they exist, like the patch below.
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2016-09-12 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 385bd560fb12cd5d404e6ddb2f01edf1fe72d729..275a828ac9e6e9b8187380c1b602ffb1b2bcfb21 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9351,6 +9351,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9359,25 +9361,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d399a3637ed834ddc4bb429594c4ec229b5c2ea8 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..4b4ec8dab9321026d1fae96d336565a7883c8203 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2][AArch64] Fix symbol offset limit
2016-11-02 16:48 ` Wilco Dijkstra
@ 2016-11-14 13:07 ` Wilco Dijkstra
0 siblings, 0 replies; 48+ messages in thread
From: Wilco Dijkstra @ 2016-11-14 13:07 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 12 September 2016 15:50
To: Richard Earnshaw; GCC Patches
Cc: nd
Subject: Re: [PATCH v2][AArch64] Fix symbol offset limit
Wilco wrote:
> The original example is from GCC itself, the fixed_regs array is small but due to
> optimization we can end up with &fixed_regs + 0xffffffff.
We could also check the bounds of each symbol if they exist, like the patch below.
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2016-09-12 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 385bd560fb12cd5d404e6ddb2f01edf1fe72d729..275a828ac9e6e9b8187380c1b602ffb1b2bcfb21 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9351,6 +9351,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9359,25 +9361,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d399a3637ed834ddc4bb429594c4ec229b5c2ea8 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..4b4ec8dab9321026d1fae96d336565a7883c8203 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2][AArch64] Fix symbol offset limit
2016-09-12 15:30 ` [PATCH v2][AArch64] " Wilco Dijkstra
2016-09-21 14:48 ` Wilco Dijkstra
2016-10-17 12:42 ` Wilco Dijkstra
@ 2016-12-06 15:07 ` Wilco Dijkstra
2017-01-17 15:14 ` [PATCH v3][AArch64] " Wilco Dijkstra
2 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2016-12-06 15:07 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches, James Greenhalgh; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 12 September 2016 15:50
To: Richard Earnshaw; GCC Patches
Cc: nd
Subject: Re: [PATCH v2][AArch64] Fix symbol offset limit
Wilco wrote:
> The original example is from GCC itself, the fixed_regs array is small but due to
> optimization we can end up with &fixed_regs + 0xffffffff.
We could also check the bounds of each symbol if they exist, like the patch below.
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2016-09-12 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 385bd560fb12cd5d404e6ddb2f01edf1fe72d729..275a828ac9e6e9b8187380c1b602ffb1b2bcfb21 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9351,6 +9351,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9359,25 +9361,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d399a3637ed834ddc4bb429594c4ec229b5c2ea8 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..4b4ec8dab9321026d1fae96d336565a7883c8203 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2016-12-06 15:07 ` Wilco Dijkstra
@ 2017-01-17 15:14 ` Wilco Dijkstra
2017-02-02 14:44 ` Wilco Dijkstra
2017-04-20 16:03 ` Wilco Dijkstra
0 siblings, 2 replies; 48+ messages in thread
From: Wilco Dijkstra @ 2017-01-17 15:14 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches, James Greenhalgh; +Cc: nd
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-01-17 15:14 ` [PATCH v3][AArch64] " Wilco Dijkstra
@ 2017-02-02 14:44 ` Wilco Dijkstra
2017-02-23 16:58 ` Wilco Dijkstra
2017-04-20 16:03 ` Wilco Dijkstra
1 sibling, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2017-02-02 14:44 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches, James Greenhalgh; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-02-02 14:44 ` Wilco Dijkstra
@ 2017-02-23 16:58 ` Wilco Dijkstra
0 siblings, 0 replies; 48+ messages in thread
From: Wilco Dijkstra @ 2017-02-23 16:58 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches, James Greenhalgh; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-01-17 15:14 ` [PATCH v3][AArch64] " Wilco Dijkstra
2017-02-02 14:44 ` Wilco Dijkstra
@ 2017-04-20 16:03 ` Wilco Dijkstra
2017-06-13 14:00 ` Wilco Dijkstra
1 sibling, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2017-04-20 16:03 UTC (permalink / raw)
To: GCC Patches, James Greenhalgh; +Cc: nd, Richard Earnshaw
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-04-20 16:03 ` Wilco Dijkstra
@ 2017-06-13 14:00 ` Wilco Dijkstra
2017-06-14 14:07 ` James Greenhalgh
2017-06-27 15:36 ` Wilco Dijkstra
0 siblings, 2 replies; 48+ messages in thread
From: Wilco Dijkstra @ 2017-06-13 14:00 UTC (permalink / raw)
To: GCC Patches, James Greenhalgh; +Cc: nd, Richard Earnshaw
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-13 14:00 ` Wilco Dijkstra
@ 2017-06-14 14:07 ` James Greenhalgh
2017-06-14 16:03 ` Wilco Dijkstra
2017-06-15 15:13 ` Richard Earnshaw (lists)
2017-06-27 15:36 ` Wilco Dijkstra
1 sibling, 2 replies; 48+ messages in thread
From: James Greenhalgh @ 2017-06-14 14:07 UTC (permalink / raw)
To: Wilco Dijkstra; +Cc: GCC Patches, nd, Richard Earnshaw, marcus.shawcroft
On Tue, Jun 13, 2017 at 03:00:28PM +0100, Wilco Dijkstra wrote:
>
> ping
I've been avoiding reviewing this patch as Richard was the last to comment
on it, and I wasn't sure that his comments had been resolved to his
satisfaction. The conversation was back in August 2016 on v1 of the patch:
> Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote:
> >
> > So isn't the real bug that we've permitted the user to create an object
> > that is too large for the data model?
>
> No that's a different issue I'm not trying to address here. The key is that as long
> as the start of the symbol is in range, we should be able to link. Due to optimization
> the offset may be huge even when the object is tiny, so the offset must be limited.
>
> > Consider, for example:
>
> char fixed_regs[0x200000000ULL];
> char fixed_regs2[100];
>
> int
> main()
> {
> return fixed_regs[0] + fixed_regs2[0];
> }
>
> > Neither offset is too large, but we still generate relocation errors
> > when trying to reference fixed_regs2.
>
> But so would creating a million objects of size 1. The linker could warn about
> large objects as well as giving better error messages for relocations that are
> out of range. But that's mostly QoI, what we have here is a case where legal
> code fails to link due to optimization. The original example is from GCC itself,
> the fixed_regs array is small but due to optimization we can end up with
> &fixed_regs + 0xffffffff.
Richard, do you have anything further to say on this patch? Or can we start
progressing the review again.
Thanks,
James
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-14 14:07 ` James Greenhalgh
@ 2017-06-14 16:03 ` Wilco Dijkstra
2017-06-15 15:13 ` Richard Earnshaw (lists)
1 sibling, 0 replies; 48+ messages in thread
From: Wilco Dijkstra @ 2017-06-14 16:03 UTC (permalink / raw)
To: James Greenhalgh; +Cc: GCC Patches, nd, Richard Earnshaw, Marcus Shawcroft
Hi,
Let's get back to the patch and the bug it fixes. The only outstanding question is what
constant offsets we should allow when generating a relocation:
> So the question is whether we should allow
> largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
> small offsets (small negative and positive offsets just outside a symbol are common).
> The only thing we can't allow is any offset like we currently do...
Wilco
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-14 14:07 ` James Greenhalgh
2017-06-14 16:03 ` Wilco Dijkstra
@ 2017-06-15 15:13 ` Richard Earnshaw (lists)
2017-06-15 16:55 ` Wilco Dijkstra
1 sibling, 1 reply; 48+ messages in thread
From: Richard Earnshaw (lists) @ 2017-06-15 15:13 UTC (permalink / raw)
To: James Greenhalgh, Wilco Dijkstra; +Cc: GCC Patches, nd, marcus.shawcroft
On 14/06/17 15:07, James Greenhalgh wrote:
> On Tue, Jun 13, 2017 at 03:00:28PM +0100, Wilco Dijkstra wrote:
>>
>> ping
>
> I've been avoiding reviewing this patch as Richard was the last to comment
> on it, and I wasn't sure that his comments had been resolved to his
> satisfaction. The conversation was back in August 2016 on v1 of the patch:
>
>> Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote:
>>>
>>> So isn't the real bug that we've permitted the user to create an object
>>> that is too large for the data model?
>>
>> No that's a different issue I'm not trying to address here. The key is that as long
>> as the start of the symbol is in range, we should be able to link. Due to optimization
>> the offset may be huge even when the object is tiny, so the offset must be limited.
>>
>>> Consider, for example:
>>
>> char fixed_regs[0x200000000ULL];
>> char fixed_regs2[100];
>>
>> int
>> main()
>> {
>> return fixed_regs[0] + fixed_regs2[0];
>> }
>>
>>> Neither offset is too large, but we still generate relocation errors
>>> when trying to reference fixed_regs2.
>>
>> But so would creating a million objects of size 1. The linker could warn about
>> large objects as well as giving better error messages for relocations that are
>> out of range. But that's mostly QoI, what we have here is a case where legal
>> code fails to link due to optimization. The original example is from GCC itself,
>> the fixed_regs array is small but due to optimization we can end up with
>> &fixed_regs + 0xffffffff.
>
> Richard, do you have anything further to say on this patch? Or can we start
> progressing the review again.
>
> Thanks,
> James
>
Yes, I still believe that this is a bug in the way we've documented the
-mcmodel=tiny and -mcmodel=small options.
-mcmode=tiny should read:
@item -mcmodel=tiny
@opindex mcmodel=tiny
Generate code for the tiny code model. The program and its static data
must fit within 1MB. Programs can be statically or dynamically linked.
The limit is not enforced by the compiler, but if you exceed the limit
you may get errors during linking saying that relocations have been
truncated.
It's the same basic text for -mcmodel=small, except that the limit is 4GB.
R.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 15:13 ` Richard Earnshaw (lists)
@ 2017-06-15 16:55 ` Wilco Dijkstra
2017-06-15 17:39 ` Richard Earnshaw (lists)
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2017-06-15 16:55 UTC (permalink / raw)
To: Richard Earnshaw, James Greenhalgh; +Cc: GCC Patches, nd, Marcus Shawcroft
Richard Earnshaw wrote:
> Yes, I still believe that this is a bug in the way we've documented the
> -mcmodel=tiny and -mcmodel=small options.
In what way could this possibly be a documentation bug? It's not at all related
to the size of a binary. There is no limit to the offset you can apply to a symbol,
I can write int a; int *p = &a + 0x80000000; and GCC is happy to create a
relocation with that offset.
Wilco
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 16:55 ` Wilco Dijkstra
@ 2017-06-15 17:39 ` Richard Earnshaw (lists)
2017-06-15 17:51 ` Wilco Dijkstra
0 siblings, 1 reply; 48+ messages in thread
From: Richard Earnshaw (lists) @ 2017-06-15 17:39 UTC (permalink / raw)
To: Wilco Dijkstra, James Greenhalgh; +Cc: GCC Patches, nd, Marcus Shawcroft
On 15/06/17 17:55, Wilco Dijkstra wrote:
> Richard Earnshaw wrote:
>> Yes, I still believe that this is a bug in the way we've documented the
>> -mcmodel=tiny and -mcmodel=small options.
>
> In what way could this possibly be a documentation bug? It's not at all
> related
> to the size of a binary. There is no limit to the offset you can apply
> to a symbol,
> I can write int a; int *p = &a + 0x80000000; and GCC is happy to create a
> relocation with that offset.
>
> Wilco
You can write it, but it's meaningless by the C standard. You can't
take the address beyond one after the size of the object, so anything
more than &a+1 has no meaning.
R.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 17:39 ` Richard Earnshaw (lists)
@ 2017-06-15 17:51 ` Wilco Dijkstra
2017-06-15 18:11 ` Richard Earnshaw (lists)
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2017-06-15 17:51 UTC (permalink / raw)
To: Richard Earnshaw, James Greenhalgh; +Cc: GCC Patches, nd, Marcus Shawcroft
Richard Earnshaw wrote:
>
> You can write it, but it's meaningless by the C standard. You can't
> take the address beyond one after the size of the object, so anything
> more than &a+1 has no meaning.
No it's perfectly valid and such out-of-range cases occur thousands of
times when building any non-trivial code. For example a[i + C] transforms
into (&a + C)[i].
Wilco
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 17:51 ` Wilco Dijkstra
@ 2017-06-15 18:11 ` Richard Earnshaw (lists)
2017-06-15 18:18 ` Wilco Dijkstra
0 siblings, 1 reply; 48+ messages in thread
From: Richard Earnshaw (lists) @ 2017-06-15 18:11 UTC (permalink / raw)
To: Wilco Dijkstra, James Greenhalgh; +Cc: GCC Patches, nd, Marcus Shawcroft
On 15/06/17 18:51, Wilco Dijkstra wrote:
> Richard Earnshaw wrote:
>>
>> You can write it, but it's meaningless by the C standard. You can't
>> take the address beyond one after the size of the object, so anything
>> more than &a+1 has no meaning.
>
> No it's perfectly valid and such out-of-range cases occur thousands of
> times when building any non-trivial code. For example a[i + C] transforms
> into (&a + C)[i].
>
> Wilco
>
>
C11: Summary of undefined behaviours.
— Addition or subtraction of a pointer into, or just beyond, an array
object and an
integer type produces a result that does not point into, or just beyond,
the same array
object (6.5.6).
R.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 18:11 ` Richard Earnshaw (lists)
@ 2017-06-15 18:18 ` Wilco Dijkstra
2017-06-15 18:34 ` Richard Earnshaw (lists)
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2017-06-15 18:18 UTC (permalink / raw)
To: Richard Earnshaw, James Greenhalgh; +Cc: GCC Patches, nd, Marcus Shawcroft
Richard Earnshaw wrote:
C11: Summary of undefined behaviours.
— Addition or subtraction of a pointer into, or just beyond, an array
object and an
integer type produces a result that does not point into, or just beyond,
the same array
object (6.5.6).
That's totally irrelevant given the addition is created by the optimizer.
Wilco
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 18:18 ` Wilco Dijkstra
@ 2017-06-15 18:34 ` Richard Earnshaw (lists)
2017-06-15 18:55 ` Wilco Dijkstra
0 siblings, 1 reply; 48+ messages in thread
From: Richard Earnshaw (lists) @ 2017-06-15 18:34 UTC (permalink / raw)
To: Wilco Dijkstra, James Greenhalgh; +Cc: GCC Patches, nd, Marcus Shawcroft
On 15/06/17 19:18, Wilco Dijkstra wrote:
> Richard Earnshaw wrote:
>
> C11: Summary of undefined behaviours.
>
> — Addition or subtraction of a pointer into, or just beyond, an array
> object and an
> integer type produces a result that does not point into, or just beyond,
> the same array
> object (6.5.6).
>
> That's totally irrelevant given the addition is created by the optimizer.
>
> Wilco
>
No it's not. The optimizer doesn't create totally random bases. If the
code + data is less than 1M in size, then any offsets it does create
will fit within the size of the relocations selected by the compiler.
R.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 18:34 ` Richard Earnshaw (lists)
@ 2017-06-15 18:55 ` Wilco Dijkstra
2017-06-15 19:52 ` Joseph Myers
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2017-06-15 18:55 UTC (permalink / raw)
To: Richard Earnshaw, James Greenhalgh; +Cc: GCC Patches, nd, Marcus Shawcroft
Richard Earnshaw wrote:
> No it's not. The optimizer doesn't create totally random bases. If the
> code + data is less than 1M in size, then any offsets it does create
> will fit within the size of the relocations selected by the compiler.
No that's completely false. There is no way you can guarantee that without
my patch. My patch is precisely there to ensure we only allow offsets that
guarantee linking succeeds if all code and data fits in 1M or 4GB.
Even then there are many cases where we don't know the size of an object and
so have to constrain the offset to something conservative (which cannot be
the full range it allows today).
Wilco
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 18:55 ` Wilco Dijkstra
@ 2017-06-15 19:52 ` Joseph Myers
2017-06-16 15:14 ` Nathan Sidwell
0 siblings, 1 reply; 48+ messages in thread
From: Joseph Myers @ 2017-06-15 19:52 UTC (permalink / raw)
To: Wilco Dijkstra
Cc: Richard Earnshaw, James Greenhalgh, GCC Patches, nd, Marcus Shawcroft
[-- Attachment #1: Type: text/plain, Size: 936 bytes --]
On Thu, 15 Jun 2017, Wilco Dijkstra wrote:
> Richard Earnshaw wrote:
> > No it's not. The optimizer doesn't create totally random bases. If the
> > code + data is less than 1M in size, then any offsets it does create
> > will fit within the size of the relocations selected by the compiler.
>
> No that's completely false. There is no way you can guarantee that without
> my patch. My patch is precisely there to ensure we only allow offsets that
> guarantee linking succeeds if all code and data fits in 1M or 4GB.
For example, given (array + (i - INT_MAX)), it's quite possible the
compiler could create a relocation for array - INT_MAX, and the original
expression is perfectly OK if i == INT_MAX even though array - INT_MAX is
far out of range. (And array - INT_MAX as a C expression is only
undefined at runtime, not at compile time if it's in code that is never
executed.)
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 19:52 ` Joseph Myers
@ 2017-06-16 15:14 ` Nathan Sidwell
0 siblings, 0 replies; 48+ messages in thread
From: Nathan Sidwell @ 2017-06-16 15:14 UTC (permalink / raw)
To: Joseph Myers, Wilco Dijkstra
Cc: Richard Earnshaw, James Greenhalgh, GCC Patches, nd, Marcus Shawcroft
On 06/15/2017 03:52 PM, Joseph Myers wrote:
> For example, given (array + (i - INT_MAX)), it's quite possible the
> compiler could create a relocation for array - INT_MAX, and the original
> expression is perfectly OK if i == INT_MAX even though array - INT_MAX is
> far out of range. (And array - INT_MAX as a C expression is only
> undefined at runtime, not at compile time if it's in code that is never
> executed.)
Some targets (typically uclinux-like things) cannot support this, as
they move the data segment relative to the text segment upon loading,
and need to know whether an address is text-relative or data-relative
(and symbol information may not be available).
There's a target hook for that, but I can't find it now.
nathan
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-13 14:00 ` Wilco Dijkstra
2017-06-14 14:07 ` James Greenhalgh
@ 2017-06-27 15:36 ` Wilco Dijkstra
2017-07-14 14:28 ` Wilco Dijkstra
1 sibling, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2017-06-27 15:36 UTC (permalink / raw)
To: GCC Patches, James Greenhalgh; +Cc: nd, Richard Earnshaw
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-27 15:36 ` Wilco Dijkstra
@ 2017-07-14 14:28 ` Wilco Dijkstra
2017-07-21 11:23 ` Wilco Dijkstra
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2017-07-14 14:28 UTC (permalink / raw)
To: GCC Patches, James Greenhalgh; +Cc: nd, Richard Earnshaw
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-07-14 14:28 ` Wilco Dijkstra
@ 2017-07-21 11:23 ` Wilco Dijkstra
2017-08-01 10:19 ` Wilco Dijkstra
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2017-07-21 11:23 UTC (permalink / raw)
To: GCC Patches, James Greenhalgh; +Cc: nd, Richard Earnshaw
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-07-21 11:23 ` Wilco Dijkstra
@ 2017-08-01 10:19 ` Wilco Dijkstra
2017-08-15 17:36 ` Wilco Dijkstra
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2017-08-01 10:19 UTC (permalink / raw)
To: GCC Patches, James Greenhalgh; +Cc: nd, Richard Earnshaw
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-08-01 10:19 ` Wilco Dijkstra
@ 2017-08-15 17:36 ` Wilco Dijkstra
0 siblings, 0 replies; 48+ messages in thread
From: Wilco Dijkstra @ 2017-08-15 17:36 UTC (permalink / raw)
To: GCC Patches, James Greenhalgh; +Cc: nd, Richard Earnshaw
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ 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))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2019-10-15 18:27 ` Wilco Dijkstra
@ 2019-10-16 8:50 ` Richard Sandiford
0 siblings, 0 replies; 48+ messages in thread
From: Richard Sandiford @ 2019-10-16 8:50 UTC (permalink / raw)
To: Wilco Dijkstra
Cc: GCC Patches, Ramana Radhakrishnan, Richard Earnshaw,
James Greenhalgh, Kyrylo Tkachov, nd
Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> Hi Richard,
>> Sure, the "extern array of unknown size" case isn't about section anchors.
>> But this part of my message (snipped above) was about the other case
>> (objects of known size), and applied to individual objects as well as
>> section anchors.
>>
>> What I was trying to say is: yes, we need better offsets for references
>> to extern objects of unknown size. But your patch does that by reducing
>> the offset range for all objects, including ones with known sizes in
>> which the documented ranges should be safe.
>>
>> I was trying to explain why I don't think we need to reduce the range
>> in that case too. If offset_within_block_p then any offset should be
>> OK (the aggressive interpretation) or the original documented ranges
>> should be OK. I think we only need the smaller range when
>> offset_within_block_p returns false.
>
> Right I see now. Yes given offset_within_block_p is not sufficient, we could test
> both conditions. Here is the updated patch:
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 7ee31a66b12d7354759f06449955e933421f5fe0..31c394a7e8567dd7d4f1698e5ba98aeb8807df38 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -14079,26 +14079,30 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
> the offset does not cause overflow of the final address. But
> we have no way of knowing the address of symbol at compile time
> so we can't accurately say if the distance between the PC and
> - symbol + offset is outside the addressible range of +/-1M in the
> - TINY code model. So we rely on images not being greater than
> - 1M and cap the offset at 1M and anything beyond 1M will have to
> - be loaded using an alternative mechanism. Furthermore if the
> - symbol is a weak reference to something that isn't known to
> - resolve to a symbol in this module, then force to memory. */
> - if ((SYMBOL_REF_WEAK (x)
> - && !aarch64_symbol_binds_local_p (x))
> - || !IN_RANGE (offset, -1048575, 1048575))
> + 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).
> + 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:
> /* Same reasoning as the tiny code model, but the offset cap here is
> - 4G. */
> - if ((SYMBOL_REF_WEAK (x)
> - && !aarch64_symbol_binds_local_p (x))
> - || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
> - HOST_WIDE_INT_C (4294967264)))
> + 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;
Think this is just the mailer eating tabs, but: || not properly indented.
OK otherwise, thanks.
Richard
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2019-10-14 16:34 ` Richard Sandiford
@ 2019-10-15 18:27 ` Wilco Dijkstra
2019-10-16 8:50 ` Richard Sandiford
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2019-10-15 18:27 UTC (permalink / raw)
To: Richard Sandiford
Cc: GCC Patches, Ramana Radhakrishnan, Richard Earnshaw,
James Greenhalgh, Kyrylo Tkachov, nd
Hi Richard,
> Sure, the "extern array of unknown size" case isn't about section anchors.
> But this part of my message (snipped above) was about the other case
> (objects of known size), and applied to individual objects as well as
> section anchors.
>
> What I was trying to say is: yes, we need better offsets for references
> to extern objects of unknown size. But your patch does that by reducing
> the offset range for all objects, including ones with known sizes in
> which the documented ranges should be safe.
>
> I was trying to explain why I don't think we need to reduce the range
> in that case too. If offset_within_block_p then any offset should be
> OK (the aggressive interpretation) or the original documented ranges
> should be OK. I think we only need the smaller range when
> offset_within_block_p returns false.
Right I see now. Yes given offset_within_block_p is not sufficient, we could test
both conditions. Here is the updated patch:
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 7ee31a66b12d7354759f06449955e933421f5fe0..31c394a7e8567dd7d4f1698e5ba98aeb8807df38 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -14079,26 +14079,30 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
the offset does not cause overflow of the final address. But
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
- symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
- if ((SYMBOL_REF_WEAK (x)
- && !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (offset, -1048575, 1048575))
+ 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).
+ 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:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
- if ((SYMBOL_REF_WEAK (x)
- && !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ 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:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..fc6a4f3ec780d9fa86de1c8e1a42a55992ee8b2d 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00080000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..d8e82fa1b2829fd300b6ccf7f80241e5573e7e17 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x80000000];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2019-10-14 15:51 ` Wilco Dijkstra
@ 2019-10-14 16:34 ` Richard Sandiford
2019-10-15 18:27 ` Wilco Dijkstra
0 siblings, 1 reply; 48+ messages in thread
From: Richard Sandiford @ 2019-10-14 16:34 UTC (permalink / raw)
To: Wilco Dijkstra
Cc: GCC Patches, Ramana Radhakrishnan, Richard Earnshaw,
James Greenhalgh, Kyrylo Tkachov, nd
Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> Hi Richard,
>
>>> No - the testcases fail with that.
>>
>> Hmm, OK. Could you give more details? What does the motivating case
>> actually look like?
>
> Well it's now a very long time ago since I first posted this patch but the failure
> was in SPEC. It did something like &array[0xffffff000 - x], presumably after some
> optimization with some specific options I was using at the time. The exact details
> don't matter since I've got minimal testcases.
>
>> One of the reasons this is a hard patch to review is that it doesn't
>> include a testcase for the kind of situation it's trying to fix.
>
> There is a very simple testcase which fails before and passes with my patch.
>
>>> It also reduces codequality by not allowing commonly used offsets as
>>> part of the symbol relocation.
>>
>> Which kinds of cases specifically? Although I'm guessing the problem is
>> when indexing into external arrays of unknown size.
>
> Well there are 41000 uses in SPEC2017 that fail the offset_within_block_p
> test but pass my range test. There are 3 cases where the reverse is true
> (a huge offset: 17694720).
>
> Overall my range test allows 99.99% of the offsets, so we can safely conclude
> my patch doesn't regress any existing code.
>
>> So IMO we should be able to assume that the start and end + 1 addresses
>> of any referenced object are within reach. For section anchors, we can
>> extend that to the containing anchor block, which is really just a
>> super-sized object.
>
> This isn't about section anchors - in many cases the array is an extern.
Sure, the "extern array of unknown size" case isn't about section anchors.
But this part of my message (snipped above) was about the other case
(objects of known size), and applied to individual objects as well as
section anchors.
What I was trying to say is: yes, we need better offsets for references
to extern objects of unknown size. But your patch does that by reducing
the offset range for all objects, including ones with known sizes in
which the documented ranges should be safe.
I was trying to explain why I don't think we need to reduce the range
in that case too. If offset_within_block_p then any offset should be
OK (the aggressive interpretation) or the original documented ranges
should be OK. I think we only need the smaller range when
offset_within_block_p returns false.
>> The question then is what to do about symbols whose size isn't known.
>> And I agree that unconditionally applying the full code-model offset
>> range is too aggressive in that case.
>
> That's very common indeed, so we need to apply some kind of reasonable
> range check.
>
>> Well, for one thing, if code quality isn't affected by using +/-64k
>> for the tiny model, do we have any evidence that we need a larger offset
>> for the other code models?
>
> Certainly for SPEC +-64KB is too small, but SPEC won't build in the tiny code
> model. For the tiny code model the emphasis should be on ensuring that code
> that fits should build correctly rather than trying to optimize it to the max and
> getting relocations that are out of range... So I believe it is reasonable to use a
> more conservative range in the tiny model.
>
>> But more importantly, we can't say definitively that code quality isn't
>> affected, only that it wasn't affected for the cases we've looked at.
>> People could patch the compiler if the new ranges turn out not to strike
>> the right balance for their use cases, but not everyone wants to do that.
>
> Well if we're worried about codequality then the offset in block approach
> affects it the most. Offsets larger than 1MB are extremely rare, so the chance
> that there will ever be a request for a larger range is simply zero.
>
>> Maybe we need a new command-line option.
>
> That's way overkill... All this analysis is overcomplicating what is really a very
> basic problem with a very simple solution.
Well, I'm not going to object if another maintainer is happy to approve
the patch as-is.
Richard
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2019-10-12 10:20 ` Richard Sandiford
2019-10-12 11:23 ` Richard Sandiford
2019-10-13 8:45 ` Jeff Law
@ 2019-10-14 15:51 ` Wilco Dijkstra
2019-10-14 16:34 ` Richard Sandiford
2 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2019-10-14 15:51 UTC (permalink / raw)
To: Richard Sandiford
Cc: GCC Patches, Ramana Radhakrishnan, Richard Earnshaw,
James Greenhalgh, Kyrylo Tkachov, nd
Hi Richard,
>> No - the testcases fail with that.
>
> Hmm, OK. Could you give more details? What does the motivating case
> actually look like?
Well it's now a very long time ago since I first posted this patch but the failure
was in SPEC. It did something like &array[0xffffff000 - x], presumably after some
optimization with some specific options I was using at the time. The exact details
don't matter since I've got minimal testcases.
> One of the reasons this is a hard patch to review is that it doesn't
> include a testcase for the kind of situation it's trying to fix.
There is a very simple testcase which fails before and passes with my patch.
>> It also reduces codequality by not allowing commonly used offsets as
>> part of the symbol relocation.
>
> Which kinds of cases specifically? Although I'm guessing the problem is
> when indexing into external arrays of unknown size.
Well there are 41000 uses in SPEC2017 that fail the offset_within_block_p
test but pass my range test. There are 3 cases where the reverse is true
(a huge offset: 17694720).
Overall my range test allows 99.99% of the offsets, so we can safely conclude
my patch doesn't regress any existing code.
> So IMO we should be able to assume that the start and end + 1 addresses
> of any referenced object are within reach. For section anchors, we can
> extend that to the containing anchor block, which is really just a
> super-sized object.
This isn't about section anchors - in many cases the array is an extern.
> The question then is what to do about symbols whose size isn't known.
> And I agree that unconditionally applying the full code-model offset
> range is too aggressive in that case.
That's very common indeed, so we need to apply some kind of reasonable
range check.
> Well, for one thing, if code quality isn't affected by using +/-64k
> for the tiny model, do we have any evidence that we need a larger offset
> for the other code models?
Certainly for SPEC +-64KB is too small, but SPEC won't build in the tiny code
model. For the tiny code model the emphasis should be on ensuring that code
that fits should build correctly rather than trying to optimize it to the max and
getting relocations that are out of range... So I believe it is reasonable to use a
more conservative range in the tiny model.
> But more importantly, we can't say definitively that code quality isn't
> affected, only that it wasn't affected for the cases we've looked at.
> People could patch the compiler if the new ranges turn out not to strike
> the right balance for their use cases, but not everyone wants to do that.
Well if we're worried about codequality then the offset in block approach
affects it the most. Offsets larger than 1MB are extremely rare, so the chance
that there will ever be a request for a larger range is simply zero.
> Maybe we need a new command-line option.
That's way overkill... All this analysis is overcomplicating what is really a very
basic problem with a very simple solution.
Cheers,
Wilco
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2019-10-12 10:20 ` Richard Sandiford
2019-10-12 11:23 ` Richard Sandiford
@ 2019-10-13 8:45 ` Jeff Law
2019-10-14 15:51 ` Wilco Dijkstra
2 siblings, 0 replies; 48+ messages in thread
From: Jeff Law @ 2019-10-13 8:45 UTC (permalink / raw)
To: Wilco Dijkstra, GCC Patches, Ramana Radhakrishnan,
Richard Earnshaw, James Greenhalgh, Kyrylo Tkachov, nd,
richard.sandiford
On 10/12/19 3:56 AM, Richard Sandiford wrote:
> Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
>> Hi Richard,
>>
>>> If global_char really is a char then isn't that UB?
>>
>> No why?
>
> Well, "simple expressions like &global_char + 0xffffff00" made it sound
> like there really was a:
>
> extern char global_char;
>
> Only &global_char and &global_char + 1 are defined in that case.
> I was probably taking the example too literally though.
>
>> We can do all kinds of arithmetic based on pointers, either using
>> pointer types or converted to uintptr_t. Note that the optimizer
>> actually creates these expressions, for example arr[N-x] can be
>> evaluated as (&arr[0] + N) - x. So this remains legal even if N is
>> well outside the bounds of the array.
>
> Yeah, true, and I remember valid "x[n - big_offset]" -> invalid
> "x + big_offset - n" being a problem for MIPS too. It was one of
> the things offset_within_block_p was helping to avoid.
Multiple targets struggle with this. PA, mn103 also come to mind
immediately.
[ ... ]
>
> But more importantly, we can't say definitively that code quality isn't
> affected, only that it wasn't affected for the cases we've looked at.
> People could patch the compiler if the new ranges turn out not to strike
> the right balance for their use cases, but not everyone wants to do that.
Absolutely. IIRC These kinds of addresses are regularly created for
Ada. Or at least they were.
One of the concepts that really helped the PA from a codegen standpoint
was to distinguish between pointers which we know point into the object
from those which might point outside the object. Given that knowledge
we could generate more efficient code for the former (particularly WRT
indexed and scaled indexed address modes) and punt the latter.
REGNO_POINTER_FLAG carries this information.
Jeff
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2019-10-12 10:20 ` Richard Sandiford
@ 2019-10-12 11:23 ` Richard Sandiford
2019-10-13 8:45 ` Jeff Law
2019-10-14 15:51 ` Wilco Dijkstra
2 siblings, 0 replies; 48+ messages in thread
From: Richard Sandiford @ 2019-10-12 11:23 UTC (permalink / raw)
To: Wilco Dijkstra
Cc: GCC Patches, Ramana Radhakrishnan, Richard Earnshaw,
James Greenhalgh, Kyrylo Tkachov, nd
Richard Sandiford <richard.sandiford@arm.com> writes:
> Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
>> We can do all kinds of arithmetic based on pointers, either using
>> pointer types or converted to uintptr_t. Note that the optimizer
>> actually creates these expressions, for example arr[N-x] can be
>> evaluated as (&arr[0] + N) - x. So this remains legal even if N is
>> well outside the bounds of the array.
>
> Yeah, true, and I remember valid "x[n - big_offset]" -> invalid
> "x + big_offset - n" being a problem for MIPS too. It was one of
> the things offset_within_block_p was helping to avoid.
Doh, I of course meant "x - big_offset + n" here ;-)
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2019-10-11 18:31 ` Wilco Dijkstra
@ 2019-10-12 10:20 ` Richard Sandiford
2019-10-12 11:23 ` Richard Sandiford
` (2 more replies)
0 siblings, 3 replies; 48+ messages in thread
From: Richard Sandiford @ 2019-10-12 10:20 UTC (permalink / raw)
To: Wilco Dijkstra
Cc: GCC Patches, Ramana Radhakrishnan, Richard Earnshaw,
James Greenhalgh, Kyrylo Tkachov, nd
Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> Hi Richard,
>
>> If global_char really is a char then isn't that UB?
>
> No why?
Well, "simple expressions like &global_char + 0xffffff00" made it sound
like there really was a:
extern char global_char;
Only &global_char and &global_char + 1 are defined in that case.
I was probably taking the example too literally though.
> We can do all kinds of arithmetic based on pointers, either using
> pointer types or converted to uintptr_t. Note that the optimizer
> actually creates these expressions, for example arr[N-x] can be
> evaluated as (&arr[0] + N) - x. So this remains legal even if N is
> well outside the bounds of the array.
Yeah, true, and I remember valid "x[n - big_offset]" -> invalid
"x + big_offset - n" being a problem for MIPS too. It was one of
the things offset_within_block_p was helping to avoid.
>> I guess we still have to compile it without error though...
>
> *THIS* a million times... Any non-trivial application relies on UB behaviour with
> 100% certainty. But we still have to compile it correctly!
>
>>> To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
>>> 3.9GB offset from its references. For the tiny code model use a 64KB offset, allowing
>>> most of the 1MB range for code/data between the symbol and its references.
>>
>> These new values seem a bit magical. Would it work for the original
>> testcase to use FORCE_TO_MEM if !offset_within_block_p (x, offset)?
>
> No - the testcases fail with that.
Hmm, OK. Could you give more details? What does the motivating case
actually look like?
One of the reasons this is a hard patch to review is that it doesn't
include a testcase for the kind of situation it's trying to fix.
> It also reduces codequality by not allowing commonly used offsets as
> part of the symbol relocation.
Which kinds of cases specifically? Although I'm guessing the problem is
when indexing into external arrays of unknown size.
I think the starting point for this stuff is that the address of every
referenced object has to be within reach for the code model. In other
words, offset 0 must be within reach. It follows that the end + 1
address of every referenced object except the last (in memory) will
also be within reach.
The question then is: do we want, as a special exception, to allow
the end + 1 address of the last object to be out of reach? I think
it would be very difficult to support that reliably without more
information. The compiler would be left guessing which object is
the final one and how much of it is out of reach.
So IMO we should be able to assume that the start and end + 1 addresses
of any referenced object are within reach. For section anchors, we can
extend that to the containing anchor block, which is really just a
super-sized object.
So when offset_within_block_p, it seems like we should apply the
documented ranges for the code model, or even ignore them altogether.
Limiting the range to 64k for the tiny model would regress valid
cases like:
char x[0x20000];
char f (void) { return x[0x1ffff]; }
I'm not saying this case is important. I'm just saying there's no
obvious reason why the offset shouldn't be valid given the above
assumptions.
The question then is what to do about symbols whose size isn't known.
And I agree that unconditionally applying the full code-model offset
range is too aggressive in that case.
> So how would you want to make the offsets less magical? There isn't a clear limitation
> like for MOVW/MOVT relocations (which are limited to +-32KB), so it just is an arbitrary
> decision which allows this optimization for all frequently used offsets but avoids relocation
> failures caused by large offsets. The values I used were chosen so that codequality isn't
> affected, but it is practically impossible to trigger relocation errors.
>
> And they can always be changed again if required - this is not meant to be the final
> AArch64 patch ever!
Well, for one thing, if code quality isn't affected by using +/-64k
for the tiny model, do we have any evidence that we need a larger offset
for the other code models?
But more importantly, we can't say definitively that code quality isn't
affected, only that it wasn't affected for the cases we've looked at.
People could patch the compiler if the new ranges turn out not to strike
the right balance for their use cases, but not everyone wants to do that.
Maybe we need a new command-line option. It seems like the cheap
way out, but it might be justified. The problem is that, without one,
the compiler would be unconditionally applying an offset range that has
no obvious relation (from a user's POV) to the documented -mcmodel range.
We can then set the option's default value to whatever we think is
reasonable. And FWIW I've no specific issue with the values you've
chosen except for the question above.
Thanks,
Richard
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2019-10-10 18:33 ` Richard Sandiford
@ 2019-10-11 18:31 ` Wilco Dijkstra
2019-10-12 10:20 ` Richard Sandiford
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2019-10-11 18:31 UTC (permalink / raw)
To: Richard Sandiford
Cc: GCC Patches, Ramana Radhakrishnan, Richard Earnshaw,
James Greenhalgh, Kyrylo Tkachov, nd
Hi Richard,
> If global_char really is a char then isn't that UB?
No why? We can do all kinds of arithmetic based on pointers, either using
pointer types or converted to uintptr_t. Note that the optimizer actually creates
these expressions, for example arr[N-x] can be evaluated as (&arr[0] + N) - x.
So this remains legal even if N is well outside the bounds of the array.
> I guess we still have to compile it without error though...
*THIS* a million times... Any non-trivial application relies on UB behaviour with
100% certainty. But we still have to compile it correctly!
>> To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
>> 3.9GB offset from its references. For the tiny code model use a 64KB offset, allowing
>> most of the 1MB range for code/data between the symbol and its references.
>
> These new values seem a bit magical. Would it work for the original
> testcase to use FORCE_TO_MEM if !offset_within_block_p (x, offset)?
No - the testcases fail with that. It also reduces codequality by not allowing
commonly used offsets as part of the symbol relocation.
So how would you want to make the offsets less magical? There isn't a clear limitation
like for MOVW/MOVT relocations (which are limited to +-32KB), so it just is an arbitrary
decision which allows this optimization for all frequently used offsets but avoids relocation
failures caused by large offsets. The values I used were chosen so that codequality isn't
affected, but it is practically impossible to trigger relocation errors.
And they can always be changed again if required - this is not meant to be the final
AArch64 patch ever!
Cheers,
Wilco
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2019-10-10 17:24 ` Wilco Dijkstra
@ 2019-10-10 18:33 ` Richard Sandiford
2019-10-11 18:31 ` Wilco Dijkstra
0 siblings, 1 reply; 48+ messages in thread
From: Richard Sandiford @ 2019-10-10 18:33 UTC (permalink / raw)
To: Wilco Dijkstra
Cc: GCC Patches, Ramana Radhakrishnan, Richard Earnshaw,
James Greenhalgh, Kyrylo Tkachov, nd
Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> ping
>
> In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
> This means the offset can use all of the +/-4GB offset, leaving no offset available
> for the symbol itself. This results in relocation overflow and link-time errors
> for simple expressions like &global_char + 0xffffff00.
If global_char really is a char then isn't that UB? I guess we still
have to compile it without error though...
> To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
> 3.9GB offset from its references. For the tiny code model use a 64KB offset, allowing
> most of the 1MB range for code/data between the symbol and its references.
These new values seem a bit magical. Would it work for the original
testcase to use FORCE_TO_MEM if !offset_within_block_p (x, offset)?
Richard
> Bootstrapped on AArch64, passes regress, OK for commit?
>
> ChangeLog:
> 2018-11-09 Wilco Dijkstra <wdijkstr@arm.com>
>
> gcc/
> * config/aarch64/aarch64.c (aarch64_classify_symbol):
> Apply reasonable limit to symbol offsets.
>
> testsuite/
> * gcc.target/aarch64/symbol-range.c (foo): Set new limit.
> * gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
>
> --
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 83453d03095018eddd1801e71ef3836849267444..0023cb37bbae5afe9387840c1bb6b43586d4fac2 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -13047,26 +13047,26 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
> the offset does not cause overflow of the final address. But
> we have no way of knowing the address of symbol at compile time
> so we can't accurately say if the distance between the PC and
> - symbol + offset is outside the addressible range of +/-1M in the
> - TINY code model. So we rely on images not being greater than
> - 1M and cap the offset at 1M and anything beyond 1M will have to
> - be loaded using an alternative mechanism. Furthermore if the
> - symbol is a weak reference to something that isn't known to
> - resolve to a symbol in this module, then force to memory. */
> + 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).
> + 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))
> - || !IN_RANGE (offset, -1048575, 1048575))
> + || !IN_RANGE (offset, -0x10000, 0x10000))
> return SYMBOL_FORCE_TO_MEM;
> +
> return SYMBOL_TINY_ABSOLUTE;
>
> case AARCH64_CMODEL_SMALL:
> /* Same reasoning as the tiny code model, but the offset cap here is
> - 4G. */
> + 1MB, allowing +/-3.9GB for the offset to the symbol. */
> if ((SYMBOL_REF_WEAK (x)
> && !aarch64_symbol_binds_local_p (x))
> - || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
> - HOST_WIDE_INT_C (4294967264)))
> + || !IN_RANGE (offset, -0x100000, 0x100000))
> return SYMBOL_FORCE_TO_MEM;
> +
> return SYMBOL_SMALL_ABSOLUTE;
>
> case AARCH64_CMODEL_TINY_PIC:
> diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
> index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
> --- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
> +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
> @@ -1,12 +1,12 @@
> -/* { dg-do compile } */
> +/* { dg-do link } */
> /* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
>
> -int fixed_regs[0x00200000];
> +char fixed_regs[0x00200000];
>
> int
> -foo()
> +main ()
> {
> - return fixed_regs[0x00080000];
> + return fixed_regs[0x000ff000];
> }
>
> /* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
> --- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> @@ -1,12 +1,12 @@
> -/* { dg-do compile } */
> +/* { dg-do link } */
> /* { dg-options "-O3 -save-temps -mcmodel=small" } */
>
> -int fixed_regs[0x200000000ULL];
> +char fixed_regs[0x200000000ULL];
>
> int
> -foo()
> +main ()
> {
> - return fixed_regs[0x100000000ULL];
> + return fixed_regs[0xfffff000ULL];
> }
>
> /* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2019-09-09 17:07 ` Wilco Dijkstra
@ 2019-10-10 17:24 ` Wilco Dijkstra
2019-10-10 18:33 ` Richard Sandiford
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2019-10-10 17:24 UTC (permalink / raw)
To: GCC Patches, Ramana Radhakrishnan, Richard Earnshaw,
James Greenhalgh, Richard Sandiford, Kyrylo Tkachov
Cc: nd
ping
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
3.9GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
Bootstrapped on AArch64, passes regress, OK for commit?
ChangeLog:
2018-11-09 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 83453d03095018eddd1801e71ef3836849267444..0023cb37bbae5afe9387840c1bb6b43586d4fac2 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13047,26 +13047,26 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
the offset does not cause overflow of the final address. But
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
- symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ 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).
+ 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))
- || !IN_RANGE (offset, -1048575, 1048575))
+ || !IN_RANGE (offset, -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1MB, allowing +/-3.9GB for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (offset, -0x100000, 0x100000))
return SYMBOL_FORCE_TO_MEM;
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2019-09-02 12:12 ` Wilco Dijkstra
@ 2019-09-09 17:07 ` Wilco Dijkstra
2019-10-10 17:24 ` Wilco Dijkstra
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2019-09-09 17:07 UTC (permalink / raw)
To: GCC Patches, Ramana Radhakrishnan, Richard Earnshaw,
James Greenhalgh, Richard Sandiford
Cc: nd
ping
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
3.9GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
Bootstrapped on AArch64, passes regress, OK for commit?
ChangeLog:
2018-11-09 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 83453d03095018eddd1801e71ef3836849267444..0023cb37bbae5afe9387840c1bb6b43586d4fac2 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13047,26 +13047,26 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
the offset does not cause overflow of the final address. But
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
- symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ 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).
+ 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))
- || !IN_RANGE (offset, -1048575, 1048575))
+ || !IN_RANGE (offset, -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1MB, allowing +/-3.9GB for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (offset, -0x100000, 0x100000))
return SYMBOL_FORCE_TO_MEM;
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2019-08-19 15:57 ` Wilco Dijkstra
@ 2019-09-02 12:12 ` Wilco Dijkstra
2019-09-09 17:07 ` Wilco Dijkstra
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2019-09-02 12:12 UTC (permalink / raw)
To: GCC Patches, Ramana Radhakrishnan, Richard Earnshaw,
James Greenhalgh, Richard Sandiford
Cc: nd
ping
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
3.9GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
Bootstrapped on AArch64, passes regress, OK for commit?
ChangeLog:
2018-11-09 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 83453d03095018eddd1801e71ef3836849267444..0023cb37bbae5afe9387840c1bb6b43586d4fac2 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13047,26 +13047,26 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
the offset does not cause overflow of the final address. But
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
- symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ 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).
+ 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))
- || !IN_RANGE (offset, -1048575, 1048575))
+ || !IN_RANGE (offset, -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1MB, allowing +/-3.9GB for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (offset, -0x100000, 0x100000))
return SYMBOL_FORCE_TO_MEM;
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2019-07-31 17:00 ` Wilco Dijkstra
@ 2019-08-19 15:57 ` Wilco Dijkstra
2019-09-02 12:12 ` Wilco Dijkstra
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2019-08-19 15:57 UTC (permalink / raw)
To: GCC Patches, Ramana Radhakrishnan, Richard Earnshaw,
James Greenhalgh, Richard Sandiford
Cc: nd
ping
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
3.9GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
Bootstrapped on AArch64, passes regress, OK for commit?
ChangeLog:
2018-11-09 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 83453d03095018eddd1801e71ef3836849267444..0023cb37bbae5afe9387840c1bb6b43586d4fac2 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13047,26 +13047,26 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
the offset does not cause overflow of the final address. But
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
- symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ 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).
+ 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))
- || !IN_RANGE (offset, -1048575, 1048575))
+ || !IN_RANGE (offset, -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1MB, allowing +/-3.9GB for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (offset, -0x100000, 0x100000))
return SYMBOL_FORCE_TO_MEM;
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2019-05-28 17:27 Wilco Dijkstra
@ 2019-07-31 17:00 ` Wilco Dijkstra
2019-08-19 15:57 ` Wilco Dijkstra
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2019-07-31 17:00 UTC (permalink / raw)
To: GCC Patches, Ramana Radhakrishnan, Richard Earnshaw,
James Greenhalgh, Richard Sandiford
Cc: nd
ping
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
3.9GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
Bootstrapped on AArch64, passes regress, OK for commit?
ChangeLog:
2018-11-09 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 83453d03095018eddd1801e71ef3836849267444..0023cb37bbae5afe9387840c1bb6b43586d4fac2 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13047,26 +13047,26 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
the offset does not cause overflow of the final address. But
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
- symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ 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).
+ 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))
- || !IN_RANGE (offset, -1048575, 1048575))
+ || !IN_RANGE (offset, -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1MB, allowing +/-3.9GB for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (offset, -0x100000, 0x100000))
return SYMBOL_FORCE_TO_MEM;
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH][AArch64] Fix symbol offset limit
@ 2019-05-28 17:27 Wilco Dijkstra
2019-07-31 17:00 ` Wilco Dijkstra
0 siblings, 1 reply; 48+ messages in thread
From: Wilco Dijkstra @ 2019-05-28 17:27 UTC (permalink / raw)
To: GCC Patches, Ramana Radhakrishnan, Richard Earnshaw,
James Greenhalgh, Richard Sandiford
Cc: nd
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
3.9GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
Bootstrapped on AArch64, passes regress, OK for commit?
ChangeLog:
2018-11-09 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 83453d03095018eddd1801e71ef3836849267444..0023cb37bbae5afe9387840c1bb6b43586d4fac2 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13047,26 +13047,26 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
the offset does not cause overflow of the final address. But
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
- symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ 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).
+ 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))
- || !IN_RANGE (offset, -1048575, 1048575))
+ || !IN_RANGE (offset, -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1MB, allowing +/-3.9GB for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (offset, -0x100000, 0x100000))
return SYMBOL_FORCE_TO_MEM;
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH][AArch64] Fix symbol offset limit
@ 2018-11-09 14:47 Wilco Dijkstra
0 siblings, 0 replies; 48+ messages in thread
From: Wilco Dijkstra @ 2018-11-09 14:47 UTC (permalink / raw)
To: GCC Patches; +Cc: nd
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
3.9GB offset from its references. For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.
Bootstrapped on AArch64, passes regress, OK for commit?
ChangeLog:
2018-11-09 Wilco Dijkstra <wdijkstr@arm.com>
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
---
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index b6ea6d3f14b212b69f76d21b72527b6a8ea8cb0e..be03aeea8cd9bab07a01a161c4a91283bd25c99d 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -11794,26 +11794,26 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
the offset does not cause overflow of the final address. But
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
- symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ 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).
+ 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))
- || !IN_RANGE (offset, -1048575, 1048575))
+ || !IN_RANGE (offset, -0x10000, 0x10000))
return SYMBOL_FORCE_TO_MEM;
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1MB, allowing +/-3.9GB for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (offset, -0x100000, 0x100000))
return SYMBOL_FORCE_TO_MEM;
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
int
-foo()
+main ()
{
- return fixed_regs[0x00080000];
+ return fixed_regs[0x000ff000];
}
/* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
/* { dg-options "-O3 -save-temps -mcmodel=small" } */
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
int
-foo()
+main ()
{
- return fixed_regs[0x100000000ULL];
+ return fixed_regs[0xfffff000ULL];
}
/* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
^ permalink raw reply [flat|nested] 48+ messages in thread
end of thread, other threads:[~2019-10-16 8:24 UTC | newest]
Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-23 14:11 [PATCH][AArch64] Fix symbol offset limit Wilco Dijkstra
2016-08-26 10:43 ` Richard Earnshaw (lists)
2016-08-26 19:07 ` Wilco Dijkstra
2016-09-12 15:30 ` [PATCH v2][AArch64] " Wilco Dijkstra
2016-09-21 14:48 ` Wilco Dijkstra
2016-10-17 12:42 ` Wilco Dijkstra
2016-10-25 9:47 ` Wilco Dijkstra
2016-11-02 16:48 ` Wilco Dijkstra
2016-11-14 13:07 ` Wilco Dijkstra
2016-12-06 15:07 ` Wilco Dijkstra
2017-01-17 15:14 ` [PATCH v3][AArch64] " Wilco Dijkstra
2017-02-02 14:44 ` Wilco Dijkstra
2017-02-23 16:58 ` Wilco Dijkstra
2017-04-20 16:03 ` Wilco Dijkstra
2017-06-13 14:00 ` Wilco Dijkstra
2017-06-14 14:07 ` James Greenhalgh
2017-06-14 16:03 ` Wilco Dijkstra
2017-06-15 15:13 ` Richard Earnshaw (lists)
2017-06-15 16:55 ` Wilco Dijkstra
2017-06-15 17:39 ` Richard Earnshaw (lists)
2017-06-15 17:51 ` Wilco Dijkstra
2017-06-15 18:11 ` Richard Earnshaw (lists)
2017-06-15 18:18 ` Wilco Dijkstra
2017-06-15 18:34 ` Richard Earnshaw (lists)
2017-06-15 18:55 ` Wilco Dijkstra
2017-06-15 19:52 ` Joseph Myers
2017-06-16 15:14 ` Nathan Sidwell
2017-06-27 15:36 ` Wilco Dijkstra
2017-07-14 14:28 ` Wilco Dijkstra
2017-07-21 11:23 ` Wilco Dijkstra
2017-08-01 10:19 ` Wilco Dijkstra
2017-08-15 17:36 ` Wilco Dijkstra
2018-11-09 14:47 [PATCH][AArch64] " Wilco Dijkstra
2019-05-28 17:27 Wilco Dijkstra
2019-07-31 17:00 ` Wilco Dijkstra
2019-08-19 15:57 ` Wilco Dijkstra
2019-09-02 12:12 ` Wilco Dijkstra
2019-09-09 17:07 ` Wilco Dijkstra
2019-10-10 17:24 ` Wilco Dijkstra
2019-10-10 18:33 ` Richard Sandiford
2019-10-11 18:31 ` Wilco Dijkstra
2019-10-12 10:20 ` Richard Sandiford
2019-10-12 11:23 ` Richard Sandiford
2019-10-13 8:45 ` Jeff Law
2019-10-14 15:51 ` Wilco Dijkstra
2019-10-14 16:34 ` Richard Sandiford
2019-10-15 18:27 ` Wilco Dijkstra
2019-10-16 8:50 ` Richard Sandiford
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).