On Fri, 25 Sep 2020, Tamar Christina wrote: > Hi All, > > This patch adds shared machinery for detecting patterns having to do with > complex number operations. The class ComplexPattern provides helpers for > matching and ultimately undoing the permutation in the tree by rebuilding the > graph. > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > > Ok for master?
I think you want to change all this to not look at individual stmts: + vect_match_expression_p (slp_tree node, tree_code code, int base, int idx, + stmt_vec_info *op1, stmt_vec_info *op2) + { + + vec<stmt_vec_info> scalar_stmts = SLP_TREE_SCALAR_STMTS (node); + _all_ lanes are supposed to match the operation in SLP_TREE_REPRESENTATIVE there's no need to do any operation-matching on lane granularity. The only thing making a difference here is VEC_PERM_EXPR nodes where again - there's no need to look at (eventually non-existant!) SLP_TREE_SCALAR_STMTS but its SLP_TREE_REPRESENTATIVE. + vec<slp_tree> children = SLP_TREE_CHILDREN (node); + + /* If it's a VEC_PERM_EXPR we need to look one deeper. VEC_PERM_EXPR + only have one entry. So pick on. */ + if (node->code == VEC_PERM_EXPR) + children = SLP_TREE_CHILDREN (children.last ()); that's too simplistic ;) + *op1 = SLP_TREE_SCALAR_STMTS (children[0])[n]; please make op1,op2 slp_tree, not stmt_vec_info. Where do you look at SLP_TREE_LANE_PERMUTATION at all? I think the result of + vect_detect_pair_op (int base, slp_tree node1, int offset1, slp_tree node2, + int offset2, vec<stmt_vec_info> *ops) could be simply the SLP_TREE_LANE_PERMUTATION? plus its two child nodes? In the ComplexAddPattern patch I see + /* Correct the arguments after matching. */ + std::swap (this->m_vects[1], this->m_vects[3]); how's that necessary? The replacement SLP node should end up with just a SLP_TREE_REPRESENTATIVE (the IFN function call). That is, the only thing necessary is verification / matching of the appropriate "interleaving" scheme imposed by SLP_TREE_LANE_PERMUTATION. I guess things would go wrong if instead of effectively blending two vectors we'd interleave two smaller vector type vectors? Say a 4-way add and a 4-way sub interleaved into a 8-way addsub, using V4SF and V8SF vectors? Going to stop looking at the series at this point, one further note is that all of the Complex*Patterns seem to share the initial match and thus redundantly do a vect_detect_pair_op repeatedly on each node for each pattern? I wonder if it could be commoned into a single pattern, they all seem to share the initial mixed plus/minus, then we have the multiplication on one or both operand cases. Richard. > Thanks, > Tamar > > gcc/ChangeLog: > > * tree-vect-slp-patterns.c (complex_operation_t,class ComplexPattern): > New. > > -- Richard Biener <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imend