Message ID | 20240801203934.1576508-1-quic_apinski@quicinc.com |
---|---|
State | New |
Headers | show |
Series | forwprop: Don't add uses to dce list if debug statement [PR116156] | expand |
On Thu, Aug 1, 2024 at 10:40 PM Andrew Pinski <quic_apinski@quicinc.com> wrote: > > The problem here is that when forwprop does a copy prop, into a statement, > we mark the uses of that statement as possibly need to be removed. But it just > happened that statement was a debug statement, there will be a difference when > compiling with debuging info turned on vs off; this is not expected. > So the fix is not to add the old use to dce list to process if it was a debug > statement. > > Bootstrapped and tested on x86_64-linux-gnu with no regressions. OK > PR tree-optimization/116156 > > gcc/ChangeLog: > > * tree-ssa-forwprop.cc (pass_forwprop::execute): Don't add > uses if the statement was a debug statement. > > gcc/testsuite/ChangeLog: > > * c-c++-common/torture/pr116156-1.c: New test. > > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> > --- > .../c-c++-common/torture/pr116156-1.c | 30 +++++++++++++++++++ > gcc/tree-ssa-forwprop.cc | 16 +++++----- > 2 files changed, 39 insertions(+), 7 deletions(-) > create mode 100644 gcc/testsuite/c-c++-common/torture/pr116156-1.c > > diff --git a/gcc/testsuite/c-c++-common/torture/pr116156-1.c b/gcc/testsuite/c-c++-common/torture/pr116156-1.c > new file mode 100644 > index 00000000000..10f938ef4e5 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/torture/pr116156-1.c > @@ -0,0 +1,30 @@ > +/* { dg-additional-options "-fcompare-debug" } */ > +/* PR tree-optimization/116156 */ > + > +/* Forwprop used to delete an unused statement > + but only with debug statements around. */ > + > +struct jpeg_compress_struct { > + int X_density; > +}; > +void gg(); > +int h(const char*,const char*) __attribute((pure)); > +int h1(const char*) __attribute((pure)); > +int f1() __attribute__((returns_twice)); > +void real_save_jpeg(char **keys, char *values) { > + struct jpeg_compress_struct cinfo; > + int x_density = 0; > + while (*keys) > + { > + if (h1(*keys) == 0) > + gg(); > + if (h1(*keys) == 0) { > + if (!*values) > + x_density = -1; > + if (x_density <= 0) > + gg(); > + } > + } > + if (f1()) > + cinfo.X_density = x_density; > +} > diff --git a/gcc/tree-ssa-forwprop.cc b/gcc/tree-ssa-forwprop.cc > index 44a6b5d39aa..2e37642359c 100644 > --- a/gcc/tree-ssa-forwprop.cc > +++ b/gcc/tree-ssa-forwprop.cc > @@ -3923,7 +3923,8 @@ pass_forwprop::execute (function *fun) > tree val = fwprop_ssa_val (use); > if (val && val != use) > { > - bitmap_set_bit (simple_dce_worklist, SSA_NAME_VERSION (use)); > + if (!is_gimple_debug (stmt)) > + bitmap_set_bit (simple_dce_worklist, SSA_NAME_VERSION (use)); > if (may_propagate_copy (use, val)) > { > propagate_value (usep, val); > @@ -3963,12 +3964,13 @@ pass_forwprop::execute (function *fun) > if (gimple_cond_true_p (cond) > || gimple_cond_false_p (cond)) > cfg_changed = true; > - /* Queue old uses for simple DCE. */ > - for (tree use : uses) > - if (TREE_CODE (use) == SSA_NAME > - && !SSA_NAME_IS_DEFAULT_DEF (use)) > - bitmap_set_bit (simple_dce_worklist, > - SSA_NAME_VERSION (use)); > + /* Queue old uses for simple DCE if not debug statement. */ > + if (!is_gimple_debug (stmt)) > + for (tree use : uses) > + if (TREE_CODE (use) == SSA_NAME > + && !SSA_NAME_IS_DEFAULT_DEF (use)) > + bitmap_set_bit (simple_dce_worklist, > + SSA_NAME_VERSION (use)); > } > > if (changed || substituted_p) > -- > 2.43.0 >
On Thu, Aug 1, 2024 at 11:55 PM Richard Biener <richard.guenther@gmail.com> wrote: > > On Thu, Aug 1, 2024 at 10:40 PM Andrew Pinski <quic_apinski@quicinc.com> wrote: > > > > The problem here is that when forwprop does a copy prop, into a statement, > > we mark the uses of that statement as possibly need to be removed. But it just > > happened that statement was a debug statement, there will be a difference when > > compiling with debuging info turned on vs off; this is not expected. > > So the fix is not to add the old use to dce list to process if it was a debug > > statement. > > > > Bootstrapped and tested on x86_64-linux-gnu with no regressions. > > OK Thanks Applied to the trunk and GCC 14 branch (since it is a regression there). Thanks, Andrew Pinski > > > PR tree-optimization/116156 > > > > gcc/ChangeLog: > > > > * tree-ssa-forwprop.cc (pass_forwprop::execute): Don't add > > uses if the statement was a debug statement. > > > > gcc/testsuite/ChangeLog: > > > > * c-c++-common/torture/pr116156-1.c: New test. > > > > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> > > --- > > .../c-c++-common/torture/pr116156-1.c | 30 +++++++++++++++++++ > > gcc/tree-ssa-forwprop.cc | 16 +++++----- > > 2 files changed, 39 insertions(+), 7 deletions(-) > > create mode 100644 gcc/testsuite/c-c++-common/torture/pr116156-1.c > > > > diff --git a/gcc/testsuite/c-c++-common/torture/pr116156-1.c b/gcc/testsuite/c-c++-common/torture/pr116156-1.c > > new file mode 100644 > > index 00000000000..10f938ef4e5 > > --- /dev/null > > +++ b/gcc/testsuite/c-c++-common/torture/pr116156-1.c > > @@ -0,0 +1,30 @@ > > +/* { dg-additional-options "-fcompare-debug" } */ > > +/* PR tree-optimization/116156 */ > > + > > +/* Forwprop used to delete an unused statement > > + but only with debug statements around. */ > > + > > +struct jpeg_compress_struct { > > + int X_density; > > +}; > > +void gg(); > > +int h(const char*,const char*) __attribute((pure)); > > +int h1(const char*) __attribute((pure)); > > +int f1() __attribute__((returns_twice)); > > +void real_save_jpeg(char **keys, char *values) { > > + struct jpeg_compress_struct cinfo; > > + int x_density = 0; > > + while (*keys) > > + { > > + if (h1(*keys) == 0) > > + gg(); > > + if (h1(*keys) == 0) { > > + if (!*values) > > + x_density = -1; > > + if (x_density <= 0) > > + gg(); > > + } > > + } > > + if (f1()) > > + cinfo.X_density = x_density; > > +} > > diff --git a/gcc/tree-ssa-forwprop.cc b/gcc/tree-ssa-forwprop.cc > > index 44a6b5d39aa..2e37642359c 100644 > > --- a/gcc/tree-ssa-forwprop.cc > > +++ b/gcc/tree-ssa-forwprop.cc > > @@ -3923,7 +3923,8 @@ pass_forwprop::execute (function *fun) > > tree val = fwprop_ssa_val (use); > > if (val && val != use) > > { > > - bitmap_set_bit (simple_dce_worklist, SSA_NAME_VERSION (use)); > > + if (!is_gimple_debug (stmt)) > > + bitmap_set_bit (simple_dce_worklist, SSA_NAME_VERSION (use)); > > if (may_propagate_copy (use, val)) > > { > > propagate_value (usep, val); > > @@ -3963,12 +3964,13 @@ pass_forwprop::execute (function *fun) > > if (gimple_cond_true_p (cond) > > || gimple_cond_false_p (cond)) > > cfg_changed = true; > > - /* Queue old uses for simple DCE. */ > > - for (tree use : uses) > > - if (TREE_CODE (use) == SSA_NAME > > - && !SSA_NAME_IS_DEFAULT_DEF (use)) > > - bitmap_set_bit (simple_dce_worklist, > > - SSA_NAME_VERSION (use)); > > + /* Queue old uses for simple DCE if not debug statement. */ > > + if (!is_gimple_debug (stmt)) > > + for (tree use : uses) > > + if (TREE_CODE (use) == SSA_NAME > > + && !SSA_NAME_IS_DEFAULT_DEF (use)) > > + bitmap_set_bit (simple_dce_worklist, > > + SSA_NAME_VERSION (use)); > > } > > > > if (changed || substituted_p) > > -- > > 2.43.0 > >
diff --git a/gcc/testsuite/c-c++-common/torture/pr116156-1.c b/gcc/testsuite/c-c++-common/torture/pr116156-1.c new file mode 100644 index 00000000000..10f938ef4e5 --- /dev/null +++ b/gcc/testsuite/c-c++-common/torture/pr116156-1.c @@ -0,0 +1,30 @@ +/* { dg-additional-options "-fcompare-debug" } */ +/* PR tree-optimization/116156 */ + +/* Forwprop used to delete an unused statement + but only with debug statements around. */ + +struct jpeg_compress_struct { + int X_density; +}; +void gg(); +int h(const char*,const char*) __attribute((pure)); +int h1(const char*) __attribute((pure)); +int f1() __attribute__((returns_twice)); +void real_save_jpeg(char **keys, char *values) { + struct jpeg_compress_struct cinfo; + int x_density = 0; + while (*keys) + { + if (h1(*keys) == 0) + gg(); + if (h1(*keys) == 0) { + if (!*values) + x_density = -1; + if (x_density <= 0) + gg(); + } + } + if (f1()) + cinfo.X_density = x_density; +} diff --git a/gcc/tree-ssa-forwprop.cc b/gcc/tree-ssa-forwprop.cc index 44a6b5d39aa..2e37642359c 100644 --- a/gcc/tree-ssa-forwprop.cc +++ b/gcc/tree-ssa-forwprop.cc @@ -3923,7 +3923,8 @@ pass_forwprop::execute (function *fun) tree val = fwprop_ssa_val (use); if (val && val != use) { - bitmap_set_bit (simple_dce_worklist, SSA_NAME_VERSION (use)); + if (!is_gimple_debug (stmt)) + bitmap_set_bit (simple_dce_worklist, SSA_NAME_VERSION (use)); if (may_propagate_copy (use, val)) { propagate_value (usep, val); @@ -3963,12 +3964,13 @@ pass_forwprop::execute (function *fun) if (gimple_cond_true_p (cond) || gimple_cond_false_p (cond)) cfg_changed = true; - /* Queue old uses for simple DCE. */ - for (tree use : uses) - if (TREE_CODE (use) == SSA_NAME - && !SSA_NAME_IS_DEFAULT_DEF (use)) - bitmap_set_bit (simple_dce_worklist, - SSA_NAME_VERSION (use)); + /* Queue old uses for simple DCE if not debug statement. */ + if (!is_gimple_debug (stmt)) + for (tree use : uses) + if (TREE_CODE (use) == SSA_NAME + && !SSA_NAME_IS_DEFAULT_DEF (use)) + bitmap_set_bit (simple_dce_worklist, + SSA_NAME_VERSION (use)); } if (changed || substituted_p)
The problem here is that when forwprop does a copy prop, into a statement, we mark the uses of that statement as possibly need to be removed. But it just happened that statement was a debug statement, there will be a difference when compiling with debuging info turned on vs off; this is not expected. So the fix is not to add the old use to dce list to process if it was a debug statement. Bootstrapped and tested on x86_64-linux-gnu with no regressions. PR tree-optimization/116156 gcc/ChangeLog: * tree-ssa-forwprop.cc (pass_forwprop::execute): Don't add uses if the statement was a debug statement. gcc/testsuite/ChangeLog: * c-c++-common/torture/pr116156-1.c: New test. Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> --- .../c-c++-common/torture/pr116156-1.c | 30 +++++++++++++++++++ gcc/tree-ssa-forwprop.cc | 16 +++++----- 2 files changed, 39 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/torture/pr116156-1.c