On Thu, Oct 27, 2016 at 3:37 PM, Bin Cheng <bin.ch...@arm.com> wrote:
> Hi,
> During analysis, vect_slp checks if statements of a group are isomorphic to
> each other, specifically, all statements have to be isomorphic to the first
> one. Apparently, operands of commutative operators (PLUS_EXPR/MINUS_EXPR
> etc.) could be swapped when checking isomorphic property. Though vect_slp
> has basic support for such commutative operators, the related code is not
> properly implemented:
> 1) vect_build_slp_tree mixes operand swapping in the current slp tree node
> and operand swapping in its child slp tree nodes.
> 2) Operand swapping in the current slp tree is incorrect when
> vect_get_and_check_slp_defs has already committed to a fixed operand order.
> In addition, operand swapping for COND_EXPR is implemented in a wrong way
> (place) because:
> 3) vect_get_and_check_slp_defs swaps operands for COND_EXPR by changing
> comparison code after vect_build_slp_tree_1 checks the code consistency for
> the statement group.
> 4) vect_build_slp_tree_1 should support operand swapping for COND_EXPR
> while it doesn't.
>
> This patch addresses above issues. It supports COND_EXPR by recording
> swapping information in vect_build_slp_tree_1 and applies the swap in
> vect_get_check_slp_defs. It supports two types swapping: swapping and
> inverting. The patch also does refactoring so that operand swapping in child
> slp tree node and the current slp tree node are differentiated. With this
> patch, failures (gcc.dg/vect/slp-cond-3.c) revealed by previous COND_EXPR
> match.pd patches are resolved.
> Bootstrap and test on x86_64 and AArch64. Is it OK?
Ok, but please re-instantiate the early-out here:
@@ -905,18 +960,10 @@ vect_build_slp_tree (vec_info *vinfo,
slp_oprnd_info oprnd_info;
FOR_EACH_VEC_ELT (stmts, i, stmt)
{
- switch (vect_get_and_check_slp_defs (vinfo, stmt, i, &oprnds_info))
- {
- case 0:
- break;
- case -1:
- matches[0] = false;
- vect_free_oprnd_info (oprnds_info);
- return NULL;
^^^^
you seem to needlessly continue checking other DEFs if one returns -1.
Thanks,
Richard.
> Thanks,
> bin
>
> 2016-10-25 Bin Cheng <bin.ch...@arm.com>
>
> * tree-vect-slp.c (vect_get_and_check_slp_defs): New parameter SWAP.
> Check slp defs for COND_EXPR by swapping/inverting operands if
> indicated by the new parameter SWAP.
> (vect_build_slp_tree_1): New parameter SWAP. Check COND_EXPR stmt
> is isomorphic to the first stmt via swapping/inverting. Store swap
> information in the new parameter SWAP.
> (vect_build_slp_tree): New local array SWAP and pass it to function
> vect_build_slp_tree_1. Cleanup result handlding code for function
> call to vect_get_and_check_slp_defs. Skip oeprand swapping if the
> order of operands has been fixed as indicated by SWAP[i].
