[Bug tree-optimization/101024] New: Missed min expression at phiopt1

[Bug tree-optimization/101024] New: Missed min expression at phiopt1

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101024

            Bug ID: 101024
           Summary: Missed min expression at phiopt1
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: missed-optimization
          Severity: enhancement
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pinskia at gcc dot gnu.org
  Target Milestone: ---

Take:
unsigned f(int nunits)
{
  int units_per = 32 / (8);


  unsigned int chunk_nunits = ((nunits) < (units_per) ? (nunits) :
(units_per));
  return chunk_nunits;
}
-------- CUT ------
Compile with the C++ front-end and -O2 (note C++ is needed as fold won't
convert ?: into min/max expression due to lvalue requirements).
You will notice this is not caught by phiopt1 even though it should be while it
is caught now (on the trunk) by phiopt2 (match-and-simplify).  That is the
minmax_replacement does not handle the above case but match does.

Note this shows up in read code (GCC) in assemble_real (varasm.c). There are
two cases of this in assemble_real but only one of the two is caught in phiopt2
though because there looks like some jump threading that had happened before
that causes other issues.

I am filing this as a bug only so I can keep track of moving minmax_replacement
to match-and-simplify.

[Bug tree-optimization/101024] Missed min expression at phiopt1

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101024

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-06-11
           Assignee|unassigned at gcc dot gnu.org      |pinskia at gcc dot gnu.org
     Ever confirmed|0                           |1
             Blocks|                            |25290
             Status|UNCONFIRMED                 |ASSIGNED

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Mine.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25290
[Bug 25290] PHI-OPT could be rewritten so that is uses match

[Bug tree-optimization/101024] Missed min expression at phiopt1

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101024

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I have a start but there are a few patterns that need to be moved.
An example is from gcc.dg/tree-ssa/pr66726-4.c:
#define SAT(x) (x < 0 ? 0 : (x > 255 ? 255 : x))

void
foo (unsigned char *p, int i)
{
  *p = SAT (i);
}

[Bug tree-optimization/101024] Missed min expression at phiopt1

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101024

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
gcc.dg/tree-ssa/phi-opt-20.c is another case which is missing right now, it
deals with EQ_EXPR/NE_EXPR.

[Bug tree-optimization/101024] Missed min expression at phiopt1

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101024

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #0)
> You will notice this is not caught by phiopt1 even though it should be while
> it is caught now (on the trunk) by phiopt2 (match-and-simplify).  That is
> the minmax_replacement does not handle the above case but match does.

This is now caught after r12-2185 but we have not moved minmax_replacement yet,
just got match-and-simplify working correctly on early phiopt (phiopt1).

[Bug tree-optimization/101024] Missed min expression at phiopt1

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101024

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=87913

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #3)
> gcc.dg/tree-ssa/phi-opt-20.c is another case which is missing right now, it
> deals with EQ_EXPR/NE_EXPR.
Which was added with https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87913

[Bug tree-optimization/101024] Missed min expression at phiopt1

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101024

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Part of this is in the patch set in bug 25290 comment # 27 patch set. Mostly
the "c ? min/max : min/max" part, I still need to implement the "c ? min : d"
part.

[Bug tree-optimization/101024] Missed min expression at phiopt1

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101024

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=95699

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
After https://gcc.gnu.org/pipermail/gcc-patches/2023-May/617758.html, these are
testcases that fail if we remove minmax_replacement :
FAIL: gcc.dg/tree-ssa/phi-opt-20.c scan-tree-dump-times phiopt1 "MAX_EXPR" 2
FAIL: gcc.dg/tree-ssa/phi-opt-20.c scan-tree-dump-times phiopt1 "MIN_EXPR" 2

phiopt match-simplify trying:
        num_2(D) != 0 ? num_2(D) : 1
        num_2(D) != 4294967295 ? num_2(D) : 4294967294
        num_2(D) != -2147483648 ? num_2(D) : -2147483647
        num_2(D) != 2147483647 ? num_2(D) : 2147483646

This should be reasonable to handle minmax_from_comparison and handle eq/ne for
the cases where that is called.


FAIL: gcc.dg/tree-ssa/pr95699.c scan-tree-dump optimized "MAX_EXPR
<[^>\n\r]*9223372036854775808[^>\n\r]*>"
FAIL: gcc.dg/tree-ssa/pr95699.c scan-tree-dump optimized "MIN_EXPR
<[^>\n\r]*9223372036854775808[^>\n\r]*>"

These are a bit more complex though:
  x.0_1 = (signed long) x_3(D);
        x.0_1 >= 0 ? 9223372036854775808 (0x8000000000000000) : x_3(D)


  x.1_1 = (signed long) x_3(D);
        x.1_1 < 0 ? 9223372036854775808 (0x8000000000000000) : x_3(D)

[Bug tree-optimization/101024] Missed min expression at phiopt1

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101024

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #7)
> After https://gcc.gnu.org/pipermail/gcc-patches/2023-May/617758.html, these
> are testcases that fail if we remove minmax_replacement :
> FAIL: gcc.dg/tree-ssa/phi-opt-20.c scan-tree-dump-times phiopt1 "MAX_EXPR" 2
> FAIL: gcc.dg/tree-ssa/phi-opt-20.c scan-tree-dump-times phiopt1 "MIN_EXPR" 2
> 
> phiopt match-simplify trying:
>         num_2(D) != 0 ? num_2(D) : 1
>         num_2(D) != 4294967295 ? num_2(D) : 4294967294
>         num_2(D) != -2147483648 ? num_2(D) : -2147483647
>         num_2(D) != 2147483647 ? num_2(D) : 2147483646
> 
> This should be reasonable to handle minmax_from_comparison and handle eq/ne
> for the cases where that is called.

I have a patch which also handles when we know the range of non-constant.
Like this:
```
int f(int num)
{
  if (num < 3) __builtin_unreachable();
  return num != 3 ?  num : 4;
}
```
That should get a MAX_EXPR(num, 4);

[Bug tree-optimization/101024] Missed min expression at phiopt1

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101024

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 55018
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55018&action=edit
Patch which implements the != part

This is the patch which I am testing for the != part.

[Bug tree-optimization/101024] Missed min expression at phiopt1

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101024

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The only thing left to do to remove minmax_replacement, is the improvement
mentioned in PR 95699 (or rather r11-1504-g2e0f4a18bc978c7362 ).

[Bug tree-optimization/101024] Missed min expression at phiopt1

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101024

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
A few testcases that have not been added to the testsuite yet.
Note it takes f1 to phiopt2 to optimize that because there is an extra
statement left behind because match does not deal with `(signed)a < 0` yet.

```
unsigned long long
f2 (unsigned long long x)
{
  if (x < 0x8000000000000000ULL)
    x = 0x8000000000000000ULL;
  else
    {
        if (x >= 0x8000000000000023ULL)
          x = 0x8000000000000023ULL;
    }
  return x;
}
unsigned long long
f1 (unsigned long long x)
{
  if (x >= 100)
    {
        if (x >= 0x8000000000000000ULL)
          x = 0x8000000000000000ULL;
    }
  else
    x = 100;
  return x;
}

```