On Tue, 27 Apr 2021 at 19:19, Richard Biener <rguent...@suse.de> wrote:
>
> DSE performs a backwards walk over stmts removing stores but it
> leaves removing resulting dead SSA defs to later passes. This
> eats into its own alias walking budget if the removed stores kept
> loads live. The following patch adds removal of trivially dead
> SSA defs which helps in this situation and reduces the amount of
> garbage followup passes need to deal with.
>
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>
> 2021-04-27 Richard Biener <rguent...@suse.de>
>
> PR tree-optimization/99912
> * tree-ssa-dse.c (dse_dom_walker::m_need_cfg_cleanup): New.
> (dse_dom_walker::todo): Likewise.
> (dse_dom_walker::dse_optimize_stmt): Move VDEF check to the
> caller.
> (dse_dom_walker::before_dom_children): Remove trivially
> dead SSA defs and schedule CFG cleanup if we removed all
> PHIs in a block.
> (pass_dse::execute): Get TODO as computed by the DOM walker
> and return it. Wipe dominator info earlier.
> ---
> gcc/tree-ssa-dse.c | 65 +++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 53 insertions(+), 12 deletions(-)
>
> diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c
> index 4967a5a9927..f5f39cbe903 100644
> --- a/gcc/tree-ssa-dse.c
> +++ b/gcc/tree-ssa-dse.c
> @@ -963,16 +963,25 @@ public:
> dse_dom_walker (cdi_direction direction)
> : dom_walker (direction),
> m_live_bytes (param_dse_max_object_size),
> - m_byte_tracking_enabled (false) {}
> + m_byte_tracking_enabled (false),
> + m_need_cfg_cleanup (false) {}
>
> virtual edge before_dom_children (basic_block);
> + unsigned todo () const;
>
> private:
> auto_sbitmap m_live_bytes;
> bool m_byte_tracking_enabled;
> + bool m_need_cfg_cleanup;
> void dse_optimize_stmt (gimple_stmt_iterator *);
> };
>
> +unsigned
> +dse_dom_walker::todo () const
> +{
> + return m_need_cfg_cleanup ? TODO_cleanup_cfg : 0;
> +}
> +
> /* Delete a dead call at GSI, which is mem* call of some kind. */
> static void
> delete_dead_or_redundant_call (gimple_stmt_iterator *gsi, const char *type)
> @@ -1049,11 +1058,6 @@ dse_dom_walker::dse_optimize_stmt
> (gimple_stmt_iterator *gsi)
> {
> gimple *stmt = gsi_stmt (*gsi);
>
> - /* If this statement has no virtual defs, then there is nothing
> - to do. */
> - if (!gimple_vdef (stmt))
> - return;
> -
> /* Don't return early on *this_2(D) ={v} {CLOBBER}. */
> if (gimple_has_volatile_ops (stmt)
> && (!gimple_clobber_p (stmt)
> @@ -1180,12 +1184,47 @@ dse_dom_walker::before_dom_children (basic_block bb)
>
> for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi);)
> {
> - dse_optimize_stmt (&gsi);
> + gimple *stmt = gsi_stmt (gsi);
> +
> + if (gimple_vdef (stmt))
> + dse_optimize_stmt (&gsi);
> + else if (def_operand_p def_p = single_ssa_def_operand (stmt,
> SSA_OP_DEF))
> + {
> + /* When we remove dead stores make sure to also delete trivially
> + dead SSA defs. */
> + if (has_zero_uses (DEF_FROM_PTR (def_p))
> + && !gimple_has_side_effects (stmt))
> + {
> + if (dump_file && (dump_flags & TDF_DETAILS))
> + {
> + fprintf (dump_file, " Deleted trivially dead stmt: ");
> + print_gimple_stmt (dump_file, stmt, 0, dump_flags);
> + fprintf (dump_file, "\n");
> + }
> + if (gsi_remove (&gsi, true) && need_eh_cleanup)
> + bitmap_set_bit (need_eh_cleanup, bb->index);
> + release_defs (stmt);
> + }
> + }
> if (gsi_end_p (gsi))
> gsi = gsi_last_bb (bb);
> else
> gsi_prev (&gsi);
> }
> + bool removed_phi = false;
> + for (gphi_iterator si = gsi_start_phis (bb); !gsi_end_p (si);)
> + {
> + gphi *phi = si.phi ();
> + if (has_zero_uses (gimple_phi_result (phi)))
> + {
> + remove_phi_node (&si, true);
> + removed_phi = true;
> + }
Hi Richard,
Just curious if this is missing dumping info about removed phi node to
dump_file ?
Thanks,
Prathamesh
> + else
> + gsi_next (&si);
> + }
> + if (removed_phi && gimple_seq_empty_p (phi_nodes (bb)))
> + m_need_cfg_cleanup = true;
> return NULL;
> }
>
> @@ -1234,21 +1273,23 @@ pass_dse::execute (function *fun)
>
> /* Dead store elimination is fundamentally a walk of the post-dominator
> tree and a backwards walk of statements within each block. */
> - dse_dom_walker (CDI_POST_DOMINATORS).walk (fun->cfg->x_exit_block_ptr);
> + dse_dom_walker walker (CDI_POST_DOMINATORS);
> + walker.walk (fun->cfg->x_exit_block_ptr);
> + free_dominance_info (CDI_POST_DOMINATORS);
> +
> + unsigned todo = walker.todo ();
>
> /* Removal of stores may make some EH edges dead. Purge such edges from
> the CFG as needed. */
> if (!bitmap_empty_p (need_eh_cleanup))
> {
> gimple_purge_all_dead_eh_edges (need_eh_cleanup);
> - cleanup_tree_cfg ();
> + todo |= TODO_cleanup_cfg;
> }
>
> BITMAP_FREE (need_eh_cleanup);
>
> - /* For now, just wipe the post-dominator information. */
> - free_dominance_info (CDI_POST_DOMINATORS);
> - return 0;
> + return todo;
> }
>
> } // anon namespace
> --
> 2.26.2
