On Thu, Jul 8, 2021 at 11:50 PM Richard Biener via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Fri, Jul 9, 2021 at 7:34 AM apinski--- via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > From: Andrew Pinski <apin...@marvell.com>
> >
> > Instead of waiting for DCE to remove the unused statement,
> > and maybe optimize another conditional, it is better if
> > we don't move the statement and have the statement
> > removed.
> >
> > gcc/ChangeLog:
> >
> > * tree-ssa-phiopt.c (used_in_seq): New function.
> > (match_simplify_replacement): Don't move the statement
> > if not used in sequence.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.dg/tree-ssa/pr96928-1.c: Update to similar as pr96928.c.
> > ---
> > gcc/testsuite/gcc.dg/tree-ssa/pr96928-1.c | 5 ++++-
> > gcc/tree-ssa-phiopt.c | 24 ++++++++++++++++++++++-
> > 2 files changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr96928-1.c
> > b/gcc/testsuite/gcc.dg/tree-ssa/pr96928-1.c
> > index 2e86620da11..9e505ac9900 100644
> > --- a/gcc/testsuite/gcc.dg/tree-ssa/pr96928-1.c
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr96928-1.c
> > @@ -2,7 +2,10 @@
> > /* { dg-do compile } */
> > /* { dg-options "-O2 -fdump-tree-phiopt2 -fdump-tree-optimized" } */
> > /* { dg-final { scan-tree-dump-times " = a_\[0-9]*\\\(D\\\) >> " 5
> > "phiopt2" } } */
> > -/* { dg-final { scan-tree-dump-times " = ~c_\[0-9]*\\\(D\\\);" 1 "phiopt2"
> > } } */
> > +/* The following check is done at optimized because a ^ (~b) is rewritten
> > as ~(a^b)
> > + and in the case of match.pd optimizing these ?:, the ~ is moved out
> > already
> > + by the time we get to phiopt2. */
> > +/* { dg-final { scan-tree-dump-times "c_\[0-9]*\\\(D\\\) \\\^" 1
> > "optimized" } } */
> > /* { dg-final { scan-tree-dump-times " = ~" 1 "optimized" } } */
> > /* { dg-final { scan-tree-dump-times " = \[abc_0-9\\\(\\\)D]* \\\^ " 5
> > "phiopt2" } } */
> > /* { dg-final { scan-tree-dump-not "a < 0" "phiopt2" } } */
> > diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> > index 7a98b7afdf1..a237df02153 100644
> > --- a/gcc/tree-ssa-phiopt.c
> > +++ b/gcc/tree-ssa-phiopt.c
> > @@ -934,6 +934,26 @@ gimple_simplify_phiopt (bool early_p, tree type,
> > gimple *comp_stmt,
> > return NULL;
> > }
> >
> > +/* Return true if the lhs of STMT is used in the SEQ sequence
> > + of statements. */
> > +static bool
> > +used_in_seq (gimple *stmt, gimple_seq seq)
> > +{
> > + tree lhs = gimple_assign_lhs (stmt);
> > + for (auto gsi = gsi_start (seq); !gsi_end_p (gsi); gsi_next_nondebug
> > (&gsi))
> > + {
> > + use_operand_p use_p;
> > + ssa_op_iter iter;
> > + gimple *stmt1 = gsi_stmt (gsi);
> > + FOR_EACH_SSA_USE_OPERAND (use_p, stmt1, iter, SSA_OP_USE)
> > + {
> > + if (USE_FROM_PTR (use_p) == lhs)
> > + return true;
> > + }
> > + }
> > + return false;
> > +}
> > +
> > /* The function match_simplify_replacement does the main work of doing the
> > replacement using match and simplify. Return true if the replacement
> > is done.
> > Otherwise return false.
> > @@ -1020,7 +1040,9 @@ match_simplify_replacement (basic_block cond_bb,
> > basic_block middle_bb,
> > return false;
> >
> > gsi = gsi_last_bb (cond_bb);
> > - if (stmt_to_move)
> > + if (stmt_to_move
> > + && (gimple_assign_lhs (stmt_to_move) == result
> > + || used_in_seq (stmt_to_move, seq)))
>
> Err, why not insert 'seq' before moving the stmt (you'd have to fiddle
> with the iterator,
> using GSI_CONTINUE_LINKING I think) and then check has_zero_uses on
> the (hopefully) only
> def of the stmt to move?
Because stmt_to_move was used in the phi and if we move
replace_phi_edge_with_variable before we move the statement, the
statement has been removed permanently as the basic block holding it
has been deleted.
What about this order instead:
remove stmt_to_move (not permanently)
call replace_phi_edge_with_variable
insert seq
if !zero_uses
insert stmt_to_move before seq
else
release defs for stmt_to_move
Thanks,
Andrew Pinski
>
> Richard.
>
> > {
> > if (dump_file && (dump_flags & TDF_DETAILS))
> > {
> > --
> > 2.27.0
> >
