Fix a typo in two_value_replacement function
diff mbox series

Message ID 1557037872-47239-1-git-send-email-helijia@linux.ibm.com
State New
Headers show
Series
  • Fix a typo in two_value_replacement function
Related show

Commit Message

Li Jia He May 5, 2019, 6:31 a.m. UTC
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

Comments

Jakub Jelinek May 5, 2019, 9:48 a.m. UTC | #1
On Sun, May 05, 2019 at 01:31:12AM -0500, Li Jia He wrote:
> GCC revision 267634 implemented two_value_replacement function.
> However, a typo occurred during the parameter check, which caused
> us to miss some optimizations.

Thanks for catching this.

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

Ok for both, but see below.

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

Don't break a line after :, only when you reach 80 columns.  So:
	* tree-ssa-phiopt.c (two_value_replacement): Fix a typo in parameter
	detection.

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

Please don't rename tests unless really necessary.  Just keep pr88676.c
and add pr88676-2.c next to it.

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

Thus remove this hunk.

> rename from gcc/testsuite/gcc.dg/tree-ssa/pr88676.c
> rename to gcc/testsuite/gcc.dg/tree-ssa/pr88676-1.c

And this one.

	Jakub
Christophe Lyon May 6, 2019, 11:19 a.m. UTC | #2
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
>
Jakub Jelinek May 6, 2019, 11:35 a.m. UTC | #3
On Mon, May 06, 2019 at 01:19:15PM +0200, Christophe Lyon wrote:
> > 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 ?

While the posted patch had:
> > +/* PR tree-optimization/88676 */
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -fdump-tree-optimized" } */
> > +/* { dg-final { scan-tree-dump-not " = PHI <" "optimized" } } */
the actually committed patch has:
/* PR tree-optimization/88676 */
/* { dg-do compile } */
/* { dg-options "-O2 -fdump-tree-phiopt1" } */
/* { dg-final { scan-tree-dump-not " = PHI <" "optimized" } } */

Dunno why this changed, if you want it in phiopt1, you need "phiopt1"
in scan-tree-dump-not as well, if you want optimized dump, you need
-fdump-tree-optimized instead.
> 
> 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?

	Jakub
Li Jia He May 6, 2019, 12:22 p.m. UTC | #4
On 2019/5/6 7:35 PM, Jakub Jelinek wrote:
> On Mon, May 06, 2019 at 01:19:15PM +0200, Christophe Lyon wrote:
>>> 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 ?
> 
> While the posted patch had:
>>> +/* PR tree-optimization/88676 */
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-O2 -fdump-tree-optimized" } */
>>> +/* { dg-final { scan-tree-dump-not " = PHI <" "optimized" } } */
> the actually committed patch has:
> /* PR tree-optimization/88676 */
> /* { dg-do compile } */
> /* { dg-options "-O2 -fdump-tree-phiopt1" } */
> /* { dg-final { scan-tree-dump-not " = PHI <" "optimized" } } */
> 
> Dunno why this changed, if you want it in phiopt1, you need "phiopt1"
> in scan-tree-dump-not as well, if you want optimized dump, you need
> -fdump-tree-optimized instead.
When I test the code again for submiting the code, I found that this 
change only affects the dump file generated by phiopt1,
so I rashly decided to change the test option to dump-tree-phiopt1. If 
there is any code change in the future, I will send another patch. 
Sorry for the error caused by this.
>>
>> 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?
> 
> 	Jakub
>
Jakub Jelinek May 6, 2019, 12:27 p.m. UTC | #5
On Mon, May 06, 2019 at 08:22:41PM +0800, Li Jia He wrote:
> > Dunno why this changed, if you want it in phiopt1, you need "phiopt1"
> > in scan-tree-dump-not as well, if you want optimized dump, you need
> > -fdump-tree-optimized instead.
> When I test the code again for submiting the code, I found that this change
> only affects the dump file generated by phiopt1,
> so I rashly decided to change the test option to dump-tree-phiopt1. If there
> is any code change in the future, I will send another patch. Sorry for the
> error caused by this.

Even if you rush up such a change (happens sometimes), if it is testsuite
only you don't really need to bootstrap/regtest fully again, but retesting
the single testcase is needed.  So in objdir/gcc
make check-gcc RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} tree-ssa.exp=pr88676*'
in this case if the target is multilib, or
make check-gcc RUNTESTFLAGS='tree-ssa.exp=pr88676*'
otherwise.

	Jakub
Li Jia He May 6, 2019, 12:30 p.m. UTC | #6
On 2019/5/6 8:27 PM, Jakub Jelinek wrote:
> On Mon, May 06, 2019 at 08:22:41PM +0800, Li Jia He wrote:
>>> Dunno why this changed, if you want it in phiopt1, you need "phiopt1"
>>> in scan-tree-dump-not as well, if you want optimized dump, you need
>>> -fdump-tree-optimized instead.
>> When I test the code again for submiting the code, I found that this change
>> only affects the dump file generated by phiopt1,
>> so I rashly decided to change the test option to dump-tree-phiopt1. If there
>> is any code change in the future, I will send another patch. Sorry for the
>> error caused by this.
> 
> Even if you rush up such a change (happens sometimes), if it is testsuite
> only you don't really need to bootstrap/regtest fully again, but retesting
> the single testcase is needed.  So in objdir/gcc
> make check-gcc RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} tree-ssa.exp=pr88676*'
> in this case if the target is multilib, or
> make check-gcc RUNTESTFLAGS='tree-ssa.exp=pr88676*'
> otherwise.
Thank you very much for this suggestion, I will be very careful in the 
future.
> 
> 	Jakub
>
Li Jia He May 6, 2019, 1:24 p.m. UTC | #7
On 2019/5/6 7:19 PM, Christophe Lyon wrote:
> 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?
Sorry for this error, I have reverted it.  Next time I will test it
well.
> 
> 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
>>
>

Patch
diff mbox series

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