On Fri, Jun 5, 2026 at 11:37 AM Andrew Pinski
<[email protected]> wrote:
>
> On Fri, Jun 5, 2026 at 4:19 AM Kyrylo Tkachov <[email protected]> wrote:
> >
> >
> >
> > > On 5 Jun 2026, at 12:11, 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
> > >
> > > 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.
> > >
> > > 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.
> > > (is_divide_or_mod_p): New function.
> > > (factor_out_conditional_operation): Handle operands > 1,
> > > including operands communitive operands. Add early_p argument
> > > for costing.
> > > (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.
> > > Co-authored-by: Kyrylo Tkachov <[email protected]>
> > >
> > > Signed-off-by: Andrew Pinski <[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/phi-factor-binary-1.c | 60 ++++++
> > > gcc/tree-if-conv.cc | 80 +-------
> > > gcc/tree-ssa-phiopt.cc | 172 ++++++++++++++++--
> > > gcc/tree-ssa-phiopt.h | 24 +++
> > > 14 files changed, 319 insertions(+), 99 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/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/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 6a7e4c70722..f8881e61d2f 100644
> > > --- a/gcc/tree-ssa-phiopt.cc
> > > +++ b/gcc/tree-ssa-phiopt.cc
> > > @@ -297,6 +297,104 @@ is_factor_profitable (gimple *def_stmt, basic_block
> > > merge, tree arg)
> > > 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:
> > > + 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 +402,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 +457,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,12 +481,62 @@ 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;
> >
> > Btw, since you asked about it in your review of my patch, this is needed to
> > avoid regressing gcc.dg/uninit-17.c
> >
> > > +
> > > /* 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];
> > > + opnum = find_different_opnum (arg0_op, arg1_op, &new_arg0,
> > > &new_arg1);
> > > + if (opnum == -1)
> > > + 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
> > > except if they are equal. */
> > > + if (early_p && arg1_op.code == POINTER_PLUS_EXPR
> > > + && opnum == 1
> > > + && TREE_CODE (new_arg0) != SSA_NAME
> > > + && TREE_CODE (new_arg1) != SSA_NAME
> > > + && !operand_equal_p (new_arg0, new_arg1))
> > > + return false;
> >
> > I think allowing the factoring of constants for other expressions like
> > PLUS_EXPR leads to regressions in the aarch64 test suite in
> > gcc.target/aarch64/cinc_common_1.c and gcc.target/aarch64/fuse_cmp_csel.c
>
> fuse_cmp_csel.c looks like the scan should change.
> It is better to put the add after the csel.
>
> cinc_common_1.c seems like a missing combine due to splitting inefficiently:
Looking into this again, I see there is already PR 112304 which would
fix this too.
So I will just go an xfail it and add a comment to that PR .
> ```
> Trying 26, 27 -> 13:
> 26: r105:SI=cc:CC<0
> REG_DEAD cc:CC
> 27: r101:SI=r105:SI-0x2
> REG_DEAD r105:SI
> 13: r104:SI=r101:SI+r103:SI
> REG_DEAD r103:SI
> REG_DEAD r101:SI
> Failed to match this instruction:
> (set (reg:SI 104 [ _3 ])
> (plus:SI (plus:SI (lt:SI (reg:CC 66 cc)
> (const_int 0 [0]))
> (reg/v:SI 103 [ x ]))
> (const_int -2 [0xfffffffffffffffe])))
> Successfully matched this instruction:
> (set (reg/v:SI 101 [ t ])
> (lt:SI (reg:CC 66 cc)
> (const_int 0 [0])))
> Failed to match this instruction:
> (set (reg:SI 104 [ _3 ])
> (plus:SI (plus:SI (reg/v:SI 101 [ t ])
> (reg/v:SI 103 [ x ]))
> (const_int -2 [0xfffffffffffffffe])))
> ```
> If we had split at:
> (plus:SI (lt:SI (reg:CC 66 cc)
> (const_int 0 [0]))
> (reg/v:SI 103 [ x ]))
> Instead then it would just work. Let me see if I can add an aarch64
> pattern to fix this.
> Note you can reproduce the same output by doing the transformation
> this patch did manually in the code to show the issue.
> I will file a bug for this and xfail the testcase too.
>
> >
> > I ran into the same thing and ended up replacing the narrow POINTER_PLUS
> > guard with an unconditional "don't introduce a PHI of two constants” check.
> >
> > if (TREE_CODE (new_arg0) != SSA_NAME && TREE_CODE (new_arg1) != SSA_NAME)
> > return false;
> >
> > This is a strict superset of the early_p POINTER_PLUS guard — the
> > !operand_equal_p
> > sub-case there can't actually be reached, since find_different_opnum only
> > reports an operand as differing when it isn't operand-equal — so the
> > POINTER_PLUS guard can be dropped. I think that would allow avoiding some
> > slsr and recip tests fallout. But up to you if you want to keep it.
>
> recip tests really are not testing the correct thing any more which is
> why -fno-ssa-phiopt is needed. The number of divisions were never 3 in
> it; it was just faked that way with the non-factored case.
>
> I was thinking about lessening the restriction on find_different_opnum
> and on the "same" operand and return 0. or return -2 for all the same
> to factor it without a phi. But that is for a different patch on top
> of this one.
> So yes operand_equal should be dropped.
>
> >
> > Thanks,
> > Kyrill
> >
> > > +
> > > + /* 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 next 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 (!is_factor_profitable (arg0_def_stmt, merge, new_arg0))
> > > + return false;
> > > if (!is_factor_profitable (arg1_def_stmt, merge, new_arg1))
> > > return false;
> > > if (gimple_has_location (arg1_def_stmt))
> > > @@ -409,14 +553,20 @@ 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;
> > > + if (!is_factor_profitable (arg0_def_stmt, merge, new_arg0))
> > > + return false;
> > >
> > > /* arg0_def_stmt should be conditional. */
> > > if (dominated_by_p (CDI_DOMINATORS, gimple_bb (phi), gimple_bb
> > > (arg0_def_stmt)))
> > > @@ -509,7 +659,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);
> > >
> > > @@ -4105,7 +4255,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
> > >
> >
