On Sun, Jun 7, 2026 at 8:15 AM Andrew Pinski
<[email protected]> wrote:
>
> This takes https://gcc.gnu.org/pipermail/gcc-patches/2026-June/719384.html
> and merge it into factor_out_conditional_operation. Also expands it to
> allow for more than just unary and binary operands.
> It handles as similar as what ifcvt does in factor_out_operators
> but rejects some cases due to those not being profitable.
> The cases which are not profitable:
> * pointer plus in early with constant operand 1 (except if equal).
> * division/mod with constant operand 1
> * Complex expr, it would cause to lose an unitialization warning
> (gcc.dg/uninit-17.c)
>
> some cases needed to be rejected for validity (copied from ifcvt):
> * BIT_FIELD_REF/BIT_INSERT_EXPR (non first operand)
> * VEC_PERM_EXPR with constant operand 2
>
> Notes on the testcase changes:
> The recip-*.c testcases need to be disable phiopt since it removes
> a division in some cases which causes the recip pass not to run.
>
> slsr-12.c and slsr-34.c need to be xfailed. SLSR pass is mostly
> in maintaince mode and is not getting improved.
>
> pr122629-1.c and vect-reduc-cond-2.c are now handled in phiopt
> rather than ifcvt.
>
> cinc_common_1.c is xfailed because of missing pattern in the aarch64
> backend, PR112304.
>
> fuse_cmp_csel.c needed to be updated since the add is now after the
> cmp/csel pair and ira puts the constants formation inbetween the cmp/csel.
> fuse_cmp_csel-1.c is new version where there is no constant formation.

Looking at the patch shouldn't we decide profitability independent of the
actual operation when there's one constant operand?

> changes since v1:
> * v2: Fix some comments. Add CEIL_MOD_EXPR and ROUND_MOD_EXPR to
> is_divide_or_mod_p. Remove operand_equal from POINTER_PLUS case.
> xfail cinc_common_1.c. Fixed up fuse_cmp_csel.c testcase.
> * v3: Fix up cost model, was only calling is_factor_profitable on
> the different operands when it needs to be on all operands.
>
>         PR tree-optimization/125557
>         PR tree-optimization/64700
>         PR tree-optimization/29144
>         PR tree-optimization/94274
> gcc/ChangeLog:
>
>         * tree-if-conv.cc: Include tree-ssa-phiopt.h
>         (find_different_opnum): Move to ...
>         * tree-ssa-phiopt.cc (find_different_opnum): Here.

Maybe this better fits gimple-match.h/-exports.cc?

>         (is_divide_or_mod_p): New function.

tree.h?  There we alrady have trunc_or_exact_div_p.  is_.*_p is duplicate,
either is_ or _p please.

Otherwise looks OK to me.

>         (is_factor_profitable): Take gimple_match_op instead of one operand.
>         Rearrange the code to check the lifetime of the operands last.
>         (factor_out_conditional_operation): Handle operands > 1,
>         including operands communitive operands. Add early_p argument
>         for costing. Update call of is_factor_profitable.
>         (pass_phiopt::execute): Pass early_p to
>         factor_out_conditional_operation.
>         * tree-ssa-phiopt.h: New file.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/tree-ssa/recip-3.c: Disable phiopt since it removes
>         one division and recip pass needs 3.
>         * gcc.dg/tree-ssa/recip-5.c: Likewise.
>         * gcc.dg/tree-ssa/recip-6.c: Likewise.
>         * gcc.dg/tree-ssa/recip-7.c: Likewise.
>         * gcc.dg/tree-ssa/slsr-12.c: xfail.
>         * gcc.dg/tree-ssa/slsr-34.c: Likewise.
>         * gcc.dg/tree-ssa/pr122629-1.c: Update to scan phiopt1.
>         * gcc.dg/vect/vect-reduc-cond-2.c: Likewise.
>         * gcc.dg/tree-ssa/phi-factor-binary-1.c: New test.
>         * gcc.dg/tree-ssa/phi-factor-binary-2.c: New test.
>         * gcc.target/aarch64/phi-factor-binary-1.c: New test.
>         * gcc.target/aarch64/cinc_common_1.c: xfail.
>         * gcc.target/aarch64/fuse_cmp_csel.c: xfail.
>         * gcc.target/aarch64/fuse_cmp_csel-1.c: New test.
>
> Signed-off-by: Andrew Pinski <[email protected]>
> Co-authored-by: Kyrylo Tkachov <[email protected]>
> ---
>  .../gcc.dg/tree-ssa/phi-factor-binary-1.c     |  20 ++
>  .../gcc.dg/tree-ssa/phi-factor-binary-2.c     |  41 +++
>  gcc/testsuite/gcc.dg/tree-ssa/pr122629-1.c    |   4 +-
>  gcc/testsuite/gcc.dg/tree-ssa/recip-3.c       |   3 +
>  gcc/testsuite/gcc.dg/tree-ssa/recip-5.c       |   2 +-
>  gcc/testsuite/gcc.dg/tree-ssa/recip-6.c       |   2 +-
>  gcc/testsuite/gcc.dg/tree-ssa/recip-7.c       |   2 +-
>  gcc/testsuite/gcc.dg/tree-ssa/slsr-12.c       |   2 +-
>  gcc/testsuite/gcc.dg/tree-ssa/slsr-34.c       |   2 +-
>  gcc/testsuite/gcc.dg/vect/vect-reduc-cond-2.c |   4 +-
>  .../gcc.target/aarch64/cinc_common_1.c        |   5 +-
>  .../gcc.target/aarch64/fuse_cmp_csel-1.c      |  35 +++
>  .../gcc.target/aarch64/fuse_cmp_csel.c        |  14 +-
>  .../gcc.target/aarch64/phi-factor-binary-1.c  |  60 ++++
>  gcc/tree-if-conv.cc                           |  80 +-----
>  gcc/tree-ssa-phiopt.cc                        | 257 +++++++++++++++---
>  gcc/tree-ssa-phiopt.h                         |  24 ++
>  17 files changed, 420 insertions(+), 137 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-factor-binary-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-factor-binary-2.c
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/fuse_cmp_csel-1.c
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/phi-factor-binary-1.c
>  create mode 100644 gcc/tree-ssa-phiopt.h
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-factor-binary-1.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/phi-factor-binary-1.c
> new file mode 100644
> index 00000000000..bfcae888125
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-factor-binary-1.c
> @@ -0,0 +1,20 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-phiopt1-details" } */
> +
> +/* PR tree-optimization/125557.  Both arms of the diamond apply the same 
> binary
> +   operation sharing one operand (* c), so phi-opt factors it out: the join
> +   becomes t = PHI <a, b>; x = t * c -- one multiply at the merge instead of
> +   one in each arm, and no extra PHI.  */
> +
> +int
> +f (int cond, int a, int b, int c)
> +{
> +  int x;
> +  if (cond)
> +    x = a * c;
> +  else
> +    x = b * c;
> +  return x;
> +}
> +
> +/* { dg-final { scan-tree-dump " changed to factor operation out from 
> COND_EXP" "phiopt1" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-factor-binary-2.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/phi-factor-binary-2.c
> new file mode 100644
> index 00000000000..eb7669b664b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-factor-binary-2.c
> @@ -0,0 +1,41 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-phiopt1-details" } */
> +
> +/* tree-optimization/125557.  factor_out_binary_common_operand handles more 
> than
> +   one operation kind and, when several join PHIs share the differing 
> operand,
> +   factors each of them -- collapsing the arms to a single select.  */
> +
> +/* Two PHIs share the differing operand a/b: the multiply and the add are 
> each
> +   factored out, so both arms reduce to one select of a/b.  */
> +void
> +f2 (int cond, int a, int b, int c, int d, int *p, int *q)
> +{
> +  int x, y;
> +  if (cond)
> +    {
> +      x = a * c;
> +      y = a + d;
> +    }
> +  else
> +    {
> +      x = b * c;
> +      y = b + d;
> +    }
> +  *p = x;
> +  *q = y;
> +}
> +
> +/* A shift is factored just like the arithmetic ops.  */
> +unsigned long
> +f3 (int cond, unsigned long a, unsigned long b, int s)
> +{
> +  unsigned long x;
> +  if (cond)
> +    x = a << s;
> +  else
> +    x = b << s;
> +  return x;
> +}
> +
> +/* f2 factors twice (the * and the +), f3 once (the <<).  */
> +/* { dg-final { scan-tree-dump-times " changed to factor operation out from 
> COND_EXP" 3 "phiopt1" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr122629-1.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/pr122629-1.c
> index a80d4a1990b..54f7bf11d48 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/pr122629-1.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr122629-1.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fdump-tree-ifcvt-details" } */
> +/* { dg-options "-O2 -fdump-tree-ifcvt-details -fdump-tree-phiopt-details" } 
> */
>  /* PR tree-optimization/122629 */
>
>  typedef int ix4 __attribute__((vector_size(4*sizeof(int))));
> @@ -33,4 +33,4 @@ int g(ix4 *a, int l, int *b, ix4 *c)
>  }
>
>  /* Make sure BIT_INSERT_EXPR/BIT_FIELD_REF is still factored out for the 
> case if operand 0 is different. */
> -/* { dg-final { scan-tree-dump-times "changed to factor operation out from" 
> 2 "ifcvt" } } */
> +/* { dg-final { scan-tree-dump-times "changed to factor operation out from" 
> 2 "phiopt1" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c
> index 036f32a9c33..3c6df8d0726 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/recip-3.c
> @@ -5,6 +5,9 @@
>     to optimize a sequence.  With a FP enabled ranger, we eliminate one of 
> them
>     earlier, causing the pass to skip this optimization.  */
>  /* { dg-additional-options "-fno-thread-jumps -fno-tree-dominator-opts" } */
> +/* PHI-OPT will factor out the `/d` from the `?:` expression which means 
> there will
> +   only be 2 `/d` left so disable that. */
> +/* { dg-additional-options "-fno-ssa-phiopt" } */
>
>  double F[5] = { 0.0, 0.0 }, e;
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/recip-5.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/recip-5.c
> index 6ac0559bc7c..f05cfe866cc 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/recip-5.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/recip-5.c
> @@ -1,4 +1,4 @@
> -/* { dg-options "-O1 -funsafe-math-optimizations -ftrapping-math 
> -fdump-tree-recip -fdump-tree-optimized" } */
> +/* { dg-options "-O1 -funsafe-math-optimizations -ftrapping-math 
> -fdump-tree-recip -fdump-tree-optimized -fno-ssa-phiopt" } */
>  /* { dg-do compile } */
>  /* { dg-warning "'-fassociative-math' disabled" "" { target *-*-* } 0 } */
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/recip-6.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/recip-6.c
> index b5d537a6ccd..c172fb34fcf 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/recip-6.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/recip-6.c
> @@ -1,4 +1,4 @@
> -/* { dg-options "-O1 -funsafe-math-optimizations -fno-trapping-math 
> -fdump-tree-recip" } */
> +/* { dg-options "-O1 -funsafe-math-optimizations -fno-trapping-math 
> -fdump-tree-recip -fno-ssa-phiopt" } */
>  /* { dg-do compile } */
>
>  /* Test inserting in a block that does not contain a division.  */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/recip-7.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/recip-7.c
> index 13fca0b0e09..0e205d17891 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/recip-7.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/recip-7.c
> @@ -1,4 +1,4 @@
> -/* { dg-options "-O1 -funsafe-math-optimizations -fno-trapping-math 
> -fdump-tree-recip" } */
> +/* { dg-options "-O1 -funsafe-math-optimizations -fno-trapping-math 
> -fdump-tree-recip -fno-ssa-phiopt" } */
>  /* { dg-do compile } */
>
>  /* Test inserting in a block that does not contain a division.  */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/slsr-12.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/slsr-12.c
> index f92b70ffafb..f50c2020157 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/slsr-12.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/slsr-12.c
> @@ -26,4 +26,4 @@ f (int s, int c)
>    return x;
>  }
>
> -/* { dg-final { scan-tree-dump-times " \\* " 3 "optimized" } } */
> +/* { dg-final { scan-tree-dump-times " \\* " 3 "optimized" { xfail *-*-* } } 
> } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/slsr-34.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/slsr-34.c
> index d7ede2efe8c..31d59b84d49 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/slsr-34.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/slsr-34.c
> @@ -38,5 +38,5 @@ f (int c, int i)
>    return x;
>  }
>
> -/* { dg-final { scan-tree-dump-times " \\* " 1 "optimized" } } */
> +/* { dg-final { scan-tree-dump-times " \\* " 1 "optimized" { xfail *-*-* } } 
> } */
>  /* { dg-final { scan-tree-dump-times "PHI" 2 "optimized" } } */
> diff --git a/gcc/testsuite/gcc.dg/vect/vect-reduc-cond-2.c 
> b/gcc/testsuite/gcc.dg/vect/vect-reduc-cond-2.c
> index 126a50f3e56..aa204d1ab67 100644
> --- a/gcc/testsuite/gcc.dg/vect/vect-reduc-cond-2.c
> +++ b/gcc/testsuite/gcc.dg/vect/vect-reduc-cond-2.c
> @@ -1,6 +1,6 @@
>  /* { dg-require-effective-target vect_int } */
>  /* { dg-require-effective-target vect_condition } */
> -/* { dg-additional-options "-fdump-tree-ifcvt-details" } */
> +/* { dg-additional-options "-fdump-tree-phiopt1-details" } */
>
>  #include <stdarg.h>
>  #include "tree-vect.h"
> @@ -59,4 +59,4 @@ int main (void)
>  }
>
>  /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { xfail { 
> vect_no_int_add } } } } */
> -/* { dg-final { scan-tree-dump-times "changed to factor operation out from 
> COND_EXPR" 2 "ifcvt" } } */
> +/* { dg-final { scan-tree-dump-times "changed to factor operation out from 
> COND_EXPR" 2 "phiopt1" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/cinc_common_1.c 
> b/gcc/testsuite/gcc.target/aarch64/cinc_common_1.c
> index f93364f74ba..030adf3e897 100644
> --- a/gcc/testsuite/gcc.target/aarch64/cinc_common_1.c
> +++ b/gcc/testsuite/gcc.target/aarch64/cinc_common_1.c
> @@ -27,8 +27,9 @@ bardi (long long x)
>    return x > 100 ? x + 4 : x + 3;
>  }
>
> -/* { dg-final { scan-assembler-times "cs?inc\tw\[0-9\]*" 2 } } */
> -/* { dg-final { scan-assembler-times "cs?inc\tx\[0-9\]*" 2 } } */
> +/* xfailed due to PR112304. */
> +/* { dg-final { scan-assembler-times "cs?inc\tw\[0-9\]*" 2 { xfail *-*-* } } 
> } */
> +/* { dg-final { scan-assembler-times "cs?inc\tx\[0-9\]*" 2 { xfail *-*-* } } 
> } */
>
>  int
>  main (void)
> diff --git a/gcc/testsuite/gcc.target/aarch64/fuse_cmp_csel-1.c 
> b/gcc/testsuite/gcc.target/aarch64/fuse_cmp_csel-1.c
> new file mode 100644
> index 00000000000..9fcca5043ec
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/fuse_cmp_csel-1.c
> @@ -0,0 +1,35 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mcpu=neoverse-v2" } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +/*
> +** f1:
> +**     ...
> +**     cmp     w[0-9]+, w[0-9]+
> +**     csel    w[0-9]+, w[0-9]+, w[0-9]+, gt
> +**     add     w[0-9]+, w[0-9]+, w[0-9]+
> +**     ret
> +*/
> +int f1 (int a, int b, int c, int d, int e)
> +{
> +  int cmp = a > b;
> +  int add1 = c + d;
> +  int add2 = c + e;
> +  return cmp ? add1 : add2;
> +}
> +
> +/*
> +** f2:
> +**     ...
> +**     cmp     x[0-9]+, x[0-9]+
> +**     csel    x[0-9]+, x[0-9]+, x[0-9]+, gt
> +**     add     x[0-9]+, x[0-9]+, x[0-9]+
> +**     ret
> +*/
> +long long f2 (long long a, long long b, long long c, long long d, long long 
> e)
> +{
> + long long cmp = a > b;
> +  long long add1 = c + d;
> +  long long add2 = c + e;
> +  return cmp ? add1 : add2;
> +}
> diff --git a/gcc/testsuite/gcc.target/aarch64/fuse_cmp_csel.c 
> b/gcc/testsuite/gcc.target/aarch64/fuse_cmp_csel.c
> index 85f302bab98..be0fb8f935d 100644
> --- a/gcc/testsuite/gcc.target/aarch64/fuse_cmp_csel.c
> +++ b/gcc/testsuite/gcc.target/aarch64/fuse_cmp_csel.c
> @@ -2,11 +2,16 @@
>  /* { dg-options "-O2 -mcpu=neoverse-v2" } */
>  /* { dg-final { check-function-bodies "**" "" } } */
>
> +/* IRA moves constant moves between cmp and csel
> +   and never recovers since they use the same register
> +   #s. */
> +
>  /*
> -** f1:
> +** f1: { xfail *-*-* }
>  **     ...
>  **     cmp     w[0-9]+, w[0-9]+
> -**     csel    w[0-9]+, w[0-9]+, w[0-9]+, le
> +**     csel    w[0-9]+, w[0-9]+, w[0-9]+, gt
> +**     add     w[0-9]+, w[0-9]+, w[0-9]+
>  **     ret
>  */
>  int f1 (int a, int b, int c)
> @@ -18,10 +23,11 @@ int f1 (int a, int b, int c)
>  }
>
>  /*
> -** f2:
> +** f2: { xfail *-*-* }
>  **     ...
>  **     cmp     x[0-9]+, x[0-9]+
> -**     csel    x[0-9]+, x[0-9]+, x[0-9]+, le
> +**     csel    x[0-9]+, x[0-9]+, x[0-9]+, gt
> +**     add     x[0-9]+, x[0-9]+, x[0-9]+
>  **     ret
>  */
>  long long f2 (long long a, long long b, long long c)
> diff --git a/gcc/testsuite/gcc.target/aarch64/phi-factor-binary-1.c 
> b/gcc/testsuite/gcc.target/aarch64/phi-factor-binary-1.c
> new file mode 100644
> index 00000000000..fd6b4f556c0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/phi-factor-binary-1.c
> @@ -0,0 +1,60 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +/* tree-optimization/125557: factor_out_binary_common_operand computes a 
> binary
> +   operation shared between a PHI's two arms once, behind a single select,
> +   instead of once per arm -- and when several join PHIs share the differing
> +   operand it collapses them to one select, which also removes the
> +   data-dependent branch.  These are the asm wins of patch 1 on its own.  */
> +
> +/* f1: one multiply behind a select, not one per arm.  */
> +int
> +f1 (int cond, int a, int b, int c)
> +{
> +  int x;
> +  if (cond)
> +    x = a * c;
> +  else
> +    x = b * c;
> +  return x;
> +}
> +
> +/* f3: one shift, not one per arm (snappy's address-arithmetic shape).  */
> +unsigned long
> +f3 (int cond, unsigned long a, unsigned long b, int s)
> +{
> +  unsigned long x;
> +  if (cond)
> +    x = a << s;
> +  else
> +    x = b << s;
> +  return x;
> +}
> +
> +/* f2: two results share the differing operand (a/b): one select feeds both, 
> so
> +   only one multiply and one add remain and the diamond is branchless.  */
> +void
> +f2 (int cond, int a, int b, int c, int d, int *p, int *q)
> +{
> +  int x, y;
> +  if (cond)
> +    {
> +      x = a * c;
> +      y = a + d;
> +    }
> +  else
> +    {
> +      x = b * c;
> +      y = b + d;
> +    }
> +  *p = x;
> +  *q = y;
> +}
> +
> +/* f1 and f2 each keep a single multiply (two without the factoring).  */
> +/* { dg-final { scan-assembler-times {\tmul\t} 2 } } */
> +/* f3 keeps a single shift (two without the factoring).  */
> +/* { dg-final { scan-assembler-times {\tlsl\t} 1 } } */
> +/* Each diamond becomes a branchless select; no data-dependent branch 
> remains.  */
> +/* { dg-final { scan-assembler-not {\tcbn?z\t} } } */
> +/* { dg-final { scan-assembler-not {\tb\.[a-z]} } } */
> diff --git a/gcc/tree-if-conv.cc b/gcc/tree-if-conv.cc
> index b685e30bd0b..b8ae52100fb 100644
> --- a/gcc/tree-if-conv.cc
> +++ b/gcc/tree-if-conv.cc
> @@ -124,6 +124,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "tree-vectorizer.h"
>  #include "tree-eh.h"
>  #include "cgraph.h"
> +#include "tree-ssa-phiopt.h"
>
>  /* For lang_hooks.types.type_for_mode.  */
>  #include "langhooks.h"
> @@ -2173,85 +2174,6 @@ gen_phi_arg_condition (gphi *phi, ifcvt_arg_entry_t 
> &arg,
>    return cond;
>  }
>
> -/* Find the operand which is different between ARG0_OP and ARG1_OP.
> -   Returns the operand num where the difference is.
> -   Set NEWARG0 and NEWARG1 from the different argument.
> -   Returns -1 if none is found.
> -   If ARG0_OP/ARG1_OP is commutative also try swapping the
> -   two commutative operands and return the operand number where
> -   the difference happens in ARG0_OP. */
> -
> -static int
> -find_different_opnum (const gimple_match_op &arg0_op,
> -                     const gimple_match_op &arg1_op,
> -                     tree *new_arg0, tree *new_arg1)
> -{
> -  unsigned opnum = -1;
> -  unsigned first;
> -  first = first_commutative_argument (arg1_op.code, arg1_op.type);
> -  for (unsigned i = 0; i < arg0_op.num_ops; i++)
> -    {
> -      if (!operand_equal_for_phi_arg_p (arg0_op.ops[i],
> -                                       arg1_op.ops[i]))
> -       {
> -         /* Can handle only one non equal operand. */
> -         if (opnum != -1u)
> -           {
> -             /* Though if opnum is right before i and opnum is equal
> -                to the first communtative argument, handle communtative
> -                specially. */
> -             if (i == opnum + 1 && opnum == first)
> -               goto commutative;
> -             return -1;
> -           }
> -         opnum = i;
> -       }
> -  }
> -  /* If all operands are equal only do this is there was single
> -     operand.  */
> -  if (opnum == -1u)
> -    {
> -      if (arg0_op.num_ops != 1)
> -       return -1;
> -      opnum = 0;
> -    }
> -  *new_arg0 = arg0_op.ops[opnum];
> -  *new_arg1 = arg1_op.ops[opnum];
> -  return opnum;
> -
> -/* Handle commutative operations. */
> -commutative:
> -  gcc_assert (first != -1u);
> -
> -  /* Check the rest of the arguments to make sure they are the same. */
> -  for (unsigned i = first + 2; i < arg0_op.num_ops; i++)
> -    if (!operand_equal_for_phi_arg_p (arg0_op.ops[i],
> -                                     arg1_op.ops[i]))
> -      return -1;
> -
> -  /* If the arg0[first+1] and arg1[first] are the same
> -     then the one which is different is arg0[first] and arg1[first+1]
> -     return first since this is based on arg0.  */
> -  if (operand_equal_for_phi_arg_p (arg0_op.ops[first + 1],
> -                                  arg1_op.ops[first]))
> -    {
> -       *new_arg0 = arg0_op.ops[first];
> -       *new_arg1 = arg1_op.ops[first + 1];
> -       return first;
> -    }
> -  /* If the arg0[first] and arg1[first+1] are the same
> -     then the one which is different is arg0[first+1] and arg1[first]
> -     return first+1 since this is based on arg0.  */
> -  if (operand_equal_for_phi_arg_p (arg0_op.ops[first],
> -                                  arg1_op.ops[first + 1]))
> -    {
> -       *new_arg0 = arg0_op.ops[first + 1];
> -       *new_arg1 = arg1_op.ops[first];
> -       return first + 1;
> -    }
> -  return -1;
> -}
> -
>  /* Factors out an operation from *ARG0 and *ARG1 and
>     create the new statement at GSI. *RES is the
>     result of that new statement. Update *ARG0 and *ARG1
> diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> index eec65a7b68d..a2e49efcbc6 100644
> --- a/gcc/tree-ssa-phiopt.cc
> +++ b/gcc/tree-ssa-phiopt.cc
> @@ -221,26 +221,17 @@ replace_phi_edge_with_variable (basic_block cond_block,
>               bb->index);
>  }
>
> -/* Returns true if the ARG used from DEF_STMT is profitable to move
> -   to a PHI node of the basic block MERGE where the new statement
> +/* Returns true if the operands of arg_op defined from DEF_STMT is 
> profitable to move
> +   to the usage into the basic block MERGE where the new statement
>     will be located.  */
>  static bool
> -is_factor_profitable (gimple *def_stmt, basic_block merge, tree arg)
> +is_factor_profitable (gimple *def_stmt, basic_block merge, const 
> gimple_match_op &arg_op)
>  {
>    /* The defining statement should be conditional.  */
>    if (dominated_by_p (CDI_DOMINATORS, merge,
>                       gimple_bb (def_stmt)))
>      return false;
>
> -  /* If the arg is invariant, then there is
> -     no extending of the live range. */
> -  if (is_gimple_min_invariant (arg))
> -    return true;
> -
> -  /* Otherwise, the arg needs to be a ssa name. */
> -  if (TREE_CODE (arg) != SSA_NAME)
> -    return false;
> -
>    /* We should not increase the live range of arg
>       across too many statements or calls.  */
>    gimple_stmt_iterator gsi = gsi_for_stmt (def_stmt);
> @@ -257,23 +248,11 @@ is_factor_profitable (gimple *def_stmt, basic_block 
> merge, tree arg)
>    if (gsi_end_p (gsi))
>      return true;
>
> -  /* Check if the uses of arg is dominated by merge block, this is a quick 
> and
> -     rough estimate if arg is still alive at the merge bb.  */
> -  /* FIXME: extend to a more complete live range detection.  */
> -  use_operand_p use_p;
> -  imm_use_iterator iter;
> -  FOR_EACH_IMM_USE_FAST (use_p, iter, arg)
> -    {
> -      gimple *use_stmt = USE_STMT (use_p);
> -      basic_block use_bb = gimple_bb (use_stmt);
> -      if (dominated_by_p (CDI_DOMINATORS, merge, use_bb))
> -       return true;
> -    }
> -
>    /* If there are a few (non-call/asm) statements between
>       the old defining statement and end of the bb, then
> -     the live range of new arg does not increase enough.  */
> +     the live range of operands will increase enough.  */
>    int max_statements = param_phiopt_factor_max_stmts_live;
> +  bool stmts_extending_ok = true;
>
>    while (!gsi_end_p (gsi))
>      {
> @@ -288,15 +267,156 @@ is_factor_profitable (gimple *def_stmt, basic_block 
> merge, tree arg)
>         }
>        /* Non-assigns will extend the live range too much.  */
>        if (gcode != GIMPLE_ASSIGN)
> -       return false;
> +       {
> +         stmts_extending_ok = false;
> +         break;
> +       }
>        max_statements --;
>        if (max_statements == 0)
> -       return false;
> +       {
> +         stmts_extending_ok = false;
> +         break;
> +       }
>        gsi_next_nondebug (&gsi);
> +  }
> +  if (stmts_extending_ok)
> +    return true;
> +
> +  /* Loop over all of the operands to see if all are used after anyways.  */
> +  for (unsigned i = 0; i < arg_op.num_ops; i++)
> +    {
> +      tree arg = arg_op.ops[i];
> +      /* If the arg is invariant, then there is
> +        no extending of the live range. */
> +      if (is_gimple_min_invariant (arg))
> +       continue;
> +
> +      /* Otherwise, the arg needs to be a ssa name. */
> +      if (TREE_CODE (arg) != SSA_NAME)
> +       return false;
> +
> +      /* Check if the uses of arg is dominated by merge block, this is a 
> quick and
> +        rough estimate if arg is still alive at the merge bb.  */
> +      /* FIXME: extend to a more complete live range detection.  */
> +      use_operand_p use_p;
> +      imm_use_iterator iter;
> +      bool usedafter = false;
> +      FOR_EACH_IMM_USE_FAST (use_p, iter, arg)
> +       {
> +         gimple *use_stmt = USE_STMT (use_p);
> +         basic_block use_bb = gimple_bb (use_stmt);
> +         if (dominated_by_p (CDI_DOMINATORS, merge, use_bb))
> +           {
> +             usedafter = true;
> +             break;
> +           }
> +       }
> +      if (!usedafter)
> +       return false;
>      }
>    return true;
>  }
>
> +/* Return true if CODE is an integer division or integer mod code. */
> +static bool
> +is_divide_or_mod_p (const code_helper &code)
> +{
> +  switch (code.get_rep())
> +    {
> +    case TRUNC_DIV_EXPR:
> +    case CEIL_DIV_EXPR:
> +    case FLOOR_DIV_EXPR:
> +    case ROUND_DIV_EXPR:
> +    case EXACT_DIV_EXPR:
> +    case TRUNC_MOD_EXPR:
> +    case FLOOR_MOD_EXPR:
> +    case CEIL_MOD_EXPR:
> +    case ROUND_MOD_EXPR:
> +      return true;
> +    default:
> +      return false;
> +    }
> +}
> +
> +/* Find the operand which is different between ARG0_OP and ARG1_OP.
> +   Returns the operand num where the difference is.
> +   Set NEWARG0 and NEWARG1 from the different argument.
> +   Returns -1 if none is found.
> +   If ARG0_OP/ARG1_OP is commutative also try swapping the
> +   two commutative operands and return the operand number where
> +   the difference happens in ARG0_OP. */
> +
> +int
> +find_different_opnum (const gimple_match_op &arg0_op,
> +                     const gimple_match_op &arg1_op,
> +                     tree *new_arg0, tree *new_arg1)
> +{
> +  unsigned opnum = -1;
> +  unsigned first;
> +  first = first_commutative_argument (arg1_op.code, arg1_op.type);
> +  for (unsigned i = 0; i < arg0_op.num_ops; i++)
> +    {
> +      if (!operand_equal_for_phi_arg_p (arg0_op.ops[i],
> +                                       arg1_op.ops[i]))
> +       {
> +         /* Can handle only one non equal operand. */
> +         if (opnum != -1u)
> +           {
> +             /* Though if opnum is right before i and opnum is equal
> +                to the first communtative argument, handle communtative
> +                specially. */
> +             if (i == opnum + 1 && opnum == first)
> +               goto commutative;
> +             return -1;
> +           }
> +         opnum = i;
> +       }
> +  }
> +  /* If all operands are equal only do this is there was single
> +     operand.  */
> +  if (opnum == -1u)
> +    {
> +      if (arg0_op.num_ops != 1)
> +       return -1;
> +      opnum = 0;
> +    }
> +  *new_arg0 = arg0_op.ops[opnum];
> +  *new_arg1 = arg1_op.ops[opnum];
> +  return opnum;
> +
> +/* Handle commutative operations. */
> +commutative:
> +  gcc_assert (first != -1u);
> +
> +  /* Check the rest of the arguments to make sure they are the same. */
> +  for (unsigned i = first + 2; i < arg0_op.num_ops; i++)
> +    if (!operand_equal_for_phi_arg_p (arg0_op.ops[i],
> +                                     arg1_op.ops[i]))
> +      return -1;
> +
> +  /* If the arg0[first+1] and arg1[first] are the same
> +     then the one which is different is arg0[first] and arg1[first+1]
> +     return first since this is based on arg0.  */
> +  if (operand_equal_for_phi_arg_p (arg0_op.ops[first + 1],
> +                                  arg1_op.ops[first]))
> +    {
> +       *new_arg0 = arg0_op.ops[first];
> +       *new_arg1 = arg1_op.ops[first + 1];
> +       return first;
> +    }
> +  /* If the arg0[first] and arg1[first+1] are the same
> +     then the one which is different is arg0[first+1] and arg1[first]
> +     return first+1 since this is based on arg0.  */
> +  if (operand_equal_for_phi_arg_p (arg0_op.ops[first],
> +                                  arg1_op.ops[first + 1]))
> +    {
> +       *new_arg0 = arg0_op.ops[first + 1];
> +       *new_arg1 = arg1_op.ops[first];
> +       return first + 1;
> +    }
> +  return -1;
> +}
> +
>  /* PR66726: Factor operations out of COND_EXPR.  If the arguments of the PHI
>     stmt are Unary operator, factor out the operation and perform the 
> operation
>     to the result of PHI stmt.  COND_STMT is the controlling predicate.
> @@ -304,7 +424,8 @@ is_factor_profitable (gimple *def_stmt, basic_block 
> merge, tree arg)
>
>  static bool
>  factor_out_conditional_operation (edge e0, edge e1, basic_block merge,
> -                                 gphi *phi, gimple *cond_stmt)
> +                                 gphi *phi, gimple *cond_stmt,
> +                                 bool early_p)
>  {
>    gimple *arg0_def_stmt = NULL, *arg1_def_stmt = NULL;
>    tree temp, result;
> @@ -358,19 +479,14 @@ factor_out_conditional_operation (edge e0, edge e1, 
> basic_block merge,
>    if (arg0_op.operands_occurs_in_abnormal_phi ())
>     return false;
>
> -  /* Currently just support one operand expressions. */
> -  if (arg0_op.num_ops != 1)
> -    return false;
> -
> -  tree new_arg0 = arg0_op.ops[0];
> +  tree new_arg0;
>    tree new_arg1;
> +  int opnum = -1;
>
>    /* If arg0 have > 1 use, then this transformation actually increases
>       the number of expressions evaluated at runtime.  */
>    if (!has_single_use (arg0))
>      return false;
> -  if (!is_factor_profitable (arg0_def_stmt, merge, new_arg0))
> -    return false;
>    if (gimple_has_location (arg0_def_stmt))
>      narg0_loc = gimple_location (arg0_def_stmt);
>
> @@ -387,14 +503,67 @@ factor_out_conditional_operation (edge e0, edge e1, 
> basic_block merge,
>        if (arg1_op.operands_occurs_in_abnormal_phi ())
>         return false;
>
> +      /* For the complex expression, don't factor
> +        out, that will confuse the uninitializing
> +        warnings.  */
> +      if (arg1_op.code == COMPLEX_EXPR)
> +       return false;
> +
>        /* If arg1 have > 1 use, then this transformation actually increases
>          the number of expressions evaluated at runtime.  */
>        if (!has_single_use (arg1))
>         return false;
>
> -      new_arg1 = arg1_op.ops[0];
> -      if (!is_factor_profitable (arg1_def_stmt, merge, new_arg1))
> +      opnum = find_different_opnum (arg0_op, arg1_op, &new_arg0, &new_arg1);
> +      if (opnum == -1)
> +       return false;
> +
> +      /* Check to make sure extending the lifetimes of all operands is ok.  
> */
> +      if (!is_factor_profitable (arg0_def_stmt, merge, arg0_op))
>         return false;
> +      if (!is_factor_profitable (arg1_def_stmt, merge, arg1_op))
> +       return false;
> +
> +      /* If this was a division and the operand is the divisor
> +        and either divisor was a constant, don't factor out
> +        the division; dividing by an explicit constant can be
> +        expanded better than without an constant.
> +        FIXME: maybe isel could undo this case.  */
> +      if (is_divide_or_mod_p (arg1_op.code)
> +         && opnum == 1
> +         && (poly_int_tree_p (new_arg0)
> +             || poly_int_tree_p (new_arg1)))
> +       return false;
> +
> +      /* For early phiopt, don't factor out constants for pointer plus.
> +        BOS pass does not like that factoring.  */
> +      if (early_p && arg1_op.code == POINTER_PLUS_EXPR
> +         && opnum == 1
> +         && TREE_CODE (new_arg0) != SSA_NAME
> +         && TREE_CODE (new_arg1) != SSA_NAME)
> +       return false;
> +
> +      /* BIT_FIELD_REF and BIT_INSERT_EXPR can't be factored out for non-0 
> operands
> +        as the other operands require constants. */
> +      if ((arg1_op.code == BIT_FIELD_REF
> +          || arg1_op.code == BIT_INSERT_EXPR)
> +         && opnum != 0)
> +       return false;
> +
> +      /* It is not profitability to factor out vec_perm with
> +        constant masks (operand 2).  The target might not support it
> +        and that might be invalid to do as such. Also with constants
> +        masks, the number of elements of the mask type does not need
> +        to match the number of elements of other operands and can be
> +        arbitrary integral vector type so factoring that out can't work.
> +        Note in the case where one mask is a constant and the other is not,
> +        the check for compatible types will reject the case the
> +        constant mask has the incompatible type.  */
> +      if (arg1_op.code == VEC_PERM_EXPR && opnum == 2
> +         && TREE_CODE (new_arg0) == VECTOR_CST
> +         && TREE_CODE (new_arg1) == VECTOR_CST)
> +       return false;
> +
>        if (gimple_has_location (arg1_def_stmt))
>         narg1_loc = gimple_location (arg1_def_stmt);
>
> @@ -409,17 +578,19 @@ factor_out_conditional_operation (edge e0, edge e1, 
> basic_block merge,
>             locus = narg0_loc;
>         }
>      }
> +  else if (arg0_op.num_ops != 1)
> +    return false;
>    else
>      {
> +      new_arg0 = arg0_op.ops[0];
> +      opnum = 0;
>        /* For constants only handle if the phi was the only one. */
>        if (single_non_singleton_phi_for_edges (phi_nodes (merge), e0, e1) == 
> NULL)
>         return false;
>        /* TODO: handle more than just casts here. */
>        if (!gimple_assign_cast_p (arg0_def_stmt))
>         return false;
> -
> -      /* arg0_def_stmt should be conditional.  */
> -      if (dominated_by_p (CDI_DOMINATORS, gimple_bb (phi), gimple_bb 
> (arg0_def_stmt)))
> +      if (!is_factor_profitable (arg0_def_stmt, merge, arg0_op))
>         return false;
>
>        /* If arg1 is an INTEGER_CST, fold it to new type if it fits, or else
> @@ -509,7 +680,7 @@ factor_out_conditional_operation (edge e0, edge e1, 
> basic_block merge,
>    gimple_match_op new_op = arg0_op;
>
>    /* Create the operation stmt if possible and insert it.  */
> -  new_op.ops[0] = temp;
> +  new_op.ops[opnum] = temp;
>    gimple_seq seq = NULL;
>    result = maybe_push_res_to_seq (&new_op, &seq, result);
>
> @@ -4108,7 +4279,7 @@ pass_phiopt::execute (function *)
>               gphi *phi = as_a <gphi *> (gsi_stmt (gsi));
>
>               if (factor_out_conditional_operation (e1, e2, merge, phi,
> -                 cond_stmt))
> +                 cond_stmt, early_p))
>                 {
>                   /* Start over if there was an operation that was factored 
> out because the new phi might have another opportunity.  */
>                   phis = phi_nodes (merge);
> diff --git a/gcc/tree-ssa-phiopt.h b/gcc/tree-ssa-phiopt.h
> new file mode 100644
> index 00000000000..bba3136965d
> --- /dev/null
> +++ b/gcc/tree-ssa-phiopt.h
> @@ -0,0 +1,24 @@
> +/* Copyright (C) 2026 Free Software Foundation, Inc.
> +
> +This file is part of GCC.
> +
> +GCC is free software; you can redistribute it and/or modify it
> +under the terms of the GNU General Public License as published by the
> +Free Software Foundation; either version 3, or (at your option) any
> +later version.
> +
> +GCC is distributed in the hope that it will be useful, but WITHOUT
> +ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> +FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> +for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with GCC; see the file COPYING3.  If not see
> +<http://www.gnu.org/licenses/>.  */
> +
> +#ifndef TREE_SSA_PHIOPT_H
> +#define TREE_SSA_PHIOPT_H
> +extern int find_different_opnum (const gimple_match_op &arg0_op,
> +                                const gimple_match_op &arg1_op,
> +                                tree *new_arg0, tree *new_arg1);
> +#endif
> --
> 2.43.0
>

Reply via email to