From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) by sourceware.org (Postfix) with ESMTP id 8B66A3858402 for ; Mon, 29 Nov 2021 22:29:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8B66A3858402 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=kernel.crashing.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kernel.crashing.org Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.14.1) with ESMTP id 1ATMSVhF019692; Mon, 29 Nov 2021 16:28:31 -0600 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 1ATMSUpa019691; Mon, 29 Nov 2021 16:28:30 -0600 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Mon, 29 Nov 2021 16:28:30 -0600 From: Segher Boessenkool To: "Kewen.Lin" Cc: GCC Patches , Bill Schmidt Subject: Re: [PATCH v2] combine: Tweak the condition of last_set invalidation Message-ID: <20211129222829.GW614@gate.crashing.org> References: <6bcd32fa-d0ef-b136-ddd9-92a1d21f60af@linux.ibm.com> <20210609201735.GJ18427@gate.crashing.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-3.4 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=no 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, 29 Nov 2021 22:29:34 -0000 Hi! On Fri, Jun 11, 2021 at 09:16:21PM +0800, Kewen.Lin wrote: > >> + /* Should pick up the lowest luid if the references > >> + are in the same block. */ > >> + if (label_tick == rsp->last_set_table_tick > >> + && rsp->last_set_table_luid > insn_luid) > >> + rsp->last_set_table_luid = insn_luid; > > > > Why? Is it conservative for the check you will do later? Please spell > > this out, it is crucial! > > Since later the combinations involving this insn probably make the > register be used in one insn sitting ahead (which has smaller luid than > the one which was recorded before). Yes, it's very conservative, this > ensure that we always use the luid of the insn which is the first insn > using this register in the block. Why would that be correct?! > The last_set invalidation is going > to catch the case like: > > ... regX // avoid the set used here ... > regX = ... > ... > > Once we have the smallest luid one of all insns which use register X, > any unsafe regX sets should be caught. Yes, you invalidate more, but because you put lies in the table :-( > * combine.c (struct reg_stat_type): New member > last_set_table_luid. This fits on one line. > (update_table_tick): Add one argument for insn luid and > set last_set_table_luid with it, remove its declaration. > (record_value_for_reg): Adjust the condition to set > last_set_invalid nonzero. These lines break earlier than they should as well. > + /* Record the luid of the insn which uses register n, the insn should > + be the first one using register n in that block of the insn which > + last_set_table_tick was set for. */ > + > + int last_set_table_luid; I'm not sure what this variable is for. The comment says something else than the variable name does, and now I don't know what to believe :-) The name says it is for a SET, the explanation says it is for a USE. Segher