public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Li Jia He <helijia@linux.ibm.com>
To: gcc-patches@gcc.gnu.org, segher@kernel.crashing.org,
	       wschmidt@linux.ibm.com, rguenther@suse.de,
	jakub@redhat.com
Cc: Li Jia He <helijia@linux.ibm.com>
Subject: [PATCH] Fix a typo in two_value_replacement function
Date: Sun, 05 May 2019 06:31:00 -0000	[thread overview]
Message-ID: <1557037872-47239-1-git-send-email-helijia@linux.ibm.com> (raw)

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.
---
 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	other threads:[~2019-05-05  6:31 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-05  6:31 Li Jia He [this message]
2019-05-05  9:48 ` Jakub Jelinek
2019-05-06 11:19 ` Christophe Lyon
2019-05-06 11:35   ` Jakub Jelinek
2019-05-06 12:22     ` Li Jia He
2019-05-06 12:27       ` Jakub Jelinek
2019-05-06 12:30         ` Li Jia He
2019-05-06 13:24   ` Li Jia He

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1557037872-47239-1-git-send-email-helijia@linux.ibm.com \
    --to=helijia@linux.ibm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=rguenther@suse.de \
    --cc=segher@kernel.crashing.org \
    --cc=wschmidt@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).