On Wed, Nov 27, 2024 at 9:43 PM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> On Wed, Nov 27, 2024 at 4:26 AM liuhongt <hongtao....@intel.com> wrote:
> >
> > When loop requires any kind of versioning which could increase register
> > pressure too much, and it's in a deeply nest big loop, don't do
> > vectorization.
> >
> > I tested the patch with both Ofast and O2 for SPEC2017, besides
> > 548.exchange_r,
> > other benchmarks are same binary.
> >
> > Bootstrapped and regtested 0on x86_64-pc-linux-gnu{-m32,}
> > Any comments?
>
> The vectorizer tries to version an outer loop when vectorizing a loop nest
> and the versioning condition is invariant. See vect_loop_versioning. This
> tries to handle such cases. Often the generated runtime alias checks are
> not invariant because we do not consider the outer evolutions. I think we
> should instead fix this there.
>
> Question below ...
>
> > gcc/ChangeLog:
> >
> > pr target/117088
> > * config/i386/i386.cc
> > (ix86_vector_costs::ix86_vect_in_deep_nested_loop_p): New function.
> > (ix86_vector_costs::finish_cost): Prevent loop vectorization
> > if it's in a deeply nested loop and require versioning.
> > * config/i386/i386.opt (--param=vect-max-loop-depth=): New
> > param.
> > ---
> > gcc/config/i386/i386.cc | 89 ++++++++++++++++++++++++++++++++++++++++
> > gcc/config/i386/i386.opt | 4 ++
> > 2 files changed, 93 insertions(+)
> >
> > diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> > index 526c9df7618..608f40413d2 100644
> > --- a/gcc/config/i386/i386.cc
> > +++ b/gcc/config/i386/i386.cc
> > @@ -25019,6 +25019,8 @@ private:
> >
> > /* Estimate register pressure of the vectorized code. */
> > void ix86_vect_estimate_reg_pressure ();
> > + /* Check if vect_loop is in a deeply-nested loop. */
> > + bool ix86_vect_in_deep_nested_loop_p (class loop *vect_loop);
> > /* Number of GENERAL_REGS/SSE_REGS used in the vectorizer, it's used for
> > estimation of register pressure.
> > ??? Currently it's only used by vec_construct/scalar_to_vec
> > @@ -25324,6 +25326,84 @@ ix86_vector_costs::ix86_vect_estimate_reg_pressure
> > ()
> > }
> > }
> >
> > +/* Return true if vect_loop is in a deeply-nested loop.
> > + .i.e vect_loop_n in below loop structure.
> > +loop1
> > +{
> > + loop2
> > + {
> > + loop3
> > + {
> > + vect_loop_1;
> > + loop4
> > + {
> > + vect_loop_2;
> > + loop5
> > + {
> > + vect_loop_3;
> > + loop6
> > + {
> > + vect_loop_4;
> > + loop7
> > + {
> > + vect_loop_5;
> > + loop8
> > + {
> > + loop9
> > + }
> > + vect_loop_6;
> > + }
> > + vect_loop_7;
> > + }
> > + }
> > + }
> > + }
> > + It's a big hammer to fix O2 regression for 548.exchange_r after
> > vectorization
> > + is enhanced by (r15-4225-g70c3db511ba14f) */
> > +bool
> > +ix86_vector_costs::ix86_vect_in_deep_nested_loop_p (class loop *vect_loop)
> > +{
> > + if (loop_depth (vect_loop) > (unsigned) ix86_vect_max_loop_depth)
> > + return true;
> > +
> > + if (loop_depth (vect_loop) < 2)
> > + return false;
> > +
>
> while the above two are "obvious", what you check below isn't clear to me.
> Is this trying to compute whether 'vect_loop' is inside of a loop nest which
> at any sibling of vect_loop (or even sibling of an outer loop of vect_loop,
> recursively) is a sub-nest with a loop depth (relative to what?) exceeds
> ix86_vect_max_loop_depth?
Yes, the function tries to find if the vect_loop is in a "big outer
loop" which contains an innermost loop with loop_depth >
ix86_vect_max_loop_depth.
If yes, then prevent vectorization for the loop if its tripcount is
not constant VF-times.(requires any kind of versioning is not
accurate, and yes it's a big hammer.)
>
> > + class loop* outer_loop = loop_outer (vect_loop);
> > +
> > + auto_vec<class loop*> m_loop_stack;
> > + auto_sbitmap m_visited_loops (number_of_loops (cfun));
> > +
> > + /* Get all sibling loops for vect_loop. */
> > + class loop* next_loop = outer_loop->inner;
> > + for (; next_loop; next_loop = next_loop->next)
> > + {
> > + m_loop_stack.safe_push (next_loop);
> > + bitmap_set_bit (m_visited_loops, next_loop->num);
> > + }
> > +
> > + /* DFS the max depth of all sibling loop. */
> > + while (!m_loop_stack.is_empty ())
> > + {
> > + next_loop = m_loop_stack.pop ();
> > + if (loop_depth (next_loop) > (unsigned) ix86_vect_max_loop_depth)
> > + return true;
> > +
> > + class loop* inner_loop = next_loop->inner;
> > + while (inner_loop)
> > + {
> > + if (!bitmap_bit_p (m_visited_loops, inner_loop->num))
> > + {
> > + m_loop_stack.safe_push (inner_loop);
> > + bitmap_set_bit (m_visited_loops, inner_loop->num);
> > + }
> > + inner_loop = inner_loop->next;
> > + }
> > + }
> > +
> > + return false;
> > +}
> > +
> > void
> > ix86_vector_costs::finish_cost (const vector_costs *scalar_costs)
> > {
> > @@ -25344,6 +25424,15 @@ ix86_vector_costs::finish_cost (const vector_costs
> > *scalar_costs)
> > && (exact_log2 (LOOP_VINFO_VECT_FACTOR (loop_vinfo).to_constant
> > ())
> > > ceil_log2 (LOOP_VINFO_INT_NITERS (loop_vinfo))))
> > m_costs[vect_body] = INT_MAX;
> > +
> > + /* Prohibit vectorization when the loop requires versioning
> > + and loop_depth exceeds threshold. */
> > + if ((LOOP_REQUIRES_VERSIONING (loop_vinfo)
> > + || LOOP_VINFO_PEELING_FOR_NITER (loop_vinfo)
> > + || LOOP_VINFO_PEELING_FOR_ALIGNMENT (loop_vinfo)
> > + || vect_apply_runtime_profitability_check_p (loop_vinfo))
>
> this also applies to loops not requiring versioning - it practically applies
> to
> all loops that do not run for a constant VF-times times.
>
> > + && ix86_vect_in_deep_nested_loop_p (LOOP_VINFO_LOOP (loop_vinfo)))
> > + m_costs[vect_body] = INT_MAX;
> > }
> >
> > ix86_vect_estimate_reg_pressure ();
> > diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
> > index 99e86f545e8..c5abf83473d 100644
> > --- a/gcc/config/i386/i386.opt
> > +++ b/gcc/config/i386/i386.opt
> > @@ -1414,3 +1414,7 @@ Support MOVRS built-in functions and code generation.
> > mamx-movrs
> > Target Mask(ISA2_AMX_MOVRS) Var(ix86_isa_flags2) Save
> > Support AMX-MOVRS built-in functions and code generation.
> > +
> > +-param=vect-max-loop-depth=
> > +Target Joined UInteger Var(ix86_vect_max_loop_depth) Init(8) Param
> > +Preversion loop vectorization when it's in a deeply nested loop and
> > requires versioning, since it may increase register pressure too much.
> > --
> > 2.34.1
> >
--
BR,
Hongtao
