> -----Original Message-----
> From: Richard Biener <[email protected]>
> Sent: 24 August 2026 12:02
> To: Tamar Christina <[email protected]>
> Cc: [email protected]
> Subject: RE: [PATCH] tree-optimization/126984 - ICE with complex SLP
> patterns
>
> 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.
>
Agreed! Tbh I don't remember why it's not :) But the code is "old" hehe.
Thanks,
Tamar
> 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)
