On Mon, 19 Jun 2023, Jan Hubicka wrote: > Hi, > this patch avoids unnecessary post dominator and update_ssa in phiprop. > > Bootstrapped/regtested x86_64-linux, OK? > > gcc/ChangeLog: > > * tree-ssa-phiprop.cc (propagate_with_phi): Add > post_dominators_computed; > compute post dominators lazilly. > (const pass_data pass_data_phiprop): Remove TODO_update_ssa. > (pass_phiprop::execute): Update; return TODO_update_ssa if something > changed. > > diff --git a/gcc/tree-ssa-phiprop.cc b/gcc/tree-ssa-phiprop.cc > index 3cb4900b6be..87e3a2ccf3a 100644 > --- a/gcc/tree-ssa-phiprop.cc > +++ b/gcc/tree-ssa-phiprop.cc > @@ -260,7 +260,7 @@ chk_uses (tree, tree *idx, void *data) > > static bool > propagate_with_phi (basic_block bb, gphi *phi, struct phiprop_d *phivn, > - size_t n) > + size_t n, bool *post_dominators_computed) > { > tree ptr = PHI_RESULT (phi); > gimple *use_stmt; > @@ -324,6 +324,12 @@ propagate_with_phi (basic_block bb, gphi *phi, struct > phiprop_d *phivn, > gimple *def_stmt; > tree vuse; > > + if (!*post_dominators_computed) > + { > + calculate_dominance_info (CDI_POST_DOMINATORS); > + *post_dominators_computed = true;
I think you can save the parameter by using dom_info_available_p () here and ... > + } > + > /* Only replace loads in blocks that post-dominate the PHI node. That > makes sure we don't end up speculating loads. */ > if (!dominated_by_p (CDI_POST_DOMINATORS, > @@ -465,7 +471,7 @@ const pass_data pass_data_phiprop = > 0, /* properties_provided */ > 0, /* properties_destroyed */ > 0, /* todo_flags_start */ > - TODO_update_ssa, /* todo_flags_finish */ > + 0, /* todo_flags_finish */ > }; > > class pass_phiprop : public gimple_opt_pass > @@ -490,9 +497,9 @@ pass_phiprop::execute (function *fun) > gphi_iterator gsi; > unsigned i; > size_t n; > + bool post_dominators_computed = false; > > calculate_dominance_info (CDI_DOMINATORS); > - calculate_dominance_info (CDI_POST_DOMINATORS); > > n = num_ssa_names; > phivn = XCNEWVEC (struct phiprop_d, n); > @@ -508,7 +515,8 @@ pass_phiprop::execute (function *fun) > if (bb_has_abnormal_pred (bb)) > continue; > for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi)) > - did_something |= propagate_with_phi (bb, gsi.phi (), phivn, n); > + did_something |= propagate_with_phi (bb, gsi.phi (), phivn, n, > + &post_dominators_computed); > } > > if (did_something) > @@ -516,9 +524,10 @@ pass_phiprop::execute (function *fun) > > free (phivn); > > - free_dominance_info (CDI_POST_DOMINATORS); > + if (post_dominators_computed) > + free_dominance_info (CDI_POST_DOMINATORS); unconditionally free_dominance_info here. > - return 0; > + return did_something ? TODO_update_ssa : 0; I guess that change is following general practice and good to catch undesired changes (update_ssa will exit early when there's nothing to do anyway). OK with those changes.