From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x629.google.com (mail-ej1-x629.google.com [IPv6:2a00:1450:4864:20::629]) by sourceware.org (Postfix) with ESMTPS id ADAE43858C2B for ; Thu, 5 Nov 2020 13:43:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org ADAE43858C2B Received: by mail-ej1-x629.google.com with SMTP id p5so2724200ejj.2 for ; Thu, 05 Nov 2020 05:43:27 -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:references:in-reply-to:from:date :message-id:subject:to:cc; bh=T4mOepiJXi/JkNkq79p2kcADSm1RHmBaE6annyi+UIQ=; b=JRc68EpS7GC4W90ZBm8hKMC2jX2wRrmkhAaFi5Cjjg1N4TPoWLqvqQlSOeMorz3ocA lH68Pl8y/cdx78Uc9eJruS8erspafnJoRiGpEs4qoG+kI5SZAWbp3Al7HXX2L92/SyZZ RKv6cifXvdrPmVIdPNogVR3LUB5VxLmgg7jSPIu47L4jahZb+nNKefz7nx7o4yQWe9uv +vbWV1msna7Hq0JIVv7KOVBwePBN3EVRIMr0uXw5Drf9cQ9/8b/XG/AJyj93Ym1V5wpb lDsUzEiA6Xoq17P1KovNODP/7+UHzDU4ugMpEACihPI06VjKiKxxt3tpVkvrYbBMzS2Y yC2w== X-Gm-Message-State: AOAM530ncBRxzZ69mGTUe0rM0DdZlgpdczVmjnKnYp833uYyZ8mzNWyK dq3eaRTHiUf8TiMZ6GozbUl/yXyBFE8N6Cdqnp4= X-Google-Smtp-Source: ABdhPJzf+QlUc3IvZWFomihr77SkX/MNi45PfYFb0C53rKxjjTfXmQQlXOBGY+1fEWY0BaeN25/ePJdg91a//SL7Ge0= X-Received: by 2002:a17:906:6949:: with SMTP id c9mr2334406ejs.482.1604583806586; Thu, 05 Nov 2020 05:43:26 -0800 (PST) MIME-Version: 1.0 References: <20201105131836.1043501-1-guojiufu@linux.ibm.com> In-Reply-To: <20201105131836.1043501-1-guojiufu@linux.ibm.com> From: Richard Biener Date: Thu, 5 Nov 2020 14:43:15 +0100 Message-ID: Subject: Re: [PATCH] Clean up loop-closed PHIs at loopdone pass To: guojiufu Cc: GCC Patches , Richard Guenther , Segher Boessenkool , Bill Schmidt , David Edelsohn Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Thu, 05 Nov 2020 13:43:29 -0000 On Thu, Nov 5, 2020 at 2:19 PM guojiufu via Gcc-patches wrote: > > In PR87473, there are discussions about loop-closed PHIs which > are generated for loop optimization passes. It would be helpful > to clean them up after loop optimization is done, then this may > simplify some jobs of following passes. > This patch introduces a cheaper way to propagate them out in > pass_tree_loop_done. > > This patch passes bootstrap and regtest on ppc64le. Is this ok for trunk? Huh, I think this is somewhat useless work, the PHIs won't survive for long and you certainly cannot expect degenerate PHIs to not occur anyway. You probably can replace propagate_rhs_into_lhs by the existing replace_uses_by function. You're walking loop exits after loop_optimizer_finalize () - that's wasting work. If you want to avoid inconsistent state and we really want to go with this I suggest to instead add a flag to loop_optimizer_finalize () as to whether to propagate out LC PHI nodes or not and do this from within there. Thanks, Richard. > gcc/ChangeLog > 2020-10-05 Jiufu Guo > > * tree-ssa-loop.h (clean_up_loop_closed_phi): New declaration. > * tree-ssa-loop.c (tree_ssa_loop_done): Call clean_up_loop_closed_phi. > * tree-ssa-propagate.c (propagate_rhs_into_lhs): New function. > > gcc/testsuite/ChangeLog > 2020-10-05 Jiufu Guo > > * gcc.dg/tree-ssa/loopclosedphi.c: New test. > --- > gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c | 21 +++ > gcc/tree-ssa-loop.c | 1 + > gcc/tree-ssa-loop.h | 1 + > gcc/tree-ssa-propagate.c | 120 ++++++++++++++++++ > 4 files changed, 143 insertions(+) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c b/gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c > new file mode 100644 > index 00000000000..d71b757fbca > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c > @@ -0,0 +1,21 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O3 -fno-tree-ch -w -fdump-tree-loopdone-details" } */ > + > +void > +t6 (int qz, int wh) > +{ > + int jl = wh; > + > + while (1.0 * qz / wh < 1) > + { > + qz = wh * (wh + 2); > + > + while (wh < 1) > + jl = 0; > + } > + > + while (qz < 1) > + qz = jl * wh; > +} > + > +/* { dg-final { scan-tree-dump-times "Replacing" 2 "loopdone"} } */ > diff --git a/gcc/tree-ssa-loop.c b/gcc/tree-ssa-loop.c > index 5e8365d4e83..7d680b2f5d2 100644 > --- a/gcc/tree-ssa-loop.c > +++ b/gcc/tree-ssa-loop.c > @@ -530,6 +530,7 @@ tree_ssa_loop_done (void) > free_numbers_of_iterations_estimates (cfun); > scev_finalize (); > loop_optimizer_finalize (); > + clean_up_loop_closed_phi (cfun); > return 0; > } > > diff --git a/gcc/tree-ssa-loop.h b/gcc/tree-ssa-loop.h > index 9e35125e6e8..baa940b9d1e 100644 > --- a/gcc/tree-ssa-loop.h > +++ b/gcc/tree-ssa-loop.h > @@ -67,6 +67,7 @@ public: > extern bool for_each_index (tree *, bool (*) (tree, tree *, void *), void *); > extern char *get_lsm_tmp_name (tree ref, unsigned n, const char *suffix = NULL); > extern unsigned tree_num_loop_insns (class loop *, struct eni_weights *); > +extern unsigned clean_up_loop_closed_phi (function *); > > /* Returns the loop of the statement STMT. */ > > diff --git a/gcc/tree-ssa-propagate.c b/gcc/tree-ssa-propagate.c > index 87dbf55fab9..813143852b9 100644 > --- a/gcc/tree-ssa-propagate.c > +++ b/gcc/tree-ssa-propagate.c > @@ -1549,4 +1549,123 @@ propagate_tree_value_into_stmt (gimple_stmt_iterator *gsi, tree val) > else > gcc_unreachable (); > } > + > +/* Propagate RHS into all uses of LHS (when possible). > + > + RHS and LHS are derived from STMT, which is passed in solely so > + that we can remove it if propagation is successful. */ > + > +static bool > +propagate_rhs_into_lhs (gphi *stmt, tree lhs, tree rhs) > +{ > + use_operand_p use_p; > + imm_use_iterator iter; > + gimple_stmt_iterator gsi; > + gimple *use_stmt; > + bool changed = false; > + bool all = true; > + > + /* Dump details. */ > + if (dump_file && (dump_flags & TDF_DETAILS)) > + { > + fprintf (dump_file, " Replacing '"); > + print_generic_expr (dump_file, lhs, dump_flags); > + fprintf (dump_file, "' with '"); > + print_generic_expr (dump_file, rhs, dump_flags); > + fprintf (dump_file, "'\n"); > + } > + > + /* Walk over every use of LHS and try to replace the use with RHS. */ > + FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs) > + { > + /* It is not safe to propagate into below stmts. */ > + if (gimple_debug_bind_p (use_stmt) > + || (gimple_code (use_stmt) == GIMPLE_ASM > + && !may_propagate_copy_into_asm (lhs)) > + || (TREE_CODE (rhs) == SSA_NAME > + && SSA_NAME_DEF_STMT (rhs) == use_stmt)) > + { > + all = false; > + continue; > + } > + > + /* Dump details. */ > + if (dump_file && (dump_flags & TDF_DETAILS)) > + { > + fprintf (dump_file, " Original statement:"); > + print_gimple_stmt (dump_file, use_stmt, 0, dump_flags); > + } > + > + /* Propagate the RHS into this use of the LHS. */ > + FOR_EACH_IMM_USE_ON_STMT (use_p, iter) > + propagate_value (use_p, rhs); > + > + /* Propagation may expose new operands to the renamer. */ > + update_stmt (use_stmt); > + > + /* If variable index is replaced with a constant, then > + update the invariant flag for ADDR_EXPRs. */ > + if (gimple_assign_single_p (use_stmt) > + && TREE_CODE (gimple_assign_rhs1 (use_stmt)) == ADDR_EXPR) > + recompute_tree_invariant_for_addr_expr (gimple_assign_rhs1 (use_stmt)); > + > + /* Dump details. */ > + if (dump_file && (dump_flags & TDF_DETAILS)) > + { > + fprintf (dump_file, " Updated statement:"); > + print_gimple_stmt (dump_file, use_stmt, 0, dump_flags); > + } > + > + changed = true; > + } > + > + /* Remove the degenerate PHI node. */ > + if (all) > + { > + gsi = gsi_for_stmt (stmt); > + remove_phi_node (&gsi, true); > + } > + > + return changed; > +} > + > +/* Check exits of each loop in FUN, walk over loop closed PHIs in > + each exit basic block and propagate degenerate PHIs. */ > + > +unsigned > +clean_up_loop_closed_phi (function *fun) > +{ > + unsigned i; > + edge e; > + gphi *phi; > + tree rhs; > + tree lhs; > + gphi_iterator gsi; > + struct loop *loop; > + bool cfg_altered = false; > + > + /* Walk over loop in function. */ > + FOR_EACH_LOOP_FN (fun, loop, 0) > + { > + /* Check each exit edge of loop. */ > + auto_vec exits = get_loop_exit_edges (loop); > + FOR_EACH_VEC_ELT (exits, i, e) > + if (single_pred_p (e->dest)) > + /* Walk over loop-closed PHIs. */ > + for (gsi = gsi_start_phis (e->dest); !gsi_end_p (gsi);) > + { > + phi = gsi.phi (); > + rhs = degenerate_phi_result (phi); > + lhs = gimple_phi_result (phi); > + > + /* Advance the iterator before stmt is removed. */ > + gsi_next (&gsi); > + > + if (rhs && !virtual_operand_p (lhs) > + && may_propagate_copy (lhs, rhs)) > + cfg_altered |= propagate_rhs_into_lhs (phi, lhs, rhs); > + } > + } > + > + return cfg_altered; > +} > -- > 2.25.1 >