From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 56944 invoked by alias); 24 Jan 2018 14:19:42 -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 55972 invoked by uid 89); 24 Jan 2018 14:19:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.7 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-wm0-f51.google.com Received: from mail-wm0-f51.google.com (HELO mail-wm0-f51.google.com) (74.125.82.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 24 Jan 2018 14:19:39 +0000 Received: by mail-wm0-f51.google.com with SMTP id 143so8746838wma.5 for ; Wed, 24 Jan 2018 06:19:38 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=OsqsyB4aIHRopu6culn4yjrBkIICNQp+wpxmBCC+OUg=; b=F3oLboH8d3JVXfrP5q7k6XSgz3v+vLIHA1Kt9qYAMMj5IzmYmtp2v9F6E73TiUqOUn 6zX5hD8ecKb0piFVSuaP+J/OP9CALZK9bUHSogcRq+mXYGSB5W03/irPPc3gTKqhGix4 sVu+SFUd5+lDd5ckcMrbOVWDNqrnNme2JlPSh/RahtnB2fn6Xx0ZwnaTICL3VudIMfeG 9t+VB0MfQzWG7sRx4UEcVNQEbhnbrPkVWDhCx2N5EJvOAfImna+MPaJsky9yEigWJJgB yku1kxaqZyxsbUoFoey+AfgAgL0vypVYVKOPZRym+WePhCpgk00wnvGNm1J7J32O/u6G XRTQ== X-Gm-Message-State: AKwxytfS31vOBVtPdDO3znqlp5gkvEB1j/mnPIUvOGUFEogKfhCoJF1e cWQoYXhHpswJEAPQH6UNDuI0Qy9R11IdxQxnHWsKkQ== X-Google-Smtp-Source: AH8x227LL4W5tPJ/WsGFqNpJKNzcY6YQRdkENc6tpIyCkJ4LpKyd/VU6M4IWiL/F9zGjsa8a7Pl0OuUKJjDwWgg6rxM= X-Received: by 10.80.191.71 with SMTP id g7mr24550797edk.246.1516803577103; Wed, 24 Jan 2018 06:19:37 -0800 (PST) MIME-Version: 1.0 Received: by 10.80.224.135 with HTTP; Wed, 24 Jan 2018 06:19:36 -0800 (PST) In-Reply-To: References: From: Richard Biener Date: Wed, 24 Jan 2018 14:40:00 -0000 Message-ID: Subject: Re: [PR81611] improve auto-inc To: Alexandre Oliva Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2018-01/txt/msg02027.txt.bz2 On Wed, Jan 24, 2018 at 4:42 AM, Alexandre Oliva wrote: > These two patches fix PR81611. > > The first one improves forwprop so that we avoid adding SSA conflicting > by forwpropping the iv increment, which may cause both the incremented > and the original value to be live, even when the iv is copied between > the PHI node and the increment. We already handled the case in which > there aren't any such copies. > > Alas, this is not enough to address the problem on avr, even though it > fixes it on e.g. ppc. The reason is that avr rejects var+offset > addresses, and this prevents the memory access in a post-inc code > sequence from being adjusted to an address that auto-inc-dec can > recognize. > > The second patch adjusts auto-inc-dec to recognize a code sequence in > which the original, unincremented pseudo is used in an address after > it's incremented into another pseudo, and turn that into a post-inc > address, leaving the copying for subsequent passes to eliminate. > > Regstrapped on x86_64-linux-gnu, i686-linux-gnu, ppc64-linux-gnu and > aarch64-linux-gnu. Ok to install? > > > I'd appreciate suggestions on how to turn the submitted testcase into a > regression test; I suppose an avr-specific test that requires the > auto-inc transformation is a possibility, but that feels a bit too > limited, doesn't it? Thoughts? Thanks in advance, > > > [PR81611] accept copies in simple_iv_increment_p > > If there are copies between the GIMPLE_PHI at the loop body and the > increment that reaches it (presumably through a back edge), still > regard it as a simple_iv_increment, so that we won't consider the > value in the back edge eligible for forwprop. Doing so would risk > making the phi node and the incremented conflicting value live > within the loop, and the phi node to be preserved for propagated > uses after the loop. > > for gcc/ChangeLog > > PR tree-optimization/81611 > * tree-ssa-dom.c (simple_iv_increment_p): Skip intervening > copies. > --- > gcc/tree-ssa-dom.c | 21 +++++++++++++++++---- > 1 file changed, 17 insertions(+), 4 deletions(-) > > diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c > index 2b371667253a..3c0ff9458342 100644 > --- a/gcc/tree-ssa-dom.c > +++ b/gcc/tree-ssa-dom.c > @@ -1276,8 +1276,11 @@ record_equality (tree x, tree y, class const_and_copies *const_and_copies) > /* Returns true when STMT is a simple iv increment. It detects the > following situation: > > - i_1 = phi (..., i_2) > - i_2 = i_1 +/- ... */ > + i_1 = phi (..., i_k) > + [...] > + i_j = i_{j-1} for each j : 2 <= j <= k-1 > + [...] > + i_k = i_{k-1} +/- ... */ > > bool > simple_iv_increment_p (gimple *stmt) > @@ -1305,8 +1308,18 @@ simple_iv_increment_p (gimple *stmt) > return false; > > phi = SSA_NAME_DEF_STMT (preinc); > - if (gimple_code (phi) != GIMPLE_PHI) > - return false; > + while (gimple_code (phi) != GIMPLE_PHI) > + { > + /* Follow trivial copies, but not the DEF used in a back edge, > + so that we don't prevent coalescing. */ > + if (gimple_code (phi) != GIMPLE_ASSIGN > + || gimple_assign_lhs (phi) != preinc > + || !gimple_assign_ssa_name_copy_p (phi)) given gimple_assign_ssa_name_copy checks it is an assign just do if (!gimple_assign_ssa-anme_Copy_p (phi)) the lhs != preinc check is always false given you got to phi via SSA_NAME_DEF_STMT of preinc. The simple_iv_increment_p change is ok with that change. The other change is RTL which I defer to somebody else. Richard. > + return false; > + > + preinc = gimple_assign_rhs1 (phi); > + phi = SSA_NAME_DEF_STMT (preinc); > + } > > for (i = 0; i < gimple_phi_num_args (phi); i++) > if (gimple_phi_arg_def (phi, i) == lhs) > > > [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 this form 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. > (merge_in_block): Attempt to use a mem insn that is the next > use of the original regno. > --- > gcc/auto-inc-dec.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 44 insertions(+), 2 deletions(-) > > diff --git a/gcc/auto-inc-dec.c b/gcc/auto-inc-dec.c > index d02fa9d081c7..4ffbcf56a456 100644 > --- a/gcc/auto-inc-dec.c > +++ b/gcc/auto-inc-dec.c > @@ -508,7 +508,11 @@ attempt_change (rtx new_addr, rtx inc_reg) > before the memory reference. */ > gcc_assert (mov_insn); > emit_insn_before (mov_insn, inc_insn.insn); > - move_dead_notes (mov_insn, inc_insn.insn, inc_insn.reg0); > + regno = REGNO (inc_insn.reg0); > + if (reg_next_use[regno] == mem_insn.insn) > + move_dead_notes (mov_insn, mem_insn.insn, inc_insn.reg0); > + else > + move_dead_notes (mov_insn, inc_insn.insn, inc_insn.reg0); > > regno = REGNO (inc_insn.reg_res); > reg_next_def[regno] = mov_insn; > @@ -842,7 +846,7 @@ find_address (rtx *address_of_x) > > if (code == MEM && rtx_equal_p (XEXP (x, 0), inc_insn.reg_res)) > { > - /* Match with *reg0. */ > + /* Match with *reg_res. */ > mem_insn.mem_loc = address_of_x; > mem_insn.reg0 = inc_insn.reg_res; > mem_insn.reg1_is_const = true; > @@ -850,6 +854,17 @@ find_address (rtx *address_of_x) > mem_insn.reg1 = GEN_INT (0); > return -1; > } > + if (code == MEM && inc_insn.reg1_is_const && inc_insn.reg0 > + && rtx_equal_p (XEXP (x, 0), inc_insn.reg0)) > + { > + /* Match with *reg0 AKA *(reg_res - reg1_val). */ > + mem_insn.mem_loc = address_of_x; > + mem_insn.reg0 = inc_insn.reg_res; > + mem_insn.reg1_is_const = true; > + mem_insn.reg1_val = -inc_insn.reg1_val; > + mem_insn.reg1 = GEN_INT (mem_insn.reg1_val); > + return -1; > + } > if (code == MEM && GET_CODE (XEXP (x, 0)) == PLUS > && rtx_equal_p (XEXP (XEXP (x, 0), 0), inc_insn.reg_res)) > { > @@ -1370,6 +1385,33 @@ merge_in_block (int max_reg, basic_block bb) > insn_is_add_or_inc = false; > } > } > + > + if (ok && insn_is_add_or_inc > + && inc_insn.reg0 != inc_insn.reg_res) > + { > + regno = REGNO (inc_insn.reg0); > + int luid = DF_INSN_LUID (mem_insn.insn); > + mem_insn.insn = get_next_ref (regno, bb, reg_next_use); > + > + /* Try a mem use of reg0 before that of reg_res > + too. If there's no further use of reg_res, > + there's no point in trying an auto-inc, and > + if the use of reg0 is after that of reg_res, > + it will be too late for the auto-inc to > + compute reg_res's correct value. */ > + if (mem_insn.insn > + && luid > DF_INSN_LUID (mem_insn.insn) > + && find_address (&PATTERN (mem_insn.insn)) == -1) > + { > + if (dump_file) > + dump_mem_insn (dump_file); > + if (try_merge ()) > + { > + success_in_block++; > + insn_is_add_or_inc = false; > + } > + } > + } > } > } > } > > > -- > Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ > You must be the change you wish to see in the world. -- Gandhi > Be Free! -- http://FSFLA.org/ FSF Latin America board member > Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer