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

Reply via email to