On Wed, Jun 10, 2026 at 5:30 AM Richard Biener <[email protected]> wrote: > > 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?
I am trying to understand this question. So the profitability in the case where the operand that is different is constant is NOT independent from the type of operand and some code depend on the non-factoring case; e.g. objectsize. For arithmetic (and logic) operations, factoring is almost always a good idea because it can lead to phiopt or ifcvt (with cmove). Plus it will produce slightly smaller code in most cases. You are replacing one phi with a different one. This code never increases the number of phis (in the future it might decrease the number). That is: `a ? b + 1: b + 2` is always better as `b + (a ? 1 : 2)`. logical operations are similar. Multiply is a wash. shifts are also a wash. For division, division by a constant is always better than not division by a variable; and for an example data profiling will try to a/cst in the condition and there is a testcase looking for that exact case. And then there are some which can't be done, VEC_PERM, BIT_FIELD_REF, BIT_INSERT_EXPR (at least for the non first operand). As far as other part of the profitability, is_factor_profitable is independent of the actual operation. We can always fine tune this slightly more if needed too. > > > 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? Yes that works, moved. > > > (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. Used int_divide_or_mod_p. > > Otherwise looks OK to me. Ok, will push after building/testing is finished. (Will send out an updated version too). Thanks, Andrea > > > (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 > >
