On Wed, Sep 16, 2026 at 10:57 AM Andrea Pinski
<[email protected]> wrote:
>
> Currently we reject most commonalization of loads in
> factor_out_conditional_load
> before the vectorizer. This was done as the easiest and simpliest herustics
> at
> the time I was writing the code. We can do better than that. This inproves
> the heurstics by allowing pointers which don't analyize via scalar evolution.
> This means loads which are from other loads can be factored out earlier than
> the loop optimizers.
>
> This changes 2 testcases which are now factored out in phiopt2 and one
> testcase where we don't want the factoring out before the vectorizer as
> we want to be able to vectorize the code (derived from eembc's
> telecom/viterb00data_1 benchmark).
>
> Bootstrapped and tested on x86_64-linux-gnu.
>
> PR tree-optimization/125945
>
> gcc/ChangeLog:
>
> * tree-ssa-phiopt.cc (induction_based): New function.
> (factor_out_conditional_load): Allow some factoring out
> with early and/or before_vect.
> (factor_out_all): Set before_vect to false when not vectorizing.
> (pass_phiopt::execute): Initialize scev in some cases.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/tree-ssa/scc-diamond-1.c:
> * gcc.dg/tree-ssa/scc-diamond-3.c:
> * gcc.dg/tree-ssa/factor_op_phi-load-vect-1.c: New test.
Just a small change to the changelog that I missed about the testsuite:
* gcc.dg/tree-ssa/scc-diamond-1.c: Update to look at phiopt2
rather than phiopt4.
* gcc.dg/tree-ssa/scc-diamond-3.c: Likewise.
Thanks,
Andrea
>
> Signed-off-by: Andrea Pinski <[email protected]>
> ---
> .../tree-ssa/factor_op_phi-load-vect-1.c | 64 ++++++++++++++++++
> gcc/testsuite/gcc.dg/tree-ssa/scc-diamond-1.c | 4 +-
> gcc/testsuite/gcc.dg/tree-ssa/scc-diamond-3.c | 4 +-
> gcc/tree-ssa-phiopt.cc | 66 ++++++++++++++++---
> 4 files changed, 124 insertions(+), 14 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/factor_op_phi-load-vect-1.c
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/factor_op_phi-load-vect-1.c
> b/gcc/testsuite/gcc.dg/tree-ssa/factor_op_phi-load-vect-1.c
> new file mode 100644
> index 00000000000..c26ea907f2e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/factor_op_phi-load-vect-1.c
> @@ -0,0 +1,64 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fstrict-aliasing -fdump-tree-phiopt-details" } */
> +/* PR tree-optimization/125945 */
> +/* These factoring out of loads should happen only after the vectorizer for
> this code. */
> +typedef short s16;
> +struct S { s16 tag; s16 key; };
> +void f(s16 *dst, const struct S *a, const struct S *b, int n)
> +{
> + for (int i = 0; i < n; i++) {
> + s16 ka = a[i].key;
> + s16 kb = b[i].key;
> + if (ka >= kb)
> + dst[i] = a[i].tag;
> + else
> + dst[i] = b[i].tag;
> + }
> +}
> +void f1(s16 *dst, const struct S *a, const struct S *b, int n)
> +{
> + for (int i = 0; i < n; i++) {
> + s16 ka = a[i].key;
> + s16 kb = b[i].key;
> + s16 t;
> + if (ka >= kb)
> + t = a[i].tag;
> + else
> + t = b[i].tag;
> + dst[i] = t;
> + }
> +}
> +void f2(s16 *dst, const struct S *a, const struct S *b, int n)
> +{
> + for (int i = 0; i < n; i++) {
> + s16 ka = a[i].key;
> + s16 kb = b[i].key;
> + s16 t;
> + const struct S *tt = &a[i];
> + const struct S *tt1 = &b[i];
> + if (ka >= kb)
> + t = tt->tag;
> + else
> + t = tt1->tag;
> + dst[i] = t;
> + }
> +}
> +void f3(s16 *dst, const struct S *a, const struct S *b, int n)
> +{
> + for (int i = 0; i < n; i++) {
> + s16 ka = a[i].key;
> + s16 kb = b[i].key;
> + s16 t;
> + const s16 *tt = &a[i].tag;
> + const s16 *tt1 = &b[i].tag;
> + if (ka >= kb)
> + t = *tt;
> + else
> + t = *tt1;
> + dst[i] = t;
> + }
> +}
> +
> +/* { dg-final { scan-tree-dump-not "changed to factor out load from"
> "phiopt3" } } */
> +/* { dg-final { scan-tree-dump-not "changed to factor out load from"
> "phiopt2" } } */
> +/* { dg-final { scan-tree-dump-times "changed to factor out load from" 4
> "phiopt4" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scc-diamond-1.c
> b/gcc/testsuite/gcc.dg/tree-ssa/scc-diamond-1.c
> index de4f003bcf3..f6206273d14 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/scc-diamond-1.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/scc-diamond-1.c
> @@ -35,6 +35,6 @@ advance (const uint8_t *ip, size_t tag, const uint8_t *end)
> }
>
> /* The diamond is if-converted (branchless): one selected-offset load
> remains. */
> -/* { dg-final { scan-tree-dump-times "changed to factor out load from" 1
> "phiopt4" } } */
> -/* { dg-final { scan-tree-dump-times "changed to factor operation out from"
> 1 "phiopt4" } } */
> +/* { dg-final { scan-tree-dump-times "changed to factor out load from" 1
> "phiopt2" } } */
> +/* { dg-final { scan-tree-dump-times "changed to factor operation out from"
> 1 "phiopt2" } } */
> /* { dg-final { scan-tree-dump-times "changed to factor operation out from"
> 2 "phiopt1" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scc-diamond-3.c
> b/gcc/testsuite/gcc.dg/tree-ssa/scc-diamond-3.c
> index 8f9f6a4dead..00c8dd278bc 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/scc-diamond-3.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/scc-diamond-3.c
> @@ -46,6 +46,6 @@ advance (const uint8_t *ip, size_t tag, const uint8_t *end)
>
> /* The mismatched alias types are merged to the alias-everything type, so the
> loads are commoned and the diamond is if-converted. */
> -/* { dg-final { scan-tree-dump-times "changed to factor out load from" 1
> "phiopt4" } } */
> -/* { dg-final { scan-tree-dump-times "changed to factor operation out from"
> 1 "phiopt4" } } */
> +/* { dg-final { scan-tree-dump-times "changed to factor out load from" 1
> "phiopt2" } } */
> +/* { dg-final { scan-tree-dump-times "changed to factor operation out from"
> 1 "phiopt2" } } */
> /* { dg-final { scan-tree-dump-times "changed to factor operation out from"
> 2 "phiopt1" } } */
> diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> index fbb4ec545e6..84a7949583d 100644
> --- a/gcc/tree-ssa-phiopt.cc
> +++ b/gcc/tree-ssa-phiopt.cc
> @@ -4063,6 +4063,22 @@ cond_if_else_store_replacement (basic_block then_bb,
> basic_block else_bb,
> return ok;
> }
>
> +/* Returns true when P is based on an induction variable
> + inside MERGE's inner most loop. */
> +static bool
> +induction_based (tree p, basic_block merge)
> +{
> + if (TREE_CODE (p) != SSA_NAME)
> + return false;
> + tree ev = analyze_scalar_evolution (merge->loop_father, p);
> + if (chrec_contains_undetermined (ev)
> + || chrec_contains_symbols_defined_in_loop (ev,
> merge->loop_father->num))
> + return false;
> + if (tree_does_not_contain_chrecs (ev))
> + return false;
> + return true;
> +}
> +
> /* If PHI at MERGE is a "load PHI", PHI <*P, *Q> whose two arguments are
> single-use, non-volatile scalar MEM_REF loads reading the same memory
> state
> (same VUSE), factor the load out: introduce P' = PHI <P, Q> and a single
> @@ -4078,15 +4094,6 @@ static bool
> factor_out_conditional_load (edge e0, edge e1, basic_block merge, gphi *phi,
> bool early_p, bool before_vect)
> {
> - /* Factoring out a load during the first phi means we can't
> - trust if this is inside a loop or not; due to before inlining. */
> - if (early_p)
> - return false;
> -
> - /* Before vectorization, we don't want to factor out loads unless not
> inside a loop. */
> - if (before_vect && bb_loop_depth (merge) != 0)
> - return false;
> -
> /* Not a virtual operand. */
> if (virtual_operand_p (gimple_phi_result (phi))
> /* can only handle the merge bb having 2 predecessors. */
> @@ -4204,6 +4211,23 @@ factor_out_conditional_load (edge e0, edge e1,
> basic_block merge, gphi *phi,
> gimple_stmt_iterator gsi;
> gsi = gsi_after_labels (merge);
>
> + // factoring of the same pointer should be allowed
> + // irrespect to loops.
> + if (p0 == p1 && operand_equal_p (index0, index1))
> + ;
> + // Before inlining, we can't tell if different
> + // pointers are going to be induction variable based
> + // or not.
> + else if (early_p)
> + return false;
> + // Before vectorization, don't factor out
> + // pointers which are based on induction variables.
> + else if (before_vect
> + && bb_loop_depth (merge) != 0
> + && (induction_based (p0, merge)
> + || induction_based (p1, merge)))
> + return false;
> +
> /* Try to handle different indices. */
> if (operand_equal_p (index0, index1))
> newindex = fold_convert (atype, index0);
> @@ -4354,6 +4378,11 @@ factor_out_all (edge e1, edge e2, basic_block merge,
> {
> bool changed = false;
> bool do_over;
> + bool before_vect = !fold_before_rtl_expansion_p ();
> + // If vectorization is disable, then we are never before the vectorizer.
> + if (!flag_tree_loop_vectorize
> + && !merge->loop_father->force_vectorize)
> + before_vect = false;
> basic_block bb1 = e1->src;
> basic_block bb2 = e2->src;
> do
> @@ -4385,7 +4414,7 @@ factor_out_all (edge e1, edge e2, basic_block merge,
> /* Conditional load elimination can only be on a diamond. */
> if ((diamond_p
> && factor_out_conditional_load (e1, e2, merge, phi, early_p,
> - !fold_before_rtl_expansion_p
> ()))
> + before_vect))
> || factor_out_conditional_operation (e1, e2, merge, phi,
> cond_stmt, early_p))
> {
> @@ -4973,6 +5002,18 @@ pass_phiopt::execute (function *)
> {
> bool do_hoist_loads = !early_p ? gate_hoist_loads () : false;
> bool cfgchanged = false;
> + bool need_loop_finalize = false;
> +
> + if (!early_p
> + && !fold_before_rtl_expansion_p ()
> + && (flag_tree_loop_vectorize
> + || cfun->has_force_vectorize_loops)
> + && number_of_loops (cfun) > 1)
> + {
> + loop_optimizer_init (LOOPS_NORMAL);
> + scev_initialize ();
> + need_loop_finalize = true;
> + }
>
> calculate_dominance_info (CDI_DOMINATORS);
> mark_ssa_maybe_undefs ();
> @@ -5063,6 +5104,11 @@ pass_phiopt::execute (function *)
>
> execute_over_cond_phis (phiopt_exec);
>
> + if (need_loop_finalize)
> + {
> + loop_optimizer_finalize ();
> + scev_finalize ();
> + }
> if (replicate_conds_over_phis ())
> {
> free_dominance_info (CDI_DOMINATORS);
> --
> 2.43.0
>
