On Wed, 10 Jun 2026, [email protected] wrote: > From: Kyrylo Tkachov <[email protected]> > > factor_out_conditional_load currently only runs from the sink pass. Also call > it from phiopt itself when the join block of a diamond load PHI <*P, *Q> lies > outside any loop: there is nothing to loop-vectorise there, so commoning the > two arm loads into a single P' = PHI <P, Q>; result = *P' is a pure win and is > cheap to attempt. > > This is restricted to: > * the late phiopt run (!early_p), since after inlining the region may yet > become part of a loop where the commoned load could inhibit vectorisation; > * SSA-name arm pointers (a new REQUIRE_SSA_ARGS argument to > factor_out_conditional_load), so that phiprop does not undo it. > > This triggers a few 100s times in SPEC2026 with no harmful effects. The > llvm benchmarks improves slightly on my aarch64 machine (1.6%) > > Bootstrapped and tested on aarch64-none-linux-gnu. > Ok for trunk after the prerequisites?
But there's a pass_sink_code immediately before the late phiopt, so why not run it from there? > > Thanks, > Kyrill > > Signed-off-by: Kyrylo Tkachov <[email protected]> > > gcc/ > PR tree-optimization/125557 > * tree-ssa-phiopt.h (conditional_load_factorable_p) > (factor_out_conditional_load): Add a REQUIRE_SSA_ARGS parameter > defaulting to false. > * tree-ssa-phiopt.cc (conditional_load_factorable_p): Honour > REQUIRE_SSA_ARGS. > (factor_out_conditional_load): Pass it through. > (pass_phiopt::execute): Call factor_out_conditional_load for > out-of-loop diamonds in the late run. > > gcc/testsuite/ > > PR tree-optimization/125557 > * gcc.dg/tree-ssa/phiopt-factor-load-1.c: New test. > --- > .../gcc.dg/tree-ssa/phiopt-factor-load-1.c | 22 ++++++++++ > gcc/tree-ssa-phiopt.cc | 42 ++++++++++++++----- > gcc/tree-ssa-phiopt.h | 8 ++-- > 3 files changed, 59 insertions(+), 13 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phiopt-factor-load-1.c > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phiopt-factor-load-1.c > b/gcc/testsuite/gcc.dg/tree-ssa/phiopt-factor-load-1.c > new file mode 100644 > index 00000000000..fe6a5e9cf3f > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/phiopt-factor-load-1.c > @@ -0,0 +1,22 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-phiopt2" } */ > + > +/* PR tree-optimization/125557: outside any loop a diamond load PHI <*p, *q> > is > + factored by phiopt itself (not only by the sink pass) into a pointer PHI > + P' = PHI <p, q> followed by a single load *P', removing the duplicate > load. > + The arm pointers here are SSA names, so phiprop does not undo it. */ > + > +int > +f (int *p, int *q, int c) > +{ > + int x; > + if (c) > + x = *p; > + else > + x = *q; > + return x; > +} > + > +/* The two arm loads collapse to a pointer PHI feeding one load of its > result. */ > +/* { dg-final { scan-tree-dump {= PHI <p_[0-9]+\(D\)} "phiopt2" } } */ > +/* { dg-final { scan-tree-dump-times {= \*_[0-9]+;} 1 "phiopt2" } } */ > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > index c2cdc54f977..44523267626 100644 > --- a/gcc/tree-ssa-phiopt.cc > +++ b/gcc/tree-ssa-phiopt.cc > @@ -437,13 +437,15 @@ remove_factored_arm_defs (gimple *d0, gimple *d1) > } > > /* Return true if factor_out_conditional_load (below) would factor the load > PHI > - at MERGE. E0/E1 are the arm->MERGE edges. This is > factor_out_conditional_load's > - eligibility test, split out so callers (the sink pass) can gate on > commonability > - without mutating: the two MUST stay in lockstep, otherwise a caller could > common > - a load the sink then cannot finish if-converting. */ > + at MERGE. E0/E1 are the arm->MERGE edges; when REQUIRE_SSA_ARGS the two > arm > + pointers must themselves be SSA names. This is > factor_out_conditional_load's > + eligibility test, split out so callers (the sink pass) can gate on > + commonability without mutating: the two MUST stay in lockstep, otherwise a > + caller could common a load the sink then cannot finish if-converting. */ > > bool > -conditional_load_factorable_p (edge e0, edge e1, basic_block merge, gphi > *phi) > +conditional_load_factorable_p (edge e0, edge e1, basic_block merge, gphi > *phi, > + bool require_ssa_args) > { > if (virtual_operand_p (gimple_phi_result (phi)) > || gimple_phi_num_args (phi) != 2 > @@ -480,6 +482,13 @@ conditional_load_factorable_p (edge e0, edge e1, > basic_block merge, gphi *phi) > if (!types_compatible_p (TREE_TYPE (p0), TREE_TYPE (p1))) > return false; > > + /* The phiopt caller (REQUIRE_SSA_ARGS) only commons SSA-name pointers: > with a > + non-SSA arm pointer phiprop would propagate the load back into the arms > and > + undo the transform. The sink caller has no such issue and passes > false. */ > + if (require_ssa_args > + && (TREE_CODE (p0) != SSA_NAME || TREE_CODE (p1) != SSA_NAME)) > + return false; > + > return true; > } > > @@ -489,12 +498,15 @@ conditional_load_factorable_p (edge e0, edge e1, > basic_block merge, gphi *phi) > load *P' replacing the PHI. No speculative load is introduced (the load > uses > whichever pointer the taken edge selected). Together with operand > factoring > this turns a conditional load into one reg-offset load. E0/E1 are the > - arm->MERGE edges. Returns true if the load was factored out. */ > + arm->MERGE edges. REQUIRE_SSA_ARGS is as in conditional_load_factorable_p > + (the phiopt caller sets it so phiprop cannot undo the transform). Returns > + true if the load was factored out. */ > > bool > -factor_out_conditional_load (edge e0, edge e1, basic_block merge, gphi *phi) > +factor_out_conditional_load (edge e0, edge e1, basic_block merge, gphi *phi, > + bool require_ssa_args) > { > - if (!conditional_load_factorable_p (e0, e1, merge, phi)) > + if (!conditional_load_factorable_p (e0, e1, merge, phi, require_ssa_args)) > return false; > > /* Re-derive the loads and pointers validated by the predicate above. */ > @@ -4407,8 +4419,18 @@ pass_phiopt::execute (function *) > { > gphi *phi = as_a <gphi *> (gsi_stmt (gsi)); > > - if (factor_out_conditional_operation (e1, e2, merge, phi, > - cond_stmt, early_p)) > + bool factored > + = factor_out_conditional_operation (e1, e2, merge, phi, > + cond_stmt, early_p); > + /* Outside any loop a diamond load PHI <*P, *Q> cannot be > + loop-vectorised, so commoning it into P' = PHI <P, Q>; *P' is a > + pure win. Do it only when not in the early run (the region may > + yet become a loop after inlining) and only for SSA-name > pointers > + so phiprop cannot undo it. */ > + if (!factored && !early_p && bb_loop_depth (merge) == 0) > + factored = factor_out_conditional_load (e1, e2, merge, phi, > + > /*require_ssa_args=*/true); > + if (factored) > { > /* 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 > index 7383095f5a5..8f9b9edcdad 100644 > --- a/gcc/tree-ssa-phiopt.h > +++ b/gcc/tree-ssa-phiopt.h > @@ -25,11 +25,13 @@ extern int find_different_opnum (const gimple_match_op > &arg0_op, > /* True if factor_out_conditional_load would factor the load PHI at MERGE; > lets > the sink pass gate on commonability without mutating. */ > extern bool conditional_load_factorable_p (edge e0, edge e1, basic_block > merge, > - gphi *phi); > + gphi *phi, > + bool require_ssa_args = false); > /* Factor a load out of a "load PHI" PHI <*P, *Q> into P' = PHI <P, Q>; > - result = *P'. */ > + result = *P'. When REQUIRE_SSA_ARGS, only do so when P and Q are SSA > names > + (the phiopt caller sets this so phiprop cannot undo the transform). */ > extern bool factor_out_conditional_load (edge e0, edge e1, basic_block merge, > - gphi *phi); > + gphi *phi, bool require_ssa_args = > false); > /* Factor a common operation (unary, conversion, or N-ary with one differing > operand) out of a 2-argument PHI. */ > extern bool factor_out_conditional_operation (edge e0, edge e1, > -- Richard Biener <[email protected]> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Jochen Jaser, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
