On Mon, 24 Aug 2026, Tamar Christina wrote:
> > -----Original Message-----
> > From: Richard Biener <[email protected]>
> > Sent: 24 August 2026 10:30
> > To: [email protected]
> > Cc: Tamar Christina <[email protected]>
> > Subject: [PATCH] tree-optimization/126984 - ICE with complex SLP patterns
> >
> > As VEC_PERM nodes no longer have a representative if those are at
> > the SLP pattern root we have to get at one by other means to be
> > able to create the scalar pattern to use.
> >
> > I've noticed that complex_pattern::build isn't really a full build
> > but is used as common helper and receives a garbled node only. So
> > I made that not an overload of build but a true helper, leaving
> > complex_pattern as pure abstract class. The new build_common helper
> > receives the representative (analysis should have picked and saved
> > one, but SLP patterns are not my area of expertise). In the callers
> > we still have the ungarbled node with original children, so we can
> > hope to figure a representative operation from there.
>
> Yeah the idea was that the callers prepare the arguments if they need to
> and call the child helper. I have no opinion on the renaming and happy to do
> so if more clear.
>
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >
> > OK?
>
> I agree with you that since the VEC_PERM_EXPR no longer has all the
> information
> needed we should store it before we modify the layout. Since all layout
> build starts
> from m_node if successful how about this instead
I'll adjust my patch to avoid the duplication, still complex_pattern
cannot stand on it's own, so making it abstract sounds better.
Thanks,
Richard.
> diff --git a/gcc/tree-vect-slp-patterns.cc b/gcc/tree-vect-slp-patterns.cc
> index bf9efacf161..d30bf1277f5 100644
> --- a/gcc/tree-vect-slp-patterns.cc
> +++ b/gcc/tree-vect-slp-patterns.cc
> @@ -483,10 +483,15 @@ class complex_pattern : public vect_pattern
> {
> protected:
> auto_vec<slp_tree> m_workset;
> + stmt_vec_info m_rep;
> complex_pattern (slp_tree *node, vec<slp_tree> *m_ops, internal_fn ifn)
> : vect_pattern (node, m_ops, ifn)
> {
> this->m_workset.safe_push (*node);
> + if (SLP_TREE_PERMUTE_P (*node))
> + this->m_rep = SLP_TREE_REPRESENTATIVE (SLP_TREE_CHILDREN (*node)[0]);
> + else
> + this->m_rep = SLP_TREE_REPRESENTATIVE (*node);
> }
>
> public:
> @@ -520,8 +525,6 @@ class complex_pattern : public vect_pattern
> void
> complex_pattern::build (vec_info *vinfo)
> {
> - stmt_vec_info stmt_info;
> -
> auto_vec<tree> args;
> args.create (this->m_num_args);
> args.quick_grow_cleared (this->m_num_args);
> @@ -534,8 +537,7 @@ complex_pattern::build (vec_info *vinfo)
> FOR_EACH_VEC_ELT (this->m_workset, ix, node)
> {
> /* Calculate the location of the statement in NODE to replace. */
> - stmt_info = SLP_TREE_SCALAR_STMTS (node)[0];
> - gimple* old_stmt = STMT_VINFO_STMT (stmt_info);
> + gimple* old_stmt = STMT_VINFO_STMT (this->m_rep);
> tree lhs_old_stmt = gimple_get_lhs (old_stmt);
> tree type = TREE_TYPE (lhs_old_stmt);
>
> @@ -556,14 +558,14 @@ complex_pattern::build (vec_info *vinfo)
> the nodes as such we need to manually update them. Any changes will
> be
> undone if SLP is cancelled. */
> call_stmt_info
> - = vinfo->add_pattern_stmt (call_stmt, vect_orig_stmt (stmt_info));
> + = vinfo->add_pattern_stmt (call_stmt, vect_orig_stmt (this->m_rep));
>
> /* Make sure to mark the representative statement pure_slp and
> relevant and transfer reduction info. */
> STMT_VINFO_RELEVANT (call_stmt_info) = vect_used_in_scope;
> STMT_SLP_TYPE (call_stmt_info) = pure_slp;
>
> - gimple_set_bb (call_stmt, gimple_bb (stmt_info->stmt));
> + gimple_set_bb (call_stmt, gimple_bb (this->m_rep->stmt));
> STMT_VINFO_VECTYPE (call_stmt_info) = SLP_TREE_VECTYPE (node);
>
> /* Since we are replacing all the statements in the group with the same
>
> --
>
> Which removes the code duplication.
>
> What ya think?
>
> Thanks,
> Tamar
>
> >
> > Thanks,
> > Richard.
> >
> > PR tree-optimization/126984
> > * tree-vect-slp-patterns.cc (complex_pattern::build):
> > Rename to ...
> > (complex_pattern::build_common): ... this and add a
> > stmt_vec_info parameter.
> > (complex_pattern::build): Likewise and adjust.
> > (complex_add_pattern::build): Compute a representative
> > and call build_common instead of complex_pattern::build.
> > (complex_mul_pattern::build): Likewise.
> > (complex_fms_pattern::build): Likewise.
> >
> > * gcc.dg/vect/vect-pr126984.c: New testcase.
> > ---
> > gcc/testsuite/gcc.dg/vect/vect-pr126984.c | 16 ++++++++++
> > gcc/tree-vect-slp-patterns.cc | 36 +++++++++++++++++------
> > 2 files changed, 43 insertions(+), 9 deletions(-)
> > create mode 100644 gcc/testsuite/gcc.dg/vect/vect-pr126984.c
> >
> > diff --git a/gcc/testsuite/gcc.dg/vect/vect-pr126984.c
> > b/gcc/testsuite/gcc.dg/vect/vect-pr126984.c
> > new file mode 100644
> > index 00000000000..ed7dc059cfa
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/vect/vect-pr126984.c
> > @@ -0,0 +1,16 @@
> > +/* { dg-do compile } */
> > +/* { dg-additional-options "-ffast-math" } */
> > +/* { dg-additional-options "-march=armv9-a" { target { aarch64-*-* } } } */
> > +
> > +double *a;
> > +double b, c, d, e, f;
> > +int g, h;
> > +void l() {
> > + double i, j, k;
> > + for (; g; g++, h += 2) {
> > + k = a[h];
> > + j = a[h + 1];
> > + a[h] = b * f - c * e + d * k - i * j;
> > + a[h + 1] = b * e + c * f + d * j + i * k;
> > + }
> > +}
> > diff --git a/gcc/tree-vect-slp-patterns.cc b/gcc/tree-vect-slp-patterns.cc
> > index bf9efacf161..879b917ab2f 100644
> > --- a/gcc/tree-vect-slp-patterns.cc
> > +++ b/gcc/tree-vect-slp-patterns.cc
> > @@ -489,9 +489,9 @@ class complex_pattern : public vect_pattern
> > this->m_workset.safe_push (*node);
> > }
> >
> > - public:
> > - void build (vec_info *) override;
> > + void build_common (vec_info *, stmt_vec_info);
> >
> > + public:
> > static internal_fn
> > matches (complex_operation_t op, slp_tree_to_load_perm_map_t *,
> > slp_tree *,
> > vec<slp_tree> *);
> > @@ -518,10 +518,8 @@ class complex_pattern : public vect_pattern
> > */
> >
> > void
> > -complex_pattern::build (vec_info *vinfo)
> > +complex_pattern::build_common (vec_info *vinfo, stmt_vec_info stmt_info)
> > {
> > - stmt_vec_info stmt_info;
> > -
> > auto_vec<tree> args;
> > args.create (this->m_num_args);
> > args.quick_grow_cleared (this->m_num_args);
> > @@ -534,7 +532,6 @@ complex_pattern::build (vec_info *vinfo)
> > FOR_EACH_VEC_ELT (this->m_workset, ix, node)
> > {
> > /* Calculate the location of the statement in NODE to replace. */
> > - stmt_info = SLP_TREE_SCALAR_STMTS (node)[0];
> > gimple* old_stmt = STMT_VINFO_STMT (stmt_info);
> > tree lhs_old_stmt = gimple_get_lhs (old_stmt);
> > tree type = TREE_TYPE (lhs_old_stmt);
> > @@ -611,6 +608,13 @@ class complex_add_pattern : public complex_pattern
> > void
> > complex_add_pattern::build (vec_info *vinfo)
> > {
> > + /* ??? We should not have to guess here, analysis should have saved it.
> > */
> > + stmt_vec_info rep;
> > + if (SLP_TREE_PERMUTE_P (*m_node))
> > + rep = SLP_TREE_REPRESENTATIVE (SLP_TREE_CHILDREN (*m_node)[0]);
> > + else
> > + rep = SLP_TREE_REPRESENTATIVE (*m_node);
> > +
> > SLP_TREE_CHILDREN (*this->m_node).reserve_exact (2);
> >
> > slp_tree node = this->m_ops[0];
> > @@ -626,7 +630,7 @@ complex_add_pattern::build (vec_info *vinfo)
> > vect_free_slp_tree (this->m_ops[0]);
> > vect_free_slp_tree (this->m_ops[1]);
> >
> > - complex_pattern::build (vinfo);
> > + build_common (vinfo, rep);
> > }
> >
> > /* Pattern matcher for trying to match complex addition pattern in SLP
> > tree.
> > @@ -1195,6 +1199,13 @@ complex_mul_pattern::recognize
> > (slp_tree_to_load_perm_map_t *perm_cache,
> > void
> > complex_mul_pattern::build (vec_info *vinfo)
> > {
> > + /* ??? We should not have to guess here, analysis should have saved it.
> > */
> > + stmt_vec_info rep;
> > + if (SLP_TREE_PERMUTE_P (*m_node))
> > + rep = SLP_TREE_REPRESENTATIVE (SLP_TREE_CHILDREN (*m_node)[0]);
> > + else
> > + rep = SLP_TREE_REPRESENTATIVE (*m_node);
> > +
> > slp_tree node;
> > unsigned i;
> > switch (this->m_ifn)
> > @@ -1243,7 +1254,7 @@ complex_mul_pattern::build (vec_info *vinfo)
> > }
> >
> > /* And then rewrite the node itself. */
> > - complex_pattern::build (vinfo);
> > + build_common (vinfo, rep);
> > }
> >
> >
> > /******************************************************************
> > *************
> > @@ -1481,6 +1492,13 @@ complex_fms_pattern::recognize
> > (slp_tree_to_load_perm_map_t *perm_cache,
> > void
> > complex_fms_pattern::build (vec_info *vinfo)
> > {
> > + /* ??? We should not have to guess here, analysis should have saved it.
> > */
> > + stmt_vec_info rep;
> > + if (SLP_TREE_PERMUTE_P (*m_node))
> > + rep = SLP_TREE_REPRESENTATIVE (SLP_TREE_CHILDREN (*m_node)[0]);
> > + else
> > + rep = SLP_TREE_REPRESENTATIVE (*m_node);
> > +
> > slp_tree node;
> > unsigned i;
> > slp_tree newnode =
> > @@ -1500,7 +1518,7 @@ complex_fms_pattern::build (vec_info *vinfo)
> > SLP_TREE_CHILDREN (*this->m_node).quick_push (this->m_ops[0]);
> >
> > /* And then rewrite the node itself. */
> > - complex_pattern::build (vinfo);
> > + build_common (vinfo, rep);
> > }
> >
> >
> > /******************************************************************
> > *************
> > --
> > 2.51.0
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG Nuernberg)
