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  <heli...@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  <heli...@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.
---
 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

Reply via email to