On Fri, 29 Jan 2021, Jakub Jelinek wrote:
> On Fri, Jan 29, 2021 at 04:43:49PM +0100, Richard Biener wrote:
> > 2021-01-29 Richard Biener <rguent...@suse.de>
> >
> > PR rtl-optimization/98863
> > * config/i386/i386-features.c (remove_partial_avx_dependency):
> > Do not perform DF analysis.
> > (pass_data_remove_partial_avx_dependency): Remove
> > TODO_df_finish.
> > ---
> > gcc/config/i386/i386-features.c | 17 +++++++----------
> > 1 file changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/gcc/config/i386/i386-features.c
> > b/gcc/config/i386/i386-features.c
> > index ef4f9406102..c845ba90caf 100644
> > --- a/gcc/config/i386/i386-features.c
> > +++ b/gcc/config/i386/i386-features.c
> > @@ -2272,6 +2272,9 @@ remove_partial_avx_dependency (void)
> >
> > auto_vec<rtx_insn *> control_flow_insns;
> >
> > + /* We create invalid RTL initially so defer rescans. */
> > + df_set_flags (DF_DEFER_INSN_RESCAN);
> > +
> > FOR_EACH_BB_FN (bb, cfun)
> > {
> > FOR_BB_INSNS (bb, insn)
> > @@ -2292,14 +2295,7 @@ remove_partial_avx_dependency (void)
> > continue;
> >
> > if (!v4sf_const0)
> > - {
> > - calculate_dominance_info (CDI_DOMINATORS);
> > - df_set_flags (DF_DEFER_INSN_RESCAN);
> > - df_chain_add_problem (DF_DU_CHAIN | DF_UD_CHAIN);
> > - df_md_add_problem ();
> > - df_analyze ();
> > - v4sf_const0 = gen_reg_rtx (V4SFmode);
> > - }
> > + v4sf_const0 = gen_reg_rtx (V4SFmode);
>
> Can't you do that df_set_flags (DF_DEFER_INSN_RESCAN):
> only here (when creating v4sf_const0)?
> >
> > /* Convert PARTIAL_XMM_UPDATE_TRUE insns, DF -> SF, SF -> DF,
> > SI -> SF, SI -> DF, DI -> SF, DI -> DF, to vec_dup and
> > @@ -2360,6 +2356,7 @@ remove_partial_avx_dependency (void)
> > {
> > /* (Re-)discover loops so that bb->loop_father can be used in the
> > analysis below. */
> > + calculate_dominance_info (CDI_DOMINATORS);
> > loop_optimizer_init (AVOID_CFG_MODIFICATIONS);
> >
> > /* Generate a vxorps at entry of the nearest dominator for basic
> > @@ -2391,7 +2388,6 @@ remove_partial_avx_dependency (void)
> > set_insn = emit_insn_after (set,
> > insn ? PREV_INSN (insn) : BB_END (bb));
> > df_insn_rescan (set_insn);
> > - df_process_deferred_rescans ();
>
> And keep df_process_deferred_rescans (); here
>
> > loop_optimizer_finalize ();
> >
> > if (!control_flow_insns.is_empty ())
> > @@ -2412,6 +2408,7 @@ remove_partial_avx_dependency (void)
> > }
> > }
> >
> > + df_process_deferred_rescans ();
>
> rather than here?
> The pass these days does two separate optimizations, and only one of those
> creates the use uninitialized pseudo and at the end initialize it.
> The other optimization is replace_constant_pool_with_broadcast, which
> doesn't do this but still calls validate_change -> df_insn_rescan.
But both are very cheap and IMHO the flow is less obfuscated this way.
Richard.
> > bitmap_obstack_release (NULL);
> > BITMAP_FREE (convert_bbs);
> >
> > @@ -2441,7 +2438,7 @@ const pass_data
> > pass_data_remove_partial_avx_dependency =
> > 0, /* properties_provided */
> > 0, /* properties_destroyed */
> > 0, /* todo_flags_start */
> > - TODO_df_finish, /* todo_flags_finish */
> > + 0, /* todo_flags_finish */
> > };
> >
> > class pass_remove_partial_avx_dependency : public rtl_opt_pass
> > --
> > 2.26.2
>
> Otherwise LGTM if it works.
>
> Jakub
