[Bug tree-optimization/68894] Recognition min/max pattern with multiple arguments.

[Bug tree-optimization/68894] Recognition min/max pattern with multiple arguments.

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |pinskia at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Mine, working on improving phi-opt here; though the hoisting of the load is
something which needs to be looked into further.

I have the match.pd pattern for what is mentioned in comment #3:
/* These was part of minmax phiopt.  */
/* Optimize (a CMP b) ? minmax<a, c> : minmax<b, c>
   to minmax<min/max<a, b>, c> */
(for minmax (min max)
 (for cmp (lt le gt ge)
  (simplify
   (cond (cmp @1 @3) (minmax:c @1 @4) (minmax:c @2 @4))
   (with
    {
      tree_code code = minmax_from_comparison (cmp, @1, @2, @1, @3);
    }
    (if (code == MIN_EXPR)
     (minmax (min @1 @2) @4)
     (if (code == MAX_EXPR)
      (minmax (max @1 @2) @4)))))))

I also have the improvements to match_simplify_replacement which are needed to
use those patterns too. I will be submitting them for GCC 14.

[Bug tree-optimization/68894] Recognition min/max pattern with multiple arguments.

[pinskia at gcc dot gnu.org]

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

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #7)
> Mine, working on improving phi-opt here; though the hoisting of the load is
> something which needs to be looked into further.
> 
> I have the match.pd pattern for what is mentioned in comment #3:
> /* These was part of minmax phiopt.  */
> /* Optimize (a CMP b) ? minmax<a, c> : minmax<b, c>
>    to minmax<min/max<a, b>, c> */
> (for minmax (min max)
>  (for cmp (lt le gt ge)
>   (simplify
>    (cond (cmp @1 @3) (minmax:c @1 @4) (minmax:c @2 @4))
>    (with
>     {
>       tree_code code = minmax_from_comparison (cmp, @1, @2, @1, @3);
>     }
>     (if (code == MIN_EXPR)
>      (minmax (min @1 @2) @4)
>      (if (code == MAX_EXPR)
>       (minmax (max @1 @2) @4)))))))
> 
> I also have the improvements to match_simplify_replacement which are needed
> to use those patterns too. I will be submitting them for GCC 14.

Note these patterns are enough for the ifcvt pass for the vectorizer to
generate:

  pretmp_24 = a3D.2740[i_20];
  _35 = MIN_EXPR <_1, _2>;
  d_9 = MIN_EXPR <pretmp_24, _35>;

Rather than the current:

  d_14 = MIN_EXPR <_1, pretmp_24>;
  _23 = _1 < _2;
  d_13 = MIN_EXPR <_2, pretmp_24>;
  d_9 = _23 ? d_14 : d_13;

[Bug tree-optimization/68894] Recognition min/max pattern with multiple arguments.

[pinskia at gcc dot gnu.org]

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

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I have a patch which fixes the phiopt issue I saw.
The problem is when do_hoist_loads is true (which is !early and
-fhoist-adjacent-loads ), we would not do the diamond case for phiopt in later
passes.
In the above case, pre needs to hoist the load first and then we could get to
it with phiopt4.

You can see the same effect with:
-O2 -fno-hoist-adjacent-loads -fno-tree-vectorize

[Bug tree-optimization/68894] Recognition min/max pattern with multiple arguments.

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:

https://gcc.gnu.org/g:7049241f6ee558cfc0b227b5a0a355ec29afd6f1

commit r14-201-g7049241f6ee558cfc0b227b5a0a355ec29afd6f1
Author: Andrew Pinski <apinski@marvell.com>
Date:   Thu Apr 20 10:56:17 2023 -0700

    PHIOPT: Allow other diamond uses when do_hoist_loads is true

    While working on adding diamond shaped form to match-and-simplify
    phiopt, I Noticed that we would not reach there if do_hoist_loads
    was true. In the original code before the cleanups it was not
    obvious why but after I finished the cleanups, it was just a matter
    of removing a continue and that is what this patch does.

    This just happens also to fix a bug report that I noticed too.

    OK? Bootstrapped and tested on x86_64-linux-gnu.

    gcc/ChangeLog:

            PR tree-optimization/68894
            * tree-ssa-phiopt.cc (tree_ssa_phiopt_worker): Remove the
            continue for the do_hoist_loads diamond case.

[Bug tree-optimization/68894] Recognition min/max pattern with multiple arguments.

[pinskia at gcc dot gnu.org]

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

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The phiopt issue is fixed for GCC 14 now.
Ifcvt will be fixed soon.

[Bug tree-optimization/68894] Recognition min/max pattern with multiple arguments.

[pinskia at gcc dot gnu.org]

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

--- Comment #12 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Just a quick note on this part of the bug report:

(In reply to Richard Biener from comment #3)
> Doing this in a classical way in phi-opt might end up being slightly
> convoluted.
> So I'd propose to try to utilize match-and-simplify by adding
> 
> 
> (cond (ge @0 @1) (max:s @0 @2) (max:s @1 @2))
> -> (max (max @0 @1) @2)

I am going to submit patches for the above later today or tomorrow.

> 
> kind patterns and from phiopt doing exploded queries of this simplification
> by seeding the cond expr from the dominating condition plus the PHI operands.
> 
>  res = gimple_simplify (COND_EXPR, build2 /* Ick */ (cond-code,
> boolean_type, cond-op0, cond-op1), true-phi-arg, false-phi-arg, &seq,
> follow_single_use_edges);
> 
> of course you need more than a single pattern or consider swapped ops 2 and 3
> (not yet supported).  In the end explicit pattern explosion might make a
> manual
> implementation in phiopt easier (who knows).

This part is fully implemented in r14-204-gf1f5cbaa3f716fcb472dee5 (there was a
few pieces of the match-and-simplify phiopt implemented in GCC 12). 

Note phiopt already does "a ? b : c" -> "!a ? c : b" (where !a folds)
(implemented in r12-2040-ga50cecb20a10) specifically because of the explosion
issue.

[Bug tree-optimization/68894] Recognition min/max pattern with multiple arguments.

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #13 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed fully at r14-337-gc43819a9b4cdaa7359e55 .