testsuite: Fix up tree-ssa/pr103514.c testcase [PR103514]

On Fri, Jan 28, 2022 at 03:14:16PM -0700, Jeff Law via Gcc-patches wrote:
> > This patch will add the missed pattern described in bug 103514 [1] to the match.pd. [1] includes proof of correctness for the patch too.
> > 
> > PR tree-optimization/103514
> > 	* match.pd (a & b) ^ (a == b) -> !(a | b): New optimization.
> > 	* match.pd (a & b) == (a ^ b) -> !(a | b): New optimization.
> > 	* gcc.dg/tree-ssa/pr103514.c: Testcase for this optimization.
> > 
> > 1) https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103514
> Note the bug was filed an fixed during stage3, review just didn't happen in
> a reasonable timeframe.
> 
> I'm going to ACK this for the trunk and go ahead and commit it for you.

The testcase FAILs on short-circuit targets like powerpc64le-linux.
While the first 2 functions are identical, the last two look like:
  <bb 2> :
  if (a_5(D) != 0)
    goto <bb 3>; [INV]
  else
    goto <bb 4>; [INV]

  <bb 3> :
  if (b_6(D) != 0)
    goto <bb 5>; [INV]
  else
    goto <bb 4>; [INV]

  <bb 4> :

  <bb 5> :
  # iftmp.1_4 = PHI <1(3), 0(4)>
  _1 = a_5(D) == b_6(D);
  _2 = (int) _1;
  _3 = _2 ^ iftmp.1_4;
  _9 = _2 != iftmp.1_4;
  return _9;
instead of the expected:
  <bb 2> :
  _3 = a_8(D) & b_9(D);
  _4 = (int) _3;
  _5 = a_8(D) == b_9(D);
  _6 = (int) _5;
  _1 = a_8(D) | b_9(D);
  _2 = ~_1;
  _7 = (int) _2;
  _10 = ~_1;
  return _10;
so no wonder it doesn't match.  E.g. x86_64-linux will also use jumps
if it isn't just a && b but a && b && c && d (will do
a & b and c & d tests and jump based on those.

As it is too late to implement this optimization even for the short
circuiting targets this late (not even sure which pass would be best),
this patch just forces non-short-circuiting for the test.

Tested on x86_64-linux -m32/-m64 and powerpc64le-linux, ok for trunk?

2022-01-29  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/103514
	* gcc.dg/tree-ssa/pr103514.c: Add
	--param logical-op-non-short-circuit=1 to dg-options.



	Jakub

Thanks Jakob for the correction. Sadly, I didn’t have any access to any non x86 architecture. But x86 was fully tested and there was no regression.

In my spare time I will look at implementation of this for short-circuit targets.

Best wishes,
Navid.

On Sat, 29 Jan 2022, Jakub Jelinek wrote:

> On Fri, Jan 28, 2022 at 03:14:16PM -0700, Jeff Law via Gcc-patches wrote:
> > > This patch will add the missed pattern described in bug 103514 [1] to the match.pd. [1] includes proof of correctness for the patch too.
> > > 
> > > PR tree-optimization/103514
> > > 	* match.pd (a & b) ^ (a == b) -> !(a | b): New optimization.
> > > 	* match.pd (a & b) == (a ^ b) -> !(a | b): New optimization.
> > > 	* gcc.dg/tree-ssa/pr103514.c: Testcase for this optimization.
> > > 
> > > 1) https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103514
> > Note the bug was filed an fixed during stage3, review just didn't happen in
> > a reasonable timeframe.
> > 
> > I'm going to ACK this for the trunk and go ahead and commit it for you.
> 
> The testcase FAILs on short-circuit targets like powerpc64le-linux.
> While the first 2 functions are identical, the last two look like:
>   <bb 2> :
>   if (a_5(D) != 0)
>     goto <bb 3>; [INV]
>   else
>     goto <bb 4>; [INV]
> 
>   <bb 3> :
>   if (b_6(D) != 0)
>     goto <bb 5>; [INV]
>   else
>     goto <bb 4>; [INV]
> 
>   <bb 4> :
> 
>   <bb 5> :
>   # iftmp.1_4 = PHI <1(3), 0(4)>
>   _1 = a_5(D) == b_6(D);
>   _2 = (int) _1;
>   _3 = _2 ^ iftmp.1_4;
>   _9 = _2 != iftmp.1_4;
>   return _9;
> instead of the expected:
>   <bb 2> :
>   _3 = a_8(D) & b_9(D);
>   _4 = (int) _3;
>   _5 = a_8(D) == b_9(D);
>   _6 = (int) _5;
>   _1 = a_8(D) | b_9(D);
>   _2 = ~_1;
>   _7 = (int) _2;
>   _10 = ~_1;
>   return _10;
> so no wonder it doesn't match.  E.g. x86_64-linux will also use jumps
> if it isn't just a && b but a && b && c && d (will do
> a & b and c & d tests and jump based on those.
> 
> As it is too late to implement this optimization even for the short
> circuiting targets this late (not even sure which pass would be best),
> this patch just forces non-short-circuiting for the test.
> 
> Tested on x86_64-linux -m32/-m64 and powerpc64le-linux, ok for trunk?

OK.

> 2022-01-29  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/103514
> 	* gcc.dg/tree-ssa/pr103514.c: Add
> 	--param logical-op-non-short-circuit=1 to dg-options.
> 
> --- gcc/testsuite/gcc.dg/tree-ssa/pr103514.c.jj	2022-01-29 11:11:39.338627697 +0100
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr103514.c	2022-01-29 17:37:18.255237211 +0100
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O -fdump-tree-optimized" } */
> +/* { dg-options "-O --param logical-op-non-short-circuit=1 -fdump-tree-optimized" } */
>  #include <stdbool.h>
>  
>  bool
> @@ -30,4 +30,4 @@ h (bool a, bool b)
>  /* Make sure we have removed "==" and "^" and "&". */
>  /* { dg-final { scan-tree-dump-not "&" "optimized"} } */
>  /* { dg-final { scan-tree-dump-not "\\^"  "optimized"} } */
> -/* { dg-final { scan-tree-dump-not "==" "optimized"} } */
> \ No newline at end of file
> +/* { dg-final { scan-tree-dump-not "==" "optimized"} } */
> 
> 
> 	Jakub
> 
>

On Sun, Jan 30, 2022 at 10:16:44AM +0000, Navid Rahimi via Gcc-patches wrote:
> Thanks Jakob for the correction. Sadly, I didn’t have any access to any non x86 architecture. But x86 was fully tested and there was no regression.
> 
> In my spare time I will look at implementation of this for short-circuit targets.

Note, it isn't just about those targets.
If you write the code as:
_Bool
g (_Bool a, _Bool b)
{
  _Bool c;
  if (!a)
    c = 0;
  else if (!b)
    c = 0;
  else
    c = 1;
  return c == (a ^ b); 
}
instead, it will not match either, not even on x86, even when it is
equivalent.

Though, maybe for non-short-circuiting targets we should recognize this
somewhere and turn into c = a & b;

Since phiopt2 it is:
  <bb 2> [local count: 1073741824]:
  if (a_4(D) != 0)
    goto <bb 3>; [50.00%]
  else
    goto <bb 4>; [50.00%]

  <bb 3> [local count: 536870913]:
  _8 = (int) b_5(D);

  <bb 4> [local count: 1073741824]:
  # iftmp.0_3 = PHI <_8(3), 0(2)>
and phiopt3 makes
  <bb 2> [local count: 1073741824]:
  if (a_4(D) != 0)
    goto <bb 3>; [50.00%]
  else
    goto <bb 4>; [50.00%]

  <bb 3> [local count: 536870913]:

  <bb 4> [local count: 1073741824]:
  # _9 = PHI <b_5(D)(3), 0(2)>
  iftmp.0_3 = (int) _9;
out of that.

CCing Andrew if he'd like to have a look for GCC 13.

	Jakub

On Mon, Jan 31, 2022 at 2:13 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Sun, Jan 30, 2022 at 10:16:44AM +0000, Navid Rahimi via Gcc-patches wrote:
> > Thanks Jakob for the correction. Sadly, I didn’t have any access to any non x86 architecture. But x86 was fully tested and there was no regression.
> >
> > In my spare time I will look at implementation of this for short-circuit targets.
>
> Note, it isn't just about those targets.
> If you write the code as:
> _Bool
> g (_Bool a, _Bool b)
> {
>   _Bool c;
>   if (!a)
>     c = 0;
>   else if (!b)
>     c = 0;
>   else
>     c = 1;
>   return c == (a ^ b);
> }
> instead, it will not match either, not even on x86, even when it is
> equivalent.
>
> Though, maybe for non-short-circuiting targets we should recognize this
> somewhere and turn into c = a & b;
>
> Since phiopt2 it is:
>   <bb 2> [local count: 1073741824]:
>   if (a_4(D) != 0)
>     goto <bb 3>; [50.00%]
>   else
>     goto <bb 4>; [50.00%]
>
>   <bb 3> [local count: 536870913]:
>   _8 = (int) b_5(D);
>
>   <bb 4> [local count: 1073741824]:
>   # iftmp.0_3 = PHI <_8(3), 0(2)>
> and phiopt3 makes
>   <bb 2> [local count: 1073741824]:
>   if (a_4(D) != 0)
>     goto <bb 3>; [50.00%]
>   else
>     goto <bb 4>; [50.00%]
>
>   <bb 3> [local count: 536870913]:
>
>   <bb 4> [local count: 1073741824]:
>   # _9 = PHI <b_5(D)(3), 0(2)>
>   iftmp.0_3 = (int) _9;
> out of that.
>
> CCing Andrew if he'd like to have a look for GCC 13.

Yes I have a patch to recognize:
  if (a_3(D) != 0)
    goto <bb 4>; [50.00%]
  else
    goto <bb 3>; [50.00%]

  <bb 3> [local count: 536870912]:

  <bb 4> [local count: 1073741824]:
  # c_2 = PHI <0(3), b_4(D)(2)>

already (a ? b : 0) into a & b.

This is already recorded as PR 89263.

Thanks,
Andrew Pinski

>
>         Jakub
>