From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 80219 invoked by alias); 16 Oct 2019 08:24:47 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 79576 invoked by uid 89); 16 Oct 2019 08:24:39 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-16.2 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_STOCKGEN autolearn=ham version=3.3.1 spammy=eating X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.110.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 16 Oct 2019 08:24:37 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 25033142F; Wed, 16 Oct 2019 01:24:28 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.126]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id EFBEF3F68E; Wed, 16 Oct 2019 01:24:26 -0700 (PDT) From: Richard Sandiford To: Wilco Dijkstra Mail-Followup-To: Wilco Dijkstra ,GCC Patches , Ramana Radhakrishnan , Richard Earnshaw , James Greenhalgh , Kyrylo Tkachov , nd , richard.sandiford@arm.com Cc: GCC Patches , Ramana Radhakrishnan , Richard Earnshaw , James Greenhalgh , Kyrylo Tkachov , nd Subject: Re: [PATCH][AArch64] Fix symbol offset limit References: Date: Wed, 16 Oct 2019 08:50:00 -0000 In-Reply-To: (Wilco Dijkstra's message of "Tue, 15 Oct 2019 19:23:37 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2019-10/txt/msg01154.txt.bz2 Wilco Dijkstra 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