[PATCH] Fix a typo in two_value_replacement function

[Christophe Lyon]

On Sun, 5 May 2019 at 08:31, Li Jia He <helijia@linux.ibm.com> wrote:
>
> Hi,
>
> GCC revision 267634 implemented two_value_replacement function.
> However, a typo occurred during the parameter check, which caused
> us to miss some optimizations.
>
> The intent of the code might be to check that the input parameters
> are const int and their difference is one.  However, when I read
> the code, I found that it is wrong to detect whether an input data
> plus one is equal to itself.  This could be a typo.
>
> The regression testing for the patch was done on GCC mainline on
>
>     powerpc64le-unknown-linux-gnu (Power 9 LE)
>
> with no regressions.  Is it OK for trunk and backport to gcc 9 ?
>
> Thanks,
> Lijia He
>
> gcc/ChangeLog
>
> 2019-05-05  Li Jia He  <helijia@linux.ibm.com>
>
>         * tree-ssa-phiopt.c (two_value_replacement):
>         Fix a typo in parameter detection.
>
> gcc/testsuite/ChangeLog
>
> 2019-05-05  Li Jia He  <helijia@linux.ibm.com>
>
>         * gcc.dg/pr88676.c: Modify the include header file.
>         * gcc.dg/tree-ssa/pr37508.c: Add the no-ssa-phiopt option to
>         skip phi optimization.
>         * gcc.dg/tree-ssa/pr88676.c: Rename to ...
>         * gcc.dg/tree-ssa/pr88676-1.c: ... this new file.
>         * gcc.dg/tree-ssa/pr88676-2.c: New testcase.

Hi,

This new testcase fails on arm and aarch64:
PASS: gcc.dg/tree-ssa/pr88676-2.c (test for excess errors)
UNRESOLVED: gcc.dg/tree-ssa/pr88676-2.c scan-tree-dump-not optimized " = PHI <"
because:
gcc.dg/tree-ssa/pr88676-2.c: dump file does not exist

Can you fix this?

Thanks,

Christophe

> ---
>  gcc/testsuite/gcc.dg/pr88676.c                |  2 +-
>  gcc/testsuite/gcc.dg/tree-ssa/pr37508.c       |  6 ++--
>  .../tree-ssa/{pr88676.c => pr88676-1.c}       |  0
>  gcc/testsuite/gcc.dg/tree-ssa/pr88676-2.c     | 30 +++++++++++++++++++
>  gcc/tree-ssa-phiopt.c                         |  2 +-
>  5 files changed, 35 insertions(+), 5 deletions(-)
>  rename gcc/testsuite/gcc.dg/tree-ssa/{pr88676.c => pr88676-1.c} (100%)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr88676-2.c
>
> diff --git a/gcc/testsuite/gcc.dg/pr88676.c b/gcc/testsuite/gcc.dg/pr88676.c
> index b5fdd9342b8..719208083ae 100644
> --- a/gcc/testsuite/gcc.dg/pr88676.c
> +++ b/gcc/testsuite/gcc.dg/pr88676.c
> @@ -2,7 +2,7 @@
>  /* { dg-do run } */
>  /* { dg-options "-O2" } */
>
> -#include "tree-ssa/pr88676.c"
> +#include "tree-ssa/pr88676-1.c"
>
>  __attribute__((noipa)) void
>  bar (int x, int y, int z)
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr37508.c b/gcc/testsuite/gcc.dg/tree-ssa/pr37508.c
> index 2ba09afe481..a6def045de4 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/pr37508.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr37508.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fno-tree-fre -fdump-tree-vrp1" } */
> +/* { dg-options "-O2 -fno-ssa-phiopt -fno-tree-fre -fdump-tree-vrp1" } */
>
>  struct foo1 {
>    int i:1;
> @@ -22,7 +22,7 @@ int test2 (struct foo2 *x)
>  {
>    if (x->i == 0)
>      return 1;
> -  else if (x->i == -1) /* This test is already folded to false by ccp1.  */
> +  else if (x->i == -1) /* This test is already optimized by ccp1 or phiopt1.  */
>      return 1;
>    return 0;
>  }
> @@ -31,7 +31,7 @@ int test3 (struct foo1 *x)
>  {
>    if (x->i == 0)
>      return 1;
> -  else if (x->i == 1) /* This test is already folded to false by fold.  */
> +  else if (x->i == 1) /* This test is already optimized by ccp1 or phiopt1.  */
>      return 1;
>    return 0;
>  }
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr88676.c b/gcc/testsuite/gcc.dg/tree-ssa/pr88676-1.c
> similarity index 100%
> rename from gcc/testsuite/gcc.dg/tree-ssa/pr88676.c
> rename to gcc/testsuite/gcc.dg/tree-ssa/pr88676-1.c
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr88676-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr88676-2.c
> new file mode 100644
> index 00000000000..d377948e14d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr88676-2.c
> @@ -0,0 +1,30 @@
> +/* PR tree-optimization/88676 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +/* { dg-final { scan-tree-dump-not " = PHI <" "optimized" } } */
> +
> +struct foo1 {
> +  int i:1;
> +};
> +struct foo2 {
> +  unsigned i:1;
> +};
> +
> +int test1 (struct foo1 *x)
> +{
> +  if (x->i == 0)
> +    return 1;
> +  else if (x->i == 1)
> +    return 1;
> +  return 0;
> +}
> +
> +int test2 (struct foo2 *x)
> +{
> +  if (x->i == 0)
> +    return 1;
> +  else if (x->i == -1)
> +    return 1;
> +  return 0;
> +}
> +
> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> index 219791ea4ba..90674a2f3c4 100644
> --- a/gcc/tree-ssa-phiopt.c
> +++ b/gcc/tree-ssa-phiopt.c
> @@ -602,7 +602,7 @@ two_value_replacement (basic_block cond_bb, basic_block middle_bb,
>        || TREE_CODE (arg1) != INTEGER_CST
>        || (tree_int_cst_lt (arg0, arg1)
>           ? wi::to_widest (arg0) + 1 != wi::to_widest (arg1)
> -         : wi::to_widest (arg1) + 1 != wi::to_widest (arg1)))
> +         : wi::to_widest (arg1) + 1 != wi::to_widest (arg0)))
>      return false;
>
>    if (!empty_block_p (middle_bb))
> --
> 2.17.1
>