From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16962 invoked by alias); 28 Feb 2018 02:39:23 -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 16952 invoked by uid 89); 28 Feb 2018 02:39:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 28 Feb 2018 02:39:20 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 93E56C047B8E for ; Wed, 28 Feb 2018 02:39:19 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-67.rdu2.redhat.com [10.10.112.67]) by smtp.corp.redhat.com (Postfix) with ESMTP id C348D177CB; Wed, 28 Feb 2018 02:39:18 +0000 (UTC) Subject: Re: [PR81611] improve auto-inc To: Alexandre Oliva Cc: gcc-patches@gcc.gnu.org References: From: Jeff Law Message-ID: Date: Wed, 28 Feb 2018 02:39:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-02/txt/msg01550.txt.bz2 On 02/27/2018 04:18 PM, Alexandre Oliva wrote: > On Feb 14, 2018, Jeff Law wrote: > >>> + regno = REGNO (inc_insn.reg0); >>> + int luid = DF_INSN_LUID (mem_insn.insn); >>> + mem_insn.insn = get_next_ref (regno, bb, reg_next_use); >> So I think a comment is warranted right as we enter the TRUE arm. > >> At that point INC_INSN is an inc/dec. But MEM_INSN is not necessarily a >> memory reference. It could be a memory reference, it could be a copy, >> it could be something completely different (it's just the next insn that >> references the result of the increment). In the case we care about we >> want it to be a copy of INC_INSN's REG_RES back to REG0. > >> ISTM that verifying MEM_INSN is a reg->reg copy (reg_res -> reg0) before >> we call get_next_ref for reg0 is advisable and probably good from a >> compile-time standpoint by avoiding calls into find_address. > > But we don't need it to be a copy. The transformation is just as > legitimate if the regs go independent ways after that point. We have > reg_res set to reg0+reg1, and then a use of reg0 in a MEM before any > other use of reg_res. We turn that into a copy of reg0 to reg_res, and > the MEM addr into a post_add of reg_res with reg1 (possibly a post_inc), > so that the MEM dereferences reg_res while it's still equal to reg0, and > after the MEM, reg_res becomes reg0+reg1, as it should for any > subsequent uses, and reg0 is unmodified. Whether or not a subsequent > copy from reg_res to reg0 is to be found won't make the transformation > any more or less legitimate. > >> After we call get_next_ref to get the next reference of the source of >> the increment, then we're hoping to find a memory reference that uses >> REG0. But it's not guaranteed it's a memory reference insn. > > Yeah, find_address will determine if it contains any of the MEM patterns > we might be interested in, but it could be anything whatsoever. The MEM > pattern might appear virtually anywhere in the insn. > >> I was having an awful time understanding how this code could work from >> the comments until I put it under a debugger and got a sense of the >> state as we entered that IF block. Then it was much clearer :-) > > Sorry, I realize the comments were written based on a lot of context > about the overall behavior of the pass, that I had learned while trying > to figure it out. At the risk of making it redundant, I've expanded the > comments, and added further tests that won't affect current behavior in > any significant way, but that might speed things up a bit and will save > us trouble should find_address be extended to catch additional patterns. > > >> I believe Georg had other testcases in subsequent comments in the BZ, >> but I don't believe they were flagged as regressions. > > However, with the testcases I realized the incremented register could > still be live, even if we didn't find a subsequent use for it. > Adjusting for that made those testcases use post_inc too. > > Here's the improved patch, regstrapped on aarch64-, ppc64-, and > ppc64el-linux-gnu. Ok to install? > > > [PR81611] turn inc-and-use-of-dead-orig into auto-inc > > When the addressing modes available on the machine don't allow offsets > in addresses, odds are that post-increments will be represented in > trees and RTL as: > > y <= x + 1 > ... *(x) ... > x <= y > > so deal with it by turning such RTL as: > > (set y (plus x n)) > ... (mem x) ... > > without intervening uses of y into > > (set y x) > ... (mem (post_add y n)) ... > > so as to create auto-inc addresses that we'd otherwise miss. > > > for gcc/ChangeLog > > PR rtl-optimization/81611 > * auto-inc-dec.c (attempt_change): Move dead note from > mem_insn if it's the next use of regno > (find_address): Take address use of reg holding > non-incremented value. Add parm to limit search to the named > reg only. > (merge_in_block): Attempt to use a mem insn that is the next > use of the original regno. OK. Thanks! Jeff