From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10656 invoked by alias); 31 Jul 2007 19:44:52 -0000 Received: (qmail 10645 invoked by uid 22791); 31 Jul 2007 19:44:49 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 31 Jul 2007 19:44:44 +0000 Received: (qmail 25851 invoked from network); 31 Jul 2007 19:44:42 -0000 Received: from unknown (HELO gateway) (10.0.0.100) by mail.codesourcery.com with SMTP; 31 Jul 2007 19:44:42 -0000 Received: by gateway (Postfix, from userid 1010) id 006636C0D2; Tue, 31 Jul 2007 12:44:41 -0700 (PDT) From: Richard Sandiford To: gcc-patches@gcc.gnu.org Mail-Followup-To: gcc-patches@gcc.gnu.org, richard@codesourcery.com Subject: RFA (was RFC): RTL sharing between decls and instructions References: <87d4ydlet3.fsf@firetop.home> Date: Tue, 31 Jul 2007 20:44:00 -0000 In-Reply-To: <87d4ydlet3.fsf@firetop.home> (Richard Sandiford's message of "Fri\, 27 Jul 2007 22\:25\:28 +0100") Message-ID: <87y7gws6hi.fsf@firetop.home> User-Agent: Gnus/5.110006 (No Gnus v0.6) Emacs/21.4 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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 X-SW-Source: 2007-07/txt/msg02203.txt.bz2 Richard Sandiford writes: > unshare_all_rtl used to unshare DECL_RTLs as well as expressions in the > instruction stream. That changed with: > > http://gcc.gnu.org/ml/gcc-patches/2007-05/msg00541.html > > I think that patch was in itself the right thing to do. However, in > anticipation of the old unshare_all_rtl behaviour, unshare_all_rtl_again > used to reset the used flags on all those DECL_RTLs. I think that's no > longer correct. unshare_all_rtl_again is used after reload, which may > have introduced invalid sharing by converting pseudo registers into > non-register expressions. These sharing problems exist between > DECL_RTLs and the instruction stream as well as between instructions > themselves. > > Thus I think the right thing now is to get unshare_all_rtl_again to > _set_ rather than reset the used flags on DECL_RTLs, so that if a > DECL_RTL mentions an ex-register, all references to that ex-register > in the instruction stream will be copied. This is what the old > unshare_all_decls did as a side effect. > > Does that sound right? I've checked that this patch fixes the case > I'm seeing, but I'm not too familiar with this code, so I wanted to > check whether I was barking up the wrong tree. I've now bootstrapped & regression-tested the patch on x86_64-linux-gnu, and regression-tested it on mipsisa64-elfoabi (options {-EB,-EL}{,-msoft-float}{,-mips16}). I don't have a testcase because I only encountered this problem with a soon-to-be-sumitted MIPS16 mode for which constant pool entries are no longer legitimate addresses. The same problem could in theory affect current non-MIPS16 targets, but I'm not sure how easy it would be to reproduce. OK to install? Richard > gcc/ > * emit-rtl.c (reset_used_decls): Rename to... > (set_used_decls): ...this. Set the used flag rather than clearing it. > (unshare_all_rtl_again): Update accordingly. Set flags on argument > DECL_RTLs rather than resetting them. > > Index: gcc/emit-rtl.c > =================================================================== > --- gcc/emit-rtl.c (revision 126993) > +++ gcc/emit-rtl.c (working copy) > @@ -167,7 +167,7 @@ #define first_label_num (cfun->emit->x_f > > static rtx make_call_insn_raw (rtx); > static rtx change_address_1 (rtx, enum machine_mode, rtx, int); > -static void reset_used_decls (tree); > +static void set_used_decls (tree); > static void mark_label_nuses (rtx); > static hashval_t const_int_htab_hash (const void *); > static int const_int_htab_eq (const void *, const void *); > @@ -2160,11 +2160,11 @@ unshare_all_rtl_again (rtx insn) > } > > /* Make sure that virtual stack slots are not shared. */ > - reset_used_decls (DECL_INITIAL (cfun->decl)); > + set_used_decls (DECL_INITIAL (cfun->decl)); > > /* Make sure that virtual parameters are not shared. */ > for (decl = DECL_ARGUMENTS (cfun->decl); decl; decl = TREE_CHAIN (decl)) > - reset_used_flags (DECL_RTL (decl)); > + set_used_flags (DECL_RTL (decl)); > > reset_used_flags (stack_slot_list); > > @@ -2353,20 +2353,28 @@ unshare_all_rtl_in_chain (rtx insn) > } > > /* Go through all virtual stack slots of a function and mark them as > - not shared. */ > + shared. We never replace the DECL_RTLs themselves with a copy, > + but expressions mentioned into a DECL_RTL cannot be shared with > + expressions in the instruction stream. > + > + Note that reload may convert pseudo registers into memories in-place. > + Pseudo registers are always shared, but MEMs never are. Thus if we > + reset the used flags on MEMs in the instruction stream, we must set > + them again on MEMs that appear in DECL_RTLs. */ > + > static void > -reset_used_decls (tree blk) > +set_used_decls (tree blk) > { > tree t; > > /* Mark decls. */ > for (t = BLOCK_VARS (blk); t; t = TREE_CHAIN (t)) > if (DECL_RTL_SET_P (t)) > - reset_used_flags (DECL_RTL (t)); > + set_used_flags (DECL_RTL (t)); > > /* Now process sub-blocks. */ > for (t = BLOCK_SUBBLOCKS (blk); t; t = TREE_CHAIN (t)) > - reset_used_decls (t); > + set_used_decls (t); > } > > /* Mark ORIG as in use, and return a copy of it if it was already in use.