From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 88848 invoked by alias); 11 Jan 2019 11:23:06 -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 88837 invoked by uid 89); 11 Jan 2019 11:23:05 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 11 Jan 2019 11:23:03 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 44E7680D; Fri, 11 Jan 2019 03:23:02 -0800 (PST) Received: from localhost (unknown [10.32.98.35]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2CBC23F70D; Fri, 11 Jan 2019 03:23:01 -0800 (PST) From: Richard Sandiford To: Steve Ellcey Mail-Followup-To: Steve Ellcey ,"sellcey\@cavium.com" , "gcc-patches\@gcc.gnu.org" , rguenther@suse.de, jakub@redhat.com, richard.sandiford@arm.com Cc: "sellcey\@cavium.com" , "gcc-patches\@gcc.gnu.org" , rguenther@suse.de, jakub@redhat.com Subject: Re: [Patch 4/4][Aarch64] v2: Implement Aarch64 SIMD ABI References: <1541699749.12016.9.camel@cavium.com> <87tvjq3kqs.fsf@arm.com> <875zv0l64x.fsf@arm.com> <7bdc455d8d0b38778e151ad8ce8cfe6b69c580c5.camel@marvell.com> <87d0p6gng4.fsf@arm.com> <175d9f25425f2aa634e2bd88f7ee6eefabae97ed.camel@marvell.com> <87bm4o98jr.fsf@arm.com> <1faa09dc8a758006fc373f6e4296ea1d5c39043d.camel@marvell.com> Date: Fri, 11 Jan 2019 11:23:00 -0000 In-Reply-To: <1faa09dc8a758006fc373f6e4296ea1d5c39043d.camel@marvell.com> (Steve Ellcey's message of "Fri, 11 Jan 2019 00:13:27 +0000") Message-ID: <87ef9j30bw.fsf@arm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2019-01/txt/msg00614.txt.bz2 Steve Ellcey writes: > OK, I fixed the issues in your last email. I initially found one > regression while testing. In lra_create_live_ranges_1 I had removed > the 'call_p = false' statement but did not replaced it with anything. > This resulted in no regressions on aarch64 but caused a single > regression on x86 (gcc.target/i386/pr87759.c). I replaced the > line with 'call_insn = NULL' and the regression went away so I > have clean bootstraps and no regressions on aarch64 and x86 now. Looks good to me bar the parameter description below. > If this looks good to you can I go ahead and check it in? I know > we are in Stage 3 now, but my recollection is that patches that were > initially submitted during Stage 1 could go ahead once approved. Yeah, like you say, this was originally posted in stage 1 and is the last patch in the series. Not committing it would leave the work incomplete in GCC 9. The problem is that we're now in stage 4 rather than stage 3. I don't think I'm neutral enough to make the call. Richard, Jakub? > diff --git a/gcc/lra-lives.c b/gcc/lra-lives.c > index a00ec38..b77b675 100644 > --- a/gcc/lra-lives.c > +++ b/gcc/lra-lives.c > @@ -576,25 +576,39 @@ lra_setup_reload_pseudo_preferenced_hard_reg (int regno, > > /* Check that REGNO living through calls and setjumps, set up conflict > regs using LAST_CALL_USED_REG_SET, and clear corresponding bits in > - PSEUDOS_LIVE_THROUGH_CALLS and PSEUDOS_LIVE_THROUGH_SETJUMPS. */ > + PSEUDOS_LIVE_THROUGH_CALLS and PSEUDOS_LIVE_THROUGH_SETJUMPS. > + CALL_INSN may be the specific call we want to check that REGNO lives > + through or a call that is guaranteed to clobber REGNO if any call > + in the current block clobbers REGNO. */ I think it would be more accurate to say: CALL_INSN is a call that is representative of all calls in the region described by the PSEUDOS_LIVE_THROUGH_* sets, in terms of the registers that it preserves and clobbers. */ > + > static inline void > check_pseudos_live_through_calls (int regno, > - HARD_REG_SET last_call_used_reg_set) > + HARD_REG_SET last_call_used_reg_set, > + rtx_insn *call_insn) > { > int hr; > + rtx_insn *old_call_insn; > > if (! sparseset_bit_p (pseudos_live_through_calls, regno)) > return; > + > + gcc_assert (call_insn && CALL_P (call_insn)); > + old_call_insn = lra_reg_info[regno].call_insn; > + if (!old_call_insn > + || (targetm.return_call_with_max_clobbers > + && targetm.return_call_with_max_clobbers (old_call_insn, call_insn) > + == call_insn)) > + lra_reg_info[regno].call_insn = call_insn; > + > sparseset_clear_bit (pseudos_live_through_calls, regno); > IOR_HARD_REG_SET (lra_reg_info[regno].conflict_hard_regs, > last_call_used_reg_set); > > for (hr = 0; HARD_REGISTER_NUM_P (hr); hr++) > - if (targetm.hard_regno_call_part_clobbered (hr, > + if (targetm.hard_regno_call_part_clobbered (call_insn, hr, > PSEUDO_REGNO_MODE (regno))) > add_to_hard_reg_set (&lra_reg_info[regno].conflict_hard_regs, > PSEUDO_REGNO_MODE (regno), hr); > - lra_reg_info[regno].call_p = true; > if (! sparseset_bit_p (pseudos_live_through_setjumps, regno)) > return; > sparseset_clear_bit (pseudos_live_through_setjumps, regno); BTW, I think we could save some compile time by moving the "for" loop into the new "if", since otherwise call_insn should have no new information. But that was true before as well (since we could have skipped the loop if lra_reg_info[regno].call_p was already true), so it's really a separate issue. Thanks, Richard