public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
To: Richard Sandiford <Richard.Sandiford@arm.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
	Ramana Radhakrishnan	<Ramana.Radhakrishnan@arm.com>,
	Richard Earnshaw <Richard.Earnshaw@arm.com>,
	James Greenhalgh <James.Greenhalgh@arm.com>,
	Kyrylo Tkachov	<Kyrylo.Tkachov@arm.com>, nd <nd@arm.com>
Subject: Re: [PATCH][AArch64] Fix symbol offset limit
Date: Tue, 15 Oct 2019 18:27:00 -0000	[thread overview]
Message-ID: <VI1PR0801MB2127AF6EA94DBD9DAA2E989F83930@VI1PR0801MB2127.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <mpty2xnz52h.fsf@arm.com>

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\\\+" } } */

  reply	other threads:[~2019-10-15 18:23 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2019-10-16  8:50                       ` Richard Sandiford
  -- strict thread matches above, loose matches on Subject: below --
2018-11-09 14:47 Wilco Dijkstra
2016-08-23 14:11 Wilco Dijkstra
2016-08-26 10:43 ` Richard Earnshaw (lists)
2016-08-26 19:07   ` Wilco Dijkstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=VI1PR0801MB2127AF6EA94DBD9DAA2E989F83930@VI1PR0801MB2127.eurprd08.prod.outlook.com \
    --to=wilco.dijkstra@arm.com \
    --cc=James.Greenhalgh@arm.com \
    --cc=Kyrylo.Tkachov@arm.com \
    --cc=Ramana.Radhakrishnan@arm.com \
    --cc=Richard.Earnshaw@arm.com \
    --cc=Richard.Sandiford@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=nd@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).