forwprop: Don't add uses to dce list if debug statement [PR116156]

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

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
> >