| Message ID | 20241012220704.372504-1-quic_apinski@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | DSE: add remove_unused_locals to the todos [PR117096] | expand |
On Sun, Oct 13, 2024 at 12:07 AM Andrew Pinski <quic_apinski@quicinc.com> wrote: > > This is a better patch to fix PR 117096 (phiopt vs clobbers). > The only time we remove clobbers of local variables is during > remove_unused_locals. Since DSE might remove all of the stores > to a local variable, it makes sense to also try to remove unused local > variables afterwards. > This shows up more in C++ code with encapsulation. > > This was previously mentioned to be done: > https://gcc.gnu.org/pipermail/gcc-patches/2021-April/567744.html > In the end it was chosen "Or we might want to place explicit schedules of > remove_unused_locals in the pass pipeline." (in stdargs, > https://gcc.gnu.org/pipermail/gcc-patches/2021-April/568807.html) but that is too > late for early phiopt. This patch does the "maybe we should unconditionally do so" > option instead. > > Bootstrapped and tested on x86_64-linux-gnu. We've been very "careful" with placing this TODO it seems. Note that now all DSE pass invocations are followed by DCE which knows how to "lazily" remove _indirect_ CLOBBERs and it can elide loads. So I think it makes more sense to stick the TODO_remove_unused_locals on the DCE pass? Maybe add a 2nd pass parameter indicating whether to remove unused locals after it? Richard. > gcc/ChangeLog: > > PR tree-optimization/117096 > * tree-ssa-dse.cc (pass_data_dse): Add TODO_remove_unused_locals > to todo_flags_finish. > > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> > --- > gcc/tree-ssa-dse.cc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/tree-ssa-dse.cc b/gcc/tree-ssa-dse.cc > index 63bf4491cf6..42a133b8631 100644 > --- a/gcc/tree-ssa-dse.cc > +++ b/gcc/tree-ssa-dse.cc > @@ -1654,7 +1654,7 @@ const pass_data pass_data_dse = > 0, /* properties_provided */ > 0, /* properties_destroyed */ > 0, /* todo_flags_start */ > - 0, /* todo_flags_finish */ > + TODO_remove_unused_locals, /* todo_flags_finish */ > }; > > class pass_dse : public gimple_opt_pass > -- > 2.43.0 >
diff --git a/gcc/tree-ssa-dse.cc b/gcc/tree-ssa-dse.cc index 63bf4491cf6..42a133b8631 100644 --- a/gcc/tree-ssa-dse.cc +++ b/gcc/tree-ssa-dse.cc @@ -1654,7 +1654,7 @@ const pass_data pass_data_dse = 0, /* properties_provided */ 0, /* properties_destroyed */ 0, /* todo_flags_start */ - 0, /* todo_flags_finish */ + TODO_remove_unused_locals, /* todo_flags_finish */ }; class pass_dse : public gimple_opt_pass
This is a better patch to fix PR 117096 (phiopt vs clobbers). The only time we remove clobbers of local variables is during remove_unused_locals. Since DSE might remove all of the stores to a local variable, it makes sense to also try to remove unused local variables afterwards. This shows up more in C++ code with encapsulation. This was previously mentioned to be done: https://gcc.gnu.org/pipermail/gcc-patches/2021-April/567744.html In the end it was chosen "Or we might want to place explicit schedules of remove_unused_locals in the pass pipeline." (in stdargs, https://gcc.gnu.org/pipermail/gcc-patches/2021-April/568807.html) but that is too late for early phiopt. This patch does the "maybe we should unconditionally do so" option instead. Bootstrapped and tested on x86_64-linux-gnu. gcc/ChangeLog: PR tree-optimization/117096 * tree-ssa-dse.cc (pass_data_dse): Add TODO_remove_unused_locals to todo_flags_finish. Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> --- gcc/tree-ssa-dse.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)