Richard Biener <rguent...@suse.de> writes:
> We have to check each operand for being in a pattern, not just the
> first when avoiding build from scalars (we could possibly handle
> the special case of some of them being the pattern stmt root, but
> that would be a followup improvement).
>
> Bootstrap & regtest running on x86-64-unknown-linux-gnu.
>
> Richard.
>
> 2019-10-25 Richard Biener <rguent...@suse.de>
>
> PR tree-optimization/92222
> * tree-vect-slp.c (_slp_oprnd_info::first_pattern): Remove.
> (_slp_oprnd_info::second_pattern): Likewise.
> (_slp_oprnd_info::any_pattern): New.
> (vect_create_oprnd_info): Adjust.
> (vect_get_and_check_slp_defs): Compute whether any stmt is
> in a pattern.
> (vect_build_slp_tree_2): Avoid building up a node from scalars
> if any of the operand defs, not just the first, is in a pattern.
Is recording this in vect_get_and_check_slp_defs enough? AIUI we don't
get that far if vect_build_slp_tree_1 fails, but that doesn't prevent us
from building the vector from scalars on return from vect_build_slp_tree.
Was wondering whether we should use a helper function that explicitly
checks whether any stmt_info in the vec is a pattern statement, rather
than computing it on the fly.
Thanks,
Richard
>
> * gcc.dg/torture/pr92222.c: New testcase.
>
> Index: gcc/tree-vect-slp.c
> ===================================================================
> --- gcc/tree-vect-slp.c (revision 277441)
> +++ gcc/tree-vect-slp.c (working copy)
> @@ -177,8 +177,7 @@ typedef struct _slp_oprnd_info
> stmt. */
> tree first_op_type;
> enum vect_def_type first_dt;
> - bool first_pattern;
> - bool second_pattern;
> + bool any_pattern;
> } *slp_oprnd_info;
>
>
> @@ -199,8 +198,7 @@ vect_create_oprnd_info (int nops, int gr
> oprnd_info->ops.create (group_size);
> oprnd_info->first_dt = vect_uninitialized_def;
> oprnd_info->first_op_type = NULL_TREE;
> - oprnd_info->first_pattern = false;
> - oprnd_info->second_pattern = false;
> + oprnd_info->any_pattern = false;
> oprnds_info.quick_push (oprnd_info);
> }
>
> @@ -339,13 +337,11 @@ vect_get_and_check_slp_defs (vec_info *v
> tree oprnd;
> unsigned int i, number_of_oprnds;
> enum vect_def_type dt = vect_uninitialized_def;
> - bool pattern = false;
> slp_oprnd_info oprnd_info;
> int first_op_idx = 1;
> unsigned int commutative_op = -1U;
> bool first_op_cond = false;
> bool first = stmt_num == 0;
> - bool second = stmt_num == 1;
>
> if (gcall *stmt = dyn_cast <gcall *> (stmt_info->stmt))
> {
> @@ -418,13 +414,12 @@ again:
> return -1;
> }
>
> - if (second)
> - oprnd_info->second_pattern = pattern;
> + if (def_stmt_info && is_pattern_stmt_p (def_stmt_info))
> + oprnd_info->any_pattern = true;
>
> if (first)
> {
> oprnd_info->first_dt = dt;
> - oprnd_info->first_pattern = pattern;
> oprnd_info->first_op_type = TREE_TYPE (oprnd);
> }
> else
> @@ -1311,7 +1306,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,
> /* ??? Rejecting patterns this way doesn't work. We'd have to
> do extra work to cancel the pattern so the uses see the
> scalar version. */
> - && !is_pattern_stmt_p (SLP_TREE_SCALAR_STMTS (child)[0]))
> + && !oprnd_info->any_pattern)
> {
> slp_tree grandchild;
>
> @@ -1358,18 +1353,16 @@ vect_build_slp_tree_2 (vec_info *vinfo,
> /* ??? Rejecting patterns this way doesn't work. We'd have to
> do extra work to cancel the pattern so the uses see the
> scalar version. */
> - && !is_pattern_stmt_p (stmt_info))
> + && !is_pattern_stmt_p (stmt_info)
> + && !oprnd_info->any_pattern)
> {
> if (dump_enabled_p ())
> dump_printf_loc (MSG_NOTE, vect_location,
> "Building vector operands from scalars\n");
> this_tree_size++;
> - child = vect_create_new_slp_node (oprnd_info->def_stmts);
> - SLP_TREE_DEF_TYPE (child) = vect_external_def;
> - SLP_TREE_SCALAR_OPS (child) = oprnd_info->ops;
> + child = vect_create_new_slp_node (oprnd_info->ops);
> children.safe_push (child);
> oprnd_info->ops = vNULL;
> - oprnd_info->def_stmts = vNULL;
> continue;
> }
>
> @@ -1469,7 +1440,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,
> /* ??? Rejecting patterns this way doesn't work. We'd have
> to do extra work to cancel the pattern so the uses see the
> scalar version. */
> - && !is_pattern_stmt_p (SLP_TREE_SCALAR_STMTS (child)[0]))
> + && !oprnd_info->any_pattern)
> {
> unsigned int j;
> slp_tree grandchild;
> Index: gcc/testsuite/gcc.dg/torture/pr92222.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/torture/pr92222.c (nonexistent)
> +++ gcc/testsuite/gcc.dg/torture/pr92222.c (working copy)
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-ftree-vectorize" } */
> +
> +unsigned char *a;
> +int b;
> +void f();
> +void c()
> +{
> + char *d;
> + int e;
> + for (; b; b++) {
> + e = 7;
> + for (; e >= 0; e--)
> + *d++ = a[b] & 1 << e ? '1' : '0';
> + }
> + f();
> +}
