From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id DAADB3851C18 for ; Fri, 30 Apr 2021 08:46:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DAADB3851C18 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 71742ED1; Fri, 30 Apr 2021 01:46:05 -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 F042C3F70D; Fri, 30 Apr 2021 01:46:04 -0700 (PDT) From: Richard Sandiford To: Wilco Dijkstra via Gcc-patches Mail-Followup-To: Wilco Dijkstra via Gcc-patches , Wilco Dijkstra , richard.sandiford@arm.com Cc: Wilco Dijkstra Subject: Re: [PATCH] AArch64: Cleanup aarch64_classify_symbol References: Date: Fri, 30 Apr 2021 09:46:03 +0100 In-Reply-To: (Wilco Dijkstra via Gcc-patches's message of "Wed, 28 Apr 2021 17:48:27 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_STOCKGEN, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Apr 2021 08:46:07 -0000 Wilco Dijkstra via Gcc-patches writes: > Hi Richard, > >> Just to check: I guess this part is an optimisation, because it >> means that we can share the GOT entry with other TUs.=C2=A0 Is that righ= t? >> I think it would be worth having a comment either way, whatever the >> rationale.=C2=A0 A couple of other very minor things: > > It's just to make the code simpler and more orthogonal - the size of the > sequence and sharing of GOT/literal is identical in both cases. Hmm, OK. I guess it makes things more consistent in that sense (PIC vs. non-PIC). But on the other side it's making things less internally consistent for non-PIC, since we don't use the GOT for anything else there. I guess in principle there's a danger that a custom *-elf linker script might not bother to set up the .got properly, on the assumption that it shouldn't be needed. But there's not much in it either way, and I agree consistency with LLVM is a factor too. > Here is v2 with comments/spaces fixed up: > > > Use a GOT indirection for extern weak symbols instead of a literal - this= is the same as > PIC/PIE and mirrors LLVM behaviour. Ensure PIC/PIE use the same offset l= imits for symbols > that don't use the GOT. > > Passes bootstrap and regress. OK for commit? > > ChangeLog: > 2021-04-27 Wilco Dijkstra > > * config/aarch64/aarch64.c (aarch64_classify_symbol): Use GOT for= extern weak symbols. > Limit symbol offsets for non-GOT symbols with PIC/PIE. OK, thanks. Richard > > --- > > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index 82957dddbe42a7f907b2960294ac7f8abf7be2ff..641c83b479e76cbcc75b299eb= 7ae5f634d9db7cd 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -17854,7 +17854,14 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT of= fset) >=20=20 > switch (aarch64_cmodel) > { > + case AARCH64_CMODEL_TINY_PIC: > case AARCH64_CMODEL_TINY: > + /* With -fPIC non-local symbols use the GOT. For orthogonality > + always use the GOT for extern weak symbols. */ > + if ((flag_pic || SYMBOL_REF_WEAK (x)) > + && !aarch64_symbol_binds_local_p (x)) > + return SYMBOL_TINY_GOT; > + > /* When we retrieve symbol + offset address, we have to make sure > the offset does not cause overflow of the final address. But > we have no way of knowing the address of symbol at compile time > @@ -17862,42 +17869,30 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT o= ffset) > symbol + offset is outside the addressible range of +/-1MB in the > TINY code model. So we limit the maximum offset to +/-64KB and > assume the offset to the symbol is not larger than +/-(1MB - 64KB). > - If offset_within_block_p is true we allow larger offsets. > - Furthermore force to memory if the symbol is a weak reference to > - something that doesn't resolve to a symbol in this module. */ > - > - if (SYMBOL_REF_WEAK (x) && !aarch64_symbol_binds_local_p (x)) > - return SYMBOL_FORCE_TO_MEM; > + If offset_within_block_p is true we allow larger offsets. */ > if (!(IN_RANGE (offset, -0x10000, 0x10000) > || offset_within_block_p (x, offset))) > return SYMBOL_FORCE_TO_MEM; >=20=20 > return SYMBOL_TINY_ABSOLUTE; >=20=20 > + > + case AARCH64_CMODEL_SMALL_SPIC: > + case AARCH64_CMODEL_SMALL_PIC: > case AARCH64_CMODEL_SMALL: > + if ((flag_pic || SYMBOL_REF_WEAK (x)) > + && !aarch64_symbol_binds_local_p (x)) > + return aarch64_cmodel =3D=3D AARCH64_CMODEL_SMALL_SPIC > + ? SYMBOL_SMALL_GOT_28K : SYMBOL_SMALL_GOT_4G; > + > /* Same reasoning as the tiny code model, but the offset cap here is > 1MB, allowing +/-3.9GB for the offset to the symbol. */ > - > - if (SYMBOL_REF_WEAK (x) && !aarch64_symbol_binds_local_p (x)) > - return SYMBOL_FORCE_TO_MEM; > if (!(IN_RANGE (offset, -0x100000, 0x100000) > || offset_within_block_p (x, offset))) > return SYMBOL_FORCE_TO_MEM; >=20=20 > return SYMBOL_SMALL_ABSOLUTE; >=20=20 > - case AARCH64_CMODEL_TINY_PIC: > - if (!aarch64_symbol_binds_local_p (x)) > - return SYMBOL_TINY_GOT; > - return SYMBOL_TINY_ABSOLUTE; > - > - case AARCH64_CMODEL_SMALL_SPIC: > - case AARCH64_CMODEL_SMALL_PIC: > - if (!aarch64_symbol_binds_local_p (x)) > - return (aarch64_cmodel =3D=3D AARCH64_CMODEL_SMALL_SPIC > - ? SYMBOL_SMALL_GOT_28K : SYMBOL_SMALL_GOT_4G); > - return SYMBOL_SMALL_ABSOLUTE; > - > case AARCH64_CMODEL_LARGE: > /* This is alright even in PIC code as the constant > pool reference is always PC relative and within