Hi Ezra,
> -----Original Message----- > From: [email protected] <[email protected]> > Sent: 09 September 2026 20:12 > To: [email protected] > Cc: Tamar Christina <[email protected]>; [email protected]; > [email protected]; Ezra Sitorus <[email protected]> > Subject: [RFC PATCH 5/5] gcc: add complex dot prod SLP pattern > > From: Ezra Sitorus <[email protected]> > > Recognise complex integer dot product patterns and try to emit complex > dot product instructions if supported. In particular, recognise loops > like this: > > struct cint16_t > { > int8_t re; > int8_t im; > }; > > for (int i = 0; i < N; i++) > res += (a[i].re * b[i].re - a[i].im * b[i].im); > > which would be transformed into this from previous patches: > > acc_1 = DOT_PROD<a_re, b_re, acc_0>; > acc_2 = 0 - acc_1; > acc_3 = DOT_PROD<a_im, b_im, acc_2>; > acc_4 = 0 - acc_3; > > and then try to emit: > > CDOT (a, b, acc) > > in the case of AArch64 with SVE2 support. > > gcc/ChangeLog: > * tree-vect-slp-patterns.cc > (slp_tree_zero_p, vect_build_complex_dot_prod_load, > complex_re_im_pair_p, > class complex_dot_prod_pattern): New. > (complex_operations_pattern::recognize): Add support for > complex_dot_prod_pattern::matches. > > gcc/testsuite/ChangeLog: > * testsuite/gcc.dg/vect/complex/bb-slp-complex-dot-int8.c: New > test. > > Here are some questions I have: > > * Why (not) not use a VEC_PERM_EXPR for > vect_build_complex_dot_prod_load? > > Something like this? > static slp_tree > vect_build_complex_dot_prod_load (slp_tree re, slp_tree im) > { > lane_permutation_t perm; > perm.create (2); > perm.quick_push (std::make_pair (0, 0)); > perm.quick_push (std::make_pair (1, 0)); > > slp_tree complex = vect_create_new_slp_node (2, VEC_PERM_EXPR); > SLP_TREE_LANE_PERMUTATION (complex) = perm; > > SLP_TREE_VECTYPE (complex) = SLP_TREE_VECTYPE (re); > SLP_TREE_REF_COUNT (complex) = 1; > SLP_TREE_LANES (complex) = 2; This isn't correct, in a repeating sequence, i.e. [r0,i0,r1,r1,..] which happens when e.g. you unroll LANES > 2. You need to maintain the same number of lanes as the children. This also assumes 1 "scalar" statement is 1 lane, which isn't always the case. > SLP_TREE_REPRESENTATIVE (complex) = NULL; > > SLP_TREE_CHILDREN (complex).quick_push (re); > SLP_TREE_CHILDREN (complex).quick_push (im); > SLP_TREE_REF_COUNT (re)++; > SLP_TREE_REF_COUNT (im)++; > > gcc_assert (STMT_VINFO_GROUPED_ACCESS (SLP_TREE_REPRESENTATIVE > (re))); > gcc_assert (STMT_VINFO_GROUPED_ACCESS (SLP_TREE_REPRESENTATIVE > (im))); > SLP_TREE_SCALAR_STMTS (complex).safe_push (SLP_TREE_SCALAR_STMTS > (re)[0]); > SLP_TREE_SCALAR_STMTS (complex).safe_push (SLP_TREE_SCALAR_STMTS > (im)[0]); > VEC_PERM_EXPR don't have scalar statements normally. > return complex; > } What's the difference between this and vect_build_combine_node? Guess I'll see when I get to the relevant part :) > > It seems to generate the same codegen as the current patch. > --- > .../vect/complex/bb-slp-complex-dot-int8.c | 25 ++ > gcc/tree-vect-slp-patterns.cc | 358 ++++++++++++++++++ > 2 files changed, 383 insertions(+) > create mode 100644 gcc/testsuite/gcc.dg/vect/complex/bb-slp-complex- > dot-int8.c > > diff --git a/gcc/testsuite/gcc.dg/vect/complex/bb-slp-complex-dot-int8.c > b/gcc/testsuite/gcc.dg/vect/complex/bb-slp-complex-dot-int8.c > new file mode 100644 > index 000000000000..861b790158f6 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/vect/complex/bb-slp-complex-dot-int8.c > @@ -0,0 +1,25 @@ > +/* { dg-do compile { target aarch64-*-* } } */ > +/* { dg-additional-options "-O3 -fdump-tree-vect-details -march=armv8.2- > a+sve2" } */ > + > +#include <stdint.h> > + > +#define N 512 > + > +struct cint16_t > +{ > + int8_t re; > + int8_t im; > +}; > + > +int32_t > +complex_dot_prod_rot0 (const struct cint16_t *restrict a, > + const struct cint16_t *restrict b) > +{ > + int32_t res = 0; > + for (int i = 0; i < N; i++) > + res += (a[i].re * b[i].re - a[i].im * b[i].im); > + return res; > +} > + > +/* { dg-final { scan-tree-dump "Found COMPLEX_DOT_PROD_ROT0" "vect" } > } */ > +/* { dg-final { scan-tree-dump "add new stmt: > .*COMPLEX_DOT_PROD_ROT0" "vect" } } */ > diff --git a/gcc/tree-vect-slp-patterns.cc b/gcc/tree-vect-slp-patterns.cc > index c53325fecdc7..582d0473f3ca 100644 > --- a/gcc/tree-vect-slp-patterns.cc > +++ b/gcc/tree-vect-slp-patterns.cc > @@ -108,6 +108,40 @@ vect_pattern_validate_optab (internal_fn ifn, > slp_tree node) > return true; > } > > +static bool > +vect_pattern_validate_optab (internal_fn ifn, slp_tree, tree_pair types) > +{ > + tree type0 = types.first; > + tree type1 = types.second; > + if (ifn == IFN_LAST || !type0 || !type1) > + return false; I think this should be an assert, since something has gone wrong then. But isn't type1 the vectype of the slp_tree? So what you really want is an optional result type to check? Could we not then add an optional tree result_type = NULL_TREE to the existing vect_pattern_validate_optab and have one implementation? > + > + if (dump_enabled_p ()) > + dump_printf_loc (MSG_NOTE, vect_location, "Found %s pattern in SLP > tree\n", > + internal_fn_name (ifn)); > + > + if (direct_internal_fn_supported_p (ifn, types, OPTIMIZE_FOR_SPEED)) > + { > + if (dump_enabled_p ()) > + dump_printf_loc ( > + MSG_NOTE, vect_location, > + "Target supports %s vectorization with types %T and %T\n", > + internal_fn_name (ifn), type0, type1); > + } > + else > + { > + if (dump_enabled_p ()) > + { > + dump_printf_loc (MSG_NOTE, vect_location, > + "Target does not support %s for vector types " > + "%T and %T\n", > + internal_fn_name (ifn), type0, type1); > + } > + return false; > + } > + return true; > +} > + > > /****************************************************************** > ************* > * General helper types > > ******************************************************************* > ***********/ > @@ -1508,6 +1542,326 @@ complex_fms_pattern::build (vec_info *vinfo) > build_common (vinfo); > } > > +/***************************************************************** > ************** > + * complex_dot_prod_pattern class > + > ******************************************************************* > ***********/ > + > +static bool > +slp_tree_zero_p (slp_tree node) > +{ > + if (!node) > + return false; Should be an assert. > + > + if (SLP_TREE_SCALAR_OPS (node).is_empty ()) > + return false; > + Should check that the SLP_TREE_DEF_TYPE is a vect_constant_def. > + tree op; > + unsigned i; > + > + FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_OPS (node), i, op) > + if (!zerop (op)) > + return false; > + Just use a normal for each iterator for (tree op : SLP_TREE_SCALAR_OPS (node)) > + return true; > +} > + > +/* Build a 2-lane SLP load node for the consecutive members represented by > RE > + and IM. This is used in complex_dot_prod_pattern::build, and > + complex_dot_prod_pattern::matches verifies using complex_re_im_pair_p > that > + they are the adjacent members of the same load group. See below. > + */ > +static slp_tree > +vect_build_complex_dot_prod_load (slp_tree re, slp_tree im) > +{ > + slp_tree complex = vect_create_new_slp_node (0, SLP_TREE_CODE (re)); > + SLP_TREE_VECTYPE (complex) = SLP_TREE_VECTYPE (re); > + SLP_TREE_REF_COUNT (complex) = 1; > + SLP_TREE_LANES (complex) = 2; Don't build new load nodes from inside the pattern matcher. We want to share loads as much as possible and the pattern matcher has no context of this load. vect_build_combine_node deals with this already. Add some testcases where you increase the number of lanes and you'll see where your assumptions fall apart, e.g. int32_t complex_dot_prod_rot0 (const struct cint16_t *restrict a, const struct cint16_t *restrict b) { int32_t res = 0; for (int i = 0; i < N; i+=2) { res += (a[i].re * b[i].re - a[i].im * b[i].im); res += (a[i+1].re * b[i+1].re - a[i+1].im * b[i+1].im); } return res; } > + SLP_TREE_REPRESENTATIVE (complex) = SLP_TREE_REPRESENTATIVE (re); > + > + auto rep = SLP_TREE_REPRESENTATIVE (re); > + gcc_assert (STMT_VINFO_GROUPED_ACCESS (rep)); > + SLP_TREE_SCALAR_STMTS (complex).safe_push (SLP_TREE_SCALAR_STMTS > (re)[0]); > + SLP_TREE_SCALAR_STMTS (complex).safe_push (SLP_TREE_SCALAR_STMTS > (im)[0]); > + > + return complex; > +} > + > +/* Returns true if RE and IM are one-lane load nodes for consecutive > members > + of the same { real, imaginary } load group. */ > +static bool > +complex_re_im_pair_p (slp_tree re, slp_tree im) > +{ > + if (!re || !im || SLP_TREE_PERMUTE_P (re) || SLP_TREE_PERMUTE_P (im) > + || SLP_TREE_SCALAR_STMTS (re).length () != 1 > + || SLP_TREE_SCALAR_STMTS (im).length () != 1) > + return false; > + > + stmt_vec_info re_rep = SLP_TREE_REPRESENTATIVE (re); > + stmt_vec_info im_rep = SLP_TREE_REPRESENTATIVE (im); > + > + if (!re_rep || !im_rep || !STMT_VINFO_GROUPED_ACCESS (re_rep) > + || !STMT_VINFO_GROUPED_ACCESS (im_rep)) > + return false; > + > + stmt_vec_info first = DR_GROUP_FIRST_ELEMENT (re_rep); > + if (first != re_rep || DR_GROUP_SIZE (first) != 2 > + || DR_GROUP_NEXT_ELEMENT (first) != im_rep || DR_GROUP_GAP > (im_rep) != 1) > + return false; > + > + return true; > +} This looks like it just needs to extend compatible_complex_nodes_p for the group load case, this also caches the result so we don't repeatedly ask. > + > +class complex_dot_prod_pattern : public complex_pattern > +{ > +protected: > + complex_dot_prod_pattern (slp_tree *node, vec<slp_tree> *m_ops, > internal_fn ifn) > + : complex_pattern (node, m_ops, ifn) > + { > + this->m_num_args = 3; > + } > + > +public: > + void build (vec_info *) final override; > + static internal_fn matches (complex_operation_t op, > + slp_tree_to_load_perm_map_t *, > + slp_compat_nodes_map_t *, slp_tree *, > + vec<slp_tree> *); > + > + static vect_pattern *recognize (slp_tree_to_load_perm_map_t *, > + slp_compat_nodes_map_t *, slp_tree *); > + > + static vect_pattern * > + mkInstance (slp_tree *node, vec<slp_tree> *m_ops, internal_fn ifn) > + { > + return new complex_dot_prod_pattern (node, m_ops, ifn); > + } > +}; > + > +void > +complex_dot_prod_pattern::build (vec_info *vinfo) > +{ > + slp_tree node; > + slp_tree a_re = this->m_ops[0]; > + slp_tree a_im = this->m_ops[1]; > + slp_tree b_re = this->m_ops[2]; > + slp_tree b_im = this->m_ops[3]; > + slp_tree acc = this->m_ops[4]; > + > + slp_tree a_complex = vect_build_complex_dot_prod_load (a_re, a_im); > + slp_tree b_complex = vect_build_complex_dot_prod_load (b_re, b_im); > + SLP_TREE_REF_COUNT (acc)++; > + > + unsigned i; > + FOR_EACH_VEC_ELT (SLP_TREE_CHILDREN (*this->m_node), i, node) > + vect_free_slp_tree (node); > + > + SLP_TREE_CHILDREN (*this->m_node).release (); > + SLP_TREE_CHILDREN (*this->m_node).safe_grow (3); > + > + SLP_TREE_CHILDREN (*this->m_node)[0] = a_complex; > + SLP_TREE_CHILDREN (*this->m_node)[1] = b_complex; > + SLP_TREE_CHILDREN (*this->m_node)[2] = acc; > + SLP_TREE_REDUC_IDX (*this->m_node) = 2; > + > + /* This is taken from complex_pattern::build_common. There are some > changes > + which are required for this specific case: > + > + * input operands 0 and 1 have the same type, operand 2 (acc) has a > different type. > + * operand 2 is the reduction cycle operand. > + * the live reduction status needs to be preserved. > + * loop relevant classification should be preserved > (vect_used_only_live). > + > + For diagnostics only, the temp_ssa_name is cdot_slp_patt. */ > + > + stmt_vec_info stmt_info; > + auto_vec<tree> args; > + args.create (this->m_num_args); > + args.quick_grow_cleared (this->m_num_args); > + unsigned ix; > + stmt_vec_info call_stmt_info; > + gcall *call_stmt = NULL; > + > + /* Now modify the nodes themselves. */ > + FOR_EACH_VEC_ELT (this->m_workset, ix, node) > + { > + /* Calculate the location of the statement in NODE to replace. */ > + stmt_info = SLP_TREE_REPRESENTATIVE (node); > + gimple *old_stmt = STMT_VINFO_STMT (stmt_info); > + tree lhs_old_stmt = gimple_get_lhs (old_stmt); > + tree type = TREE_TYPE (lhs_old_stmt); > + > + /* Create the argument set for use by gimple_build_call_internal_vec. */ > + for (unsigned i = 0; i < this->m_num_args; i++) > + args[i] = gimple_get_lhs (STMT_VINFO_STMT ( > + SLP_TREE_REPRESENTATIVE (SLP_TREE_CHILDREN (node)[i]))); > + > + /* Create the new pattern statements. */ > + call_stmt = gimple_build_call_internal_vec (this->m_ifn, args); > + tree var = make_temp_ssa_name (type, call_stmt, "cdot_slp_patt"); > + gimple_call_set_lhs (call_stmt, var); > + gimple_set_location (call_stmt, gimple_location (old_stmt)); > + gimple_call_set_nothrow (call_stmt, true); > + > + /* Adjust the book-keeping for the new and old statements for use during > + SLP. This is required to get the right VF and statement during SLP > + analysis. These changes are created after relevancy has been set for > + 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)); > + > + STMT_VINFO_REDUC_IDX (call_stmt_info) = 2; > + STMT_VINFO_LIVE_P (call_stmt_info) = STMT_VINFO_LIVE_P (stmt_info); > + > + /* Make sure to mark the representative statement pure_slp and > + relevant and transfer reduction info. */ > + STMT_VINFO_RELEVANT (call_stmt_info) = STMT_VINFO_RELEVANT > (stmt_info); > + STMT_SLP_TYPE (call_stmt_info) = pure_slp; > + > + gimple_set_bb (call_stmt, gimple_bb (stmt_info->stmt)); > + STMT_VINFO_VECTYPE (call_stmt_info) = SLP_TREE_VECTYPE (node); > + > + /* Since we are replacing all the statements in the group with the same > + thing it doesn't really matter. So just set it every time a new stmt > + is created. */ > + SLP_TREE_REPRESENTATIVE (node) = call_stmt_info; > + SLP_TREE_LANE_PERMUTATION (node).release (); > + SLP_TREE_CODE (node) = ERROR_MARK; > + } > +} This seems to be doing a lot of the same work complex_pattern::build_common with the exception of the STMT_VINFO_REDUC_IDX and copying of the liveness for the reduction. I think build_common should be extended instead, it would save a lot on duplication. > + > +/* Pattern matcher for trying to match complex integer dot product pattern > + in SLP tree. If the operation matches then IFN is set to the operation > + IFN_COMPLEX_DOT_PROD_ROT0 and the arguments to the replacement > statement are > + put in m_ops. > + > + If no match is found then IFN is set to IFN_LAST and m_ops is unchanged. > + > + This function matches the patterns shaped as: > + > + acc_1 = DOT_PROD_EXPR (a_re, b_re, acc_0); > + acc_2 = 0 - acc_1; > + acc_3 = DOT_PROD_EXPR (a_im, b_im, acc_2); > + acc_4 = 0 - acc_3; > + > + which comes from the dot_prod and neg_dot_prod pattern that comes > from statements > + like this: > + > + acc = a[i] * b[i] - a[i + 1] * b[i + 1] + acc; > + > + On success, store { A_RE, A_IM, B_RE, B_IM, ACC } in OPS and return > + IFN_COMPLEX_DOT_PROD_ROT0. On failure, return IFN_LAST with OPS > unmodified. > + */ > + > +internal_fn > +complex_dot_prod_pattern::matches (complex_operation_t op, > + slp_tree_to_load_perm_map_t *, > + slp_compat_nodes_map_t *, > + slp_tree *ref_node, vec<slp_tree> *ops) > +{ > + /* We need to ignore the two_operands nodes that may also match, > + for that we can check if they have any scalar statements and also > + check that it's not a permute node as we're looking for a normal > + MINUS_EXPR operation. */ > + if (op != CMPLX_NONE) > + return IFN_LAST; > + > + slp_tree root = *ref_node; > + if (!vect_match_expression_p (root, MINUS_EXPR)) > + return IFN_LAST; > + > + /* Check that root is of the form 0 - root1. */ > + auto root_children = SLP_TREE_CHILDREN (root); > + if (root_children.length () != 2) > + return IFN_LAST; > + slp_tree zero1 = root_children[0]; > + slp_tree root1 = root_children[1]; > + > + if (!slp_tree_zero_p (zero1) > + || !vect_match_expression_p (root1, DOT_PROD_EXPR)) > + return IFN_LAST; > + > + auto root1_children = SLP_TREE_CHILDREN (root1); > + if (root1_children.length () != 3) > + return IFN_LAST; > + slp_tree a_im = root1_children[0]; > + slp_tree b_im = root1_children[1]; > + slp_tree acc_1 = root1_children[2]; > + > + /* Check that the imaginary dot product accumulates 0 - root2. */ > + if (!vect_match_expression_p (acc_1, MINUS_EXPR)) > + return IFN_LAST; > + > + auto acc1_children = SLP_TREE_CHILDREN (acc_1); > + if (acc1_children.length () != 2) > + return IFN_LAST; > + slp_tree zero2 = acc1_children[0]; > + slp_tree root2 = acc1_children[1]; > + > + if (!slp_tree_zero_p (zero2) > + || !vect_match_expression_p (root2, DOT_PROD_EXPR)) > + return IFN_LAST; > + > + auto root2_children = SLP_TREE_CHILDREN (root2); > + if (root2_children.length () != 3) > + return IFN_LAST; > + slp_tree a_re = root2_children[0]; > + slp_tree b_re = root2_children[1]; > + slp_tree acc = root2_children[2]; > + > + /* It's only valid to form complex dot product with integer types. */ > + tree acc_vectype = SLP_TREE_VECTYPE (root); > + if (!acc_vectype || !INTEGRAL_TYPE_P (TREE_TYPE (acc_vectype))) > + return IFN_LAST; > + > + tree input_vectype = SLP_TREE_VECTYPE (a_re); > + if (!input_vectype || !INTEGRAL_TYPE_P (TREE_TYPE (input_vectype))) > + return IFN_LAST; Move these checks up, as they're cheaper to check. > + > + /* Check that it is widening: 8 -> 32 bit. */ > + if (!(TYPE_PRECISION (TREE_TYPE (acc_vectype)) == 32 > + && TYPE_PRECISION (TREE_TYPE (input_vectype)) == 8)) > + return IFN_LAST; Seems odd, in principal 16 -> 64 is possible to, and 8 > 16. e.g. for normal dotprod we have two way dot products too. So I think this check just needs to enforce that recison of acc_vectype > input_vectype. Also it's simpler to use element_precision here. > + > + if (TYPE_UNSIGNED (TREE_TYPE (input_vectype))) > + return IFN_LAST; > + > + /* The two dot products and the original accumulator must have the same > + result type. */ > + if (SLP_TREE_VECTYPE (root1) != acc_vectype > + || SLP_TREE_VECTYPE (root2) != acc_vectype > + || !acc || SLP_TREE_VECTYPE (acc) != acc_vectype) > + return IFN_LAST; > + Do we need strict equality or equality in GIMPLE? i.e. can you use types_match (ty1, ty2, ty3)? Thanks, Tamar > + /* If we got here, it means we've matched the pattern. > + Check that a_re and a_im are adjacent. Likewise for b_re and b_im. */ > + if (!complex_re_im_pair_p (a_re, a_im)) > + return IFN_LAST; > + > + if (!complex_re_im_pair_p (b_re, b_im)) > + return IFN_LAST; > + > + if (input_vectype != SLP_TREE_VECTYPE (a_im) > + || input_vectype != SLP_TREE_VECTYPE (b_re) > + || SLP_TREE_VECTYPE (b_re) != SLP_TREE_VECTYPE (b_im)) > + return IFN_LAST; > + > + if (!vect_pattern_validate_optab (IFN_COMPLEX_DOT_PROD_ROT0, > *ref_node, > + tree_pair (acc_vectype, input_vectype))) > + return IFN_LAST; > + > + ops->truncate (0); > + ops->create (5); > + ops->quick_push (a_re); > + ops->quick_push (a_im); > + ops->quick_push (b_re); > + ops->quick_push (b_im); > + ops->quick_push (acc); > + > + return IFN_COMPLEX_DOT_PROD_ROT0; > +} > + > > /****************************************************************** > ************* > * complex_operations_pattern class > > ******************************************************************* > ***********/ > @@ -1573,6 +1927,10 @@ complex_operations_pattern::recognize > (slp_tree_to_load_perm_map_t *perm_cache, > if (ifn != IFN_LAST) > return complex_add_pattern::mkInstance (node, &ops, ifn); > > + ifn = complex_dot_prod_pattern::matches (op, perm_cache, ccache, node, > &ops); > + if (ifn != IFN_LAST) > + return complex_dot_prod_pattern::mkInstance (node, &ops, ifn); > + > return NULL; > } > > -- > 2.53.0
