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 DF624385840A for ; Mon, 14 Feb 2022 09:44:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DF624385840A 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 622381396; Mon, 14 Feb 2022 01:44:53 -0800 (PST) Received: from localhost (unknown [10.32.98.88]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id AF1BF3F66F; Mon, 14 Feb 2022 01:44:52 -0800 (PST) From: Richard Sandiford To: Iain Sandoe via Gcc-patches Mail-Followup-To: Iain Sandoe via Gcc-patches , iain@sandoe.co.uk, Iain Sandoe , segher@kernel.crashing.org, richard.sandiford@arm.com Cc: iain@sandoe.co.uk, Iain Sandoe , segher@kernel.crashing.org Subject: Re: [pushed] LRA, rs6000, Darwin: Amend lo_sum use for forced constants [PR104117]. References: <20220211235904.25765-1-iain@sandoe.co.uk> Date: Mon, 14 Feb 2022 09:44:51 +0000 In-Reply-To: <20220211235904.25765-1-iain@sandoe.co.uk> (Iain Sandoe via Gcc-patches's message of "Fri, 11 Feb 2022 23:59:04 +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=-12.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Mon, 14 Feb 2022 09:44:56 -0000 Iain Sandoe via Gcc-patches writes: > Two issues resulted in this PR, which manifests when we force a constant = into > memory in LRA (in PIC code on Darwin). The presence of such forced const= ants > is quite dependent on other RTL optimisations, and it is easy for the iss= ue to > become latent for a specific case. > > First, in the Darwin-specific rs6000 backend code, we were not being care= ful > enough in rejecting invalid symbolic addresses. Specifically, when gener= ating > PIC code, we require a SYMBOL_REF to be wrapped in an UNSPEC_MACHOPIC_OFF= SET. > > Second, LRA was attempting to load a register using an invalid lo_sum add= ress. > > The LRA changes are approved in the PR by Vladimir, and the RS6000 change= s are > Darwin-specific (although, of course, any observations are welcome). > > Tested on several lo_sum targets and x86_64 all languages except as noted: > powerpc64-linux (m32/m64) -D > powerpc64le-linux -D > powerpc64-aix -Ada -Go -D > aarch64-linux -Ada -D > x86_64-linux all langs -D > powerpc-darwin9 (master and 11.2) -D -Go. > > pushed to master, thanks, > Iain > > Signed-off-by: Iain Sandoe > Co-authored-by: Vladimir Makarov > > PR target/104117 > > gcc/ChangeLog: > > * config/rs6000/rs6000.cc (darwin_rs6000_legitimate_lo_sum_const_p): > Check for UNSPEC_MACHOPIC_OFFSET wrappers on symbolic addresses when > emitting PIC code. > (legitimate_lo_sum_address_p): Likewise. > * lra-constraints.cc (process_address_1): Do not attempt to emit a reg > load from an invalid lo_sum address. > --- > gcc/config/rs6000/rs6000.cc | 38 +++++++++++++++++++++++++++++++++++-- > gcc/lra-constraints.cc | 17 ++--------------- > 2 files changed, 38 insertions(+), 17 deletions(-) > > [=E2=80=A6] > diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc > index fdff9e0720a..c700c3f4578 100644 > --- a/gcc/lra-constraints.cc > +++ b/gcc/lra-constraints.cc > @@ -3625,21 +3625,8 @@ process_address_1 (int nop, bool check_only_p, > *ad.inner =3D gen_rtx_LO_SUM (Pmode, new_reg, addr); > if (!valid_address_p (op, &ad, cn)) > { > - /* Try to put lo_sum into register. */ > - insn =3D emit_insn (gen_rtx_SET > - (new_reg, > - gen_rtx_LO_SUM (Pmode, new_reg, addr))); > - code =3D recog_memoized (insn); > - if (code >=3D 0) > - { > - *ad.inner =3D new_reg; > - if (!valid_address_p (op, &ad, cn)) > - { > - *ad.inner =3D addr; > - code =3D -1; > - } > - } > - > + *ad.inner =3D addr; /* Punt. */ > + code =3D -1; > } > } > if (code < 0) Could you go into more details about this? Why is it OK to continue to try: (lo_sum new_reg addr) directly as an address (the context at the top of the hunk), but not try moving the lo_sum into a register? They should be semantically equivalent, so it seems that if one is wrong, the other would be too. Thanks, Richard