Bug 56365

I see, at -O2, on x86_64 in 070t.phiopt:

test_04 (int a, int b)
{
  int D.1744;
  int D.1741;
  int _3;
  int _4;

  <bb 2>:
  _3 = MIN_EXPR <a_1(D), 127>;
  _4 = MAX_EXPR <_3, -128>;
  return _4;

}

for the other cases you run into the issue that the tree-level phiopt
can be confused by phi-merging:

test_05 (int a)
{
  <bb 2>:
  if (a_2(D) > 126)
    goto <bb 5>;
  else
    goto <bb 3>;

  <bb 3>:
  if (a_2(D) < -127)
    goto <bb 5>;
  else
    goto <bb 4>;

  <bb 4>:

  <bb 5>:
  # a_1 = PHI <127(2), a_2(D)(4), -128(3)>
  return a_1;

(In reply to comment #1)
> I see, at -O2, on x86_64 in 070t.phiopt:
> 
> test_04 (int a, int b)
> {
>   int D.1744;
>   int D.1741;
>   int _3;
>   int _4;
> 
>   <bb 2>:
>   _3 = MIN_EXPR <a_1(D), 127>;
>   _4 = MAX_EXPR <_3, -128>;
>   return _4;
> 
> }

Ah yes, now I see that here, too.  I don't know where or how I was looking, sorry.

(In reply to Oleg Endo from comment #2)
> (In reply to comment #1)
> > I see, at -O2, on x86_64 in 070t.phiopt:
> > 
> > test_04 (int a, int b)
> > {
> >   int D.1744;
> >   int D.1741;
> >   int _3;
> >   int _4;
> > 
> >   <bb 2>:
> >   _3 = MIN_EXPR <a_1(D), 127>;
> >   _4 = MAX_EXPR <_3, -128>;
> >   return _4;
> > 
> > }
> 
> Ah yes, now I see that here, too.  I don't know where or how I was looking,
> sorry.

If compiled as C (-x c -std=gnu99) the min/max patterns work.  Compiling as C++ (-x c++ -std=c++11) does not work.  Probably that was reason for my original confusion -- usually I compile such small test snippets as C++ ...

As of r213381 this problem still exists.

compiled as C 003t.original:

;; Function min (null)
;; enabled by -tree-original


{
  return MIN_EXPR <b, a>;
}


;; Function max (null)
;; enabled by -tree-original


{
  return MAX_EXPR <a, b>;
}


;; Function test_04 (null)
;; enabled by -tree-original


{
  return max (-128, min (127, a));
}


compiled as C++ 003t.original:


;; Function int min(int, int) (null)
;; enabled by -tree-original


return <retval> = a < b ? a : b;


;; Function int max(int, int) (null)
;; enabled by -tree-original


return <retval> = a < b ? b : a;


;; Function int test_04(int, int) (null)
;; enabled by -tree-original


<<cleanup_point return <retval> = max (-128, min (127, a))>>;

Richard, any chance this might get fixed with the match-and-simplify branch?

No, that doesn't handle PHI nodes.  phiopt needs to be improved to handle
merged PHIs for this case.

Ok, so let me fix this for GCC 7.  It isn't realy "phi merging" but
phiopt being confused by our a <= -128 to a < -127 canonicalization.

(In reply to Richard Biener from comment #7)
> Ok, so let me fix this for GCC 7.

Thanks :)

In the following testcase we used to do better with GCC 4.3 at least, handling
test_02 and test_04 completely and test_01 and test_03 half-way.  Compared
to GCC 5 which only handles test_04 completely and test_03 half-way and
test_01 and test_02 not at all.

So this is a regression probably caused by adding maybe_canonicalize_comparison
to comparison folding, PR26899.

int test_01 (int a)
{
  if (127 <= a)
    a = 127;
  else if (a <= -128)
    a = -128;
  return a;
}
int test_02 (int a)
{
  if (127 < a)
    a = 127;
  else if (a <= -128)
    a = -128;
  return a;
}
int test_03 (int a)
{
  if (127 <= a)
    a = 127;
  else if (a < -128)
    a = -128;
  return a;
}
int test_04 (int a)
{
  if (127 < a)
    a = 127;
  else if (a < -128)
    a = -128;
  return a;
}

Fixed on trunk sofar.  Not exactly planning to backport.

Author: rguenth
Date: Mon Mar 14 14:50:40 2016
New Revision: 234183

URL: https://gcc.gnu.org/viewcvs?rev=234183&root=gcc&view=rev
Log:
2016-03-14  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/56365
	* tree-ssa-phiopt.c (minmax_replacement): Handle alternate
	constants to compare against.

	* gcc.dg/tree-ssa/phi-opt-14.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-14.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-phiopt.c

(In reply to Richard Biener from comment #10)
> Fixed on trunk sofar.  Not exactly planning to backport.

Hm yeah.  Although the patch applies and builds on the 5 branch without issues, test_04 from c#0 doesn't seem to work.

GCC 4.9 branch is being closed

Richi, if you're not going to backport any patches, maybe close this one as fixed?

Keeping it open as it is a regression and to mark the last GCC 5 release as known-to-fail.

GCC 5 branch has been closed, should be fixed in GCC 6 and later.