> -----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
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
